Skip to content

Commit e7659be

Browse files
authored
fix(reactivity): unlink effect scopes on out-of-order off (#14734)
close #14733
1 parent 268115d commit e7659be

4 files changed

Lines changed: 117 additions & 1 deletion

File tree

packages/reactivity/__tests__/effectScope.spec.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,25 @@ describe('reactivity/effect/scope', () => {
296296
})
297297
})
298298

299+
it('calling .off() out of order should unlink the scope from the active chain', () => {
300+
const parentScope = effectScope(true)
301+
const firstScope = effectScope(true)
302+
const secondScope = effectScope(true)
303+
304+
parentScope.on()
305+
firstScope.on()
306+
secondScope.on()
307+
308+
firstScope.off()
309+
expect(getCurrentScope()).toBe(secondScope)
310+
311+
secondScope.off()
312+
expect(getCurrentScope()).toBe(parentScope)
313+
314+
parentScope.off()
315+
expect(getCurrentScope()).toBeUndefined()
316+
})
317+
299318
it('should pause/resume EffectScope', async () => {
300319
const counter = reactive({ num: 0 })
301320
const fnSpy = vi.fn(() => counter.num)

packages/reactivity/src/effectScope.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,26 @@ export class EffectScope {
124124
*/
125125
off(): void {
126126
if (this._on > 0 && --this._on === 0) {
127-
activeEffectScope = this.prevScope
127+
// Fast path: in the common LIFO case this scope is still at the top
128+
// of the active chain, so we can restore the previous scope directly.
129+
if (activeEffectScope === this) {
130+
activeEffectScope = this.prevScope
131+
} else {
132+
// withAsyncContext() restores the current component scope for the
133+
// current async continuation, then defers its cleanup to a microtask.
134+
// If sibling continuations interleave (A restore -> B restore ->
135+
// A cleanup), activeEffectScope is already B instead of this scope A
136+
// when A's cleanup calls off(). Unlink A from the middle of the
137+
// active chain so a stale scope doesn't remain globally reachable.
138+
let current = activeEffectScope
139+
while (current) {
140+
if (current.prevScope === this) {
141+
current.prevScope = this.prevScope
142+
break
143+
}
144+
current = current.prevScope
145+
}
146+
}
128147
this.prevScope = undefined
129148
}
130149
}

packages/server-renderer/__tests__/ssrWatch.spec.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,22 @@ import {
33
defineComponent,
44
h,
55
nextTick,
6+
onScopeDispose,
67
ref,
78
watch,
89
watchEffect,
910
withAsyncContext,
1011
} from 'vue'
1112
import { type SSRContext, renderToString } from '../src'
1213

14+
const gc = () =>
15+
new Promise<void>(resolve => {
16+
setTimeout(() => {
17+
global.gc!()
18+
resolve()
19+
})
20+
})
21+
1322
describe('ssr: watch', () => {
1423
// #6013
1524
test('should work w/ flush:sync', async () => {
@@ -284,3 +293,62 @@ describe('ssr: watchEffect', () => {
284293
expect(msg).toBe('unchanged')
285294
})
286295
})
296+
297+
describe.skipIf(!global.gc)('ssr: watch gc', () => {
298+
test('should not retain apps when a watcher stop handle is registered with onScopeDispose after async context restore', async () => {
299+
const weakRefs: { deref(): unknown | undefined }[] = []
300+
301+
const ComponentA = defineComponent({
302+
async setup() {
303+
let __temp: any, __restore: any
304+
;[__temp, __restore] = withAsyncContext(() => Promise.resolve(false))
305+
const enabled = await __temp
306+
__restore()
307+
308+
const el = ref(null)
309+
const stop = watch(
310+
() => el.value,
311+
() => {},
312+
{ immediate: true },
313+
)
314+
onScopeDispose(stop)
315+
316+
return () => h('div', { ref: el }, `Component A ${enabled}`)
317+
},
318+
})
319+
320+
const ComponentB = defineComponent({
321+
async setup() {
322+
let __temp: any, __restore: any
323+
;[__temp, __restore] = withAsyncContext(() => Promise.resolve(false))
324+
const enabled = await __temp
325+
__restore()
326+
327+
return () => h('div', `Component B ${enabled}`)
328+
},
329+
})
330+
331+
async function renderOnce() {
332+
const app = createSSRApp({
333+
render: () => h('div', [h(ComponentA), h(ComponentB)]),
334+
})
335+
// @ts-expect-error ES2021 API
336+
weakRefs.push(new WeakRef(app))
337+
338+
const html = await renderToString(app)
339+
340+
expect(html).toContain('Component A false')
341+
expect(html).toContain('Component B false')
342+
}
343+
344+
for (let i = 0; i < 10; i++) {
345+
await renderOnce()
346+
}
347+
348+
for (let i = 0; i < 5; i++) {
349+
await gc()
350+
}
351+
352+
expect(weakRefs.filter(ref => ref.deref()).length).toBe(0)
353+
})
354+
})

vitest.config.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,19 @@ export default defineConfig({
5656
...configDefaults.exclude,
5757
'**/e2e/**',
5858
'**/{vue,vue-compat,runtime-dom}/**',
59+
'packages/server-renderer/__tests__/ssrWatch.spec.ts',
5960
],
6061
},
6162
},
63+
{
64+
extends: true,
65+
test: {
66+
name: 'unit-gc',
67+
pool: 'forks',
68+
include: ['packages/server-renderer/__tests__/ssrWatch.spec.ts'],
69+
execArgv: ['--expose-gc'],
70+
},
71+
},
6272
{
6373
extends: true,
6474
test: {

0 commit comments

Comments
 (0)