[React] Don't track signals read during SSR render #640
Conversation
🦋 Changeset detectedLatest commit: 8108153 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for preact-signals-demo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Size Change: +57 B (+0.07%) Total Size: 83.1 kB
ℹ️ View Unchanged
|
| @@ -312,7 +339,11 @@ export function _useSignalsImplementation( | |||
|
|
|||
| const storeRef = useRef<EffectStore>(); | |||
| if (storeRef.current == null) { | |||
| storeRef.current = createEffectStore(_usage); | |||
| if (typeof window === "undefined") { | |||
There was a problem hiding this comment.
The thing I am worried about here is react-native and others, shouldn't we detect this with i.e. layout-effect/effect firing?
There was a problem hiding this comment.
I guess it's not really possible so I'd add a global so we have an escape hatch, like a function enableX which defaults to the typeof window check
There was a problem hiding this comment.
Yea that may be a decent escape hatch, but Imma sit on this for a day or two more to see if some other solution presents itself.
What I'd really like is some hook or something that React would call as an effect in both SSR and client-side renders but I couldn't find one...
There was a problem hiding this comment.
It looks like React Native does define a global window object:
- https://stackoverflow.com/a/49911697
- https://github.com/facebook/react-native/blob/ec72af403ca6cc64a6c7e99b00a6a6b5d56ff2db/packages/react-native/Libraries/Core/setUpGlobals.js
So maybe this check will suffice?
Currently during all renders we setup an effect to track which signals a component reads, so we can rerender that component when a signal it uses changes. When that component unmounts, React calls our unsubscribe function and we dispose of the created effects.
However, during an SSR render, React does not call our unsubscribe function. Internally we use
useSyncExternalStoreto hook up React to signals. Subscribing/unsubscribing to stores is treated as a passive effect (aka like auseEffect), which aren't invoked on SSR renders. So while we setup our effect to track signal usage during SSR, the corresponding unsubscribe method is never called at the end of SSR so the tracking objects are never cleaned up from the Signal.In practice, each tracking object is ~100 bytes and so many applications may not notice this if they deploy frequently. You may notice GC times taking longer first before memory pressure. Regardless, this PR fixes it.
This PR fixes the memory leak by not creating any tracking effect if the
windowobject isundefined.