Skip to content

Commit f42dd40

Browse files
committed
Fix deferred subscription lifecycle
Assisted-By: devx/e0620a8d-8624-4748-b9c7-ce390238fc37
1 parent d10812a commit f42dd40

2 files changed

Lines changed: 366 additions & 18 deletions

File tree

packages/react/runtime/src/index.ts

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ interface Effect {
5959
_dispose(): void;
6060
}
6161

62+
type SubscriptionNode = Parameters<Signal["_subscribe"]>[0];
63+
6264
/**
6365
* Use this flag to represent a bare `useSignals` call that doesn't manually
6466
* close its effect store and relies on auto-closing when the next useSignals is
@@ -102,38 +104,57 @@ export interface EffectStore {
102104
getSnapshot(): number;
103105
/** startEffect - begin tracking signals used in this component */
104106
_start(): void;
105-
_subscribers: Array<{ signal: Signal; node: any }>;
106-
_sub: typeof Signal.prototype._subscribe;
107+
_subscribers: Array<{ signal: Signal; node: SubscriptionNode }>;
107108
/** finishEffect - stop tracking the signals used in this component */
108109
f(): void;
109110
[symDispose](): void;
110111
}
111112

112113
let currentStore: EffectStore | undefined;
113114

114-
const realSubscribe = Signal.prototype._subscribe;
115115
function startComponentEffect(
116116
prevStore: EffectStore | undefined,
117117
nextStore: EffectStore
118118
) {
119-
nextStore._sub = prevStore ? prevStore._sub : realSubscribe;
120-
Signal.prototype._subscribe = function (this: Signal, node: any) {
121-
nextStore._subscribers.push({ signal: this, node });
119+
const previousSubscribe = Signal.prototype._subscribe;
120+
Signal.prototype._subscribe = function (
121+
this: Signal,
122+
node: SubscriptionNode
123+
) {
124+
if ((node as { _target: Effect })._target === nextStore.effect) {
125+
nextStore._subscribers.push({ signal: this, node });
126+
}
122127
};
123-
const endEffect = nextStore.effect._start();
128+
129+
let endEffect: () => void;
130+
try {
131+
endEffect = nextStore.effect._start();
132+
} catch (error) {
133+
Signal.prototype._subscribe = previousSubscribe;
134+
throw error;
135+
}
124136
currentStore = nextStore;
125137

126-
return finishComponentEffect.bind(nextStore, prevStore, endEffect);
138+
return finishComponentEffect.bind(
139+
nextStore,
140+
prevStore,
141+
endEffect,
142+
previousSubscribe
143+
);
127144
}
128145

129146
function finishComponentEffect(
130147
this: EffectStore,
131148
prevStore: EffectStore | undefined,
132-
endEffect: () => void
149+
endEffect: () => void,
150+
previousSubscribe: typeof Signal.prototype._subscribe
133151
) {
134-
Signal.prototype._subscribe = prevStore ? prevStore._sub : realSubscribe;
135-
endEffect();
136-
currentStore = prevStore;
152+
Signal.prototype._subscribe = previousSubscribe;
153+
try {
154+
endEffect();
155+
} finally {
156+
currentStore = prevStore;
157+
}
137158
}
138159

139160
/**
@@ -180,15 +201,15 @@ function createEffectStore(
180201
if (onChangeNotifyReact) onChangeNotifyReact();
181202
};
182203

183-
return {
204+
const store: EffectStore = {
184205
_usage,
185206
_subscribers: [],
186-
_sub: realSubscribe,
187207
effect: effectInstance,
188208
subscribe(onStoreChange) {
189209
onChangeNotifyReact = onStoreChange;
190210

191211
return function () {
212+
store._subscribers = [];
192213
/**
193214
* Rotate to next version when unsubscribing to ensure that components are re-run
194215
* when subscribing again.
@@ -310,16 +331,18 @@ function createEffectStore(
310331
},
311332
[symDispose]() {
312333
this.f();
334+
this._subscribers = [];
313335
},
314336
};
337+
338+
return store;
315339
}
316340

317341
const noop = () => {};
318342

319343
function createEmptyEffectStore(): EffectStore {
320344
return {
321345
_subscribers: [],
322-
_sub: noop as any,
323346
_usage: UNMANAGED,
324347
effect: {
325348
_sources: undefined,
@@ -385,10 +408,17 @@ export function _useSignalsImplementation(
385408
if (_usage === UNMANAGED) useIsomorphicLayoutEffect(cleanupTrailingStore);
386409

387410
useIsomorphicLayoutEffect(() => {
388-
store._subscribers.forEach(({ signal, node }) => {
389-
realSubscribe.call(signal, node);
390-
});
411+
const subscribers = store._subscribers;
391412
store._subscribers = [];
413+
414+
subscribers.forEach(({ signal, node }) => {
415+
if (
416+
(node as { _target: Effect })._target === store.effect &&
417+
(node as { _version: number })._version !== -1
418+
) {
419+
signal._subscribe(node);
420+
}
421+
});
392422
});
393423

394424
return store;

0 commit comments

Comments
 (0)