Skip to content

Commit bb6e850

Browse files
committed
unsubscribe from effect in store
1 parent d4f1d7a commit bb6e850

File tree

1 file changed

+29
-11
lines changed

1 file changed

+29
-11
lines changed

packages/react/src/index.ts

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,28 @@ const ProxyHandlers = {
3636
* With the native Proxy, all other calls such as access/setting to/of properties will
3737
* be forwarded to the target Component, so we don't need to copy the Component's
3838
* own or inherited properties.
39+
*
40+
* @see https://github.com/facebook/react/blob/2d80a0cd690bb5650b6c8a6c079a87b5dc42bd15/packages/react-reconciler/src/ReactFiberHooks.old.js#L460
3941
*/
4042
apply(Component: FunctionComponent, thisArg: any, argumentsList: any) {
4143
const store = useMemo(createEffectStore, Empty);
4244

4345
useSyncExternalStore(store.subscribe, store.getSnapshot, store.getSnapshot);
4446

45-
const ends = store.updater._start();
46-
const kids = Component.apply(thisArg, argumentsList);
47-
ends();
48-
49-
return kids;
47+
const stop = store.updater._start();
48+
49+
try {
50+
const children = Component.apply(thisArg, argumentsList);
51+
return children;
52+
} catch (e) {
53+
// Re-throwing promises that'll be handled by suspense
54+
// or an actual error.
55+
throw e;
56+
} finally {
57+
// Stop effects in either case before return or throw,
58+
// Otherwise the effect will leak.
59+
stop();
60+
}
5061
},
5162
};
5263

@@ -108,10 +119,8 @@ function createEffectStore() {
108119
updater = this;
109120
});
110121
updater._callback = function () {
111-
if (!onChangeNotifyReact) return void unsubscribe();
112-
113122
version = (version + 1) | 0;
114-
onChangeNotifyReact!();
123+
if (onChangeNotifyReact) onChangeNotifyReact();
115124
};
116125

117126
return {
@@ -121,10 +130,18 @@ function createEffectStore() {
121130

122131
return function () {
123132
/**
124-
* @todo - we may want to unsubscribe here instead?
125-
* Components wrapped in `memo` no longer get updated if we unsubscribe here.
133+
* Rotate to next version when unsubscribing to ensure that components are re-run
134+
* when subscribing again.
135+
*
136+
* In StrictMode, 'memo'-ed components seem to keep a stale snapshot version, so
137+
* don't re-run after subscribing again if the version is the same as last time.
138+
*
139+
* Because we unsubscribe from the effect, the version may not change. We simply
140+
* set a new initial version in case of stale snapshots here.
126141
*/
142+
version = (version + 1) | 0;
127143
onChangeNotifyReact = undefined;
144+
unsubscribe();
128145
};
129146
},
130147
getSnapshot() {
@@ -142,7 +159,8 @@ function WrapJsx<T>(jsx: T): T {
142159
}
143160

144161
if (type && typeof type === "object" && type.$$typeof === ReactMemoType) {
145-
return jsx.call(jsx, ProxyFunctionalComponent(type.type), props, ...rest);
162+
type.type = ProxyFunctionalComponent(type.type);
163+
return jsx.call(jsx, type, props, ...rest);
146164
}
147165

148166
if (typeof type === "string" && props) {

0 commit comments

Comments
 (0)