fix: react strict mode compatibility #7
Conversation
| // Eagerly subscribe to sync set `entry.currentValue` to what the observable returns | ||
| // @TODO: perf opt opportunity: don't setup sync subscription initialValue is !== undefined | ||
| // Why not check `initialValue`? Because it might change during re-renders, but here we're only concerned with the first run | ||
| // if (entry.currentValue === undefined) { |
There was a problem hiding this comment.
I've tested this optimization and I didn't find any regressions.
However, given the scope of how it potentially impacts Studio v3 internals, I decided to comment it out for now.
| ] | ||
| }, [observable]) | ||
|
|
||
| useIsomorphicEffect(() => { |
There was a problem hiding this comment.
As this side-effect is only needed to clear up memory after unmounting it's not necessary to use useIsomorphicEffect/useLayoutEffect. The sync subscription ensures that, if possible, record.currentValue will have a value on first render and we don't have to pay the perf cost of useLayoutEffect (with its render + block browser paint + layout effect + render + allow browser paint flow).
| if (shouldRestoreSubscriptionRef.current) { | ||
| if (store.subscription.closed) { |
There was a problem hiding this comment.
These two checks ensures that the hook can be called out of order, and enforces a subscribe => unsubscribe => subscribe => unsubscribe symmetry no matter what React might decide doing in the new v18 features.
| // React StrictMode will call effects as `setup + teardown + setup` thus we can't trust this callback as "react is about to unmount" | ||
| // Tracking this ref lets us set the subscription back up on the next `setup` call if needed, and if it really did unmounted then all is well | ||
| shouldRestoreSubscriptionRef.current = !store.subscription.closed | ||
| store.subscription.unsubscribe() |
There was a problem hiding this comment.
Using store from the setup function, instead of doing a getOrCreateStore(observable, getValue(initialValue)) like before in teardown, it's ensured that if observable changes identity we don't "forget" to unsubscribe old stores and leak memory.
Co-authored-by: Cody Olsen <stipsan@gmail.com>
Co-authored-by: Cody Olsen <stipsan@gmail.com> Co-authored-by: Bjørge Næss <bjoerge@gmail.com>
2 participants
Fixes bugs related to React StrictMode as well as other minor nits