diff --git a/packages/reactivity/__tests__/effect.spec.ts b/packages/reactivity/__tests__/effect.spec.ts index 240af0b4fc0..1a91dff258c 100644 --- a/packages/reactivity/__tests__/effect.spec.ts +++ b/packages/reactivity/__tests__/effect.spec.ts @@ -7,7 +7,13 @@ import { render, serializeInner, } from '@vue/runtime-test' -import { ITERATE_KEY, getDepFromReactive } from '../src/dep' +import { + Dep, + ITERATE_KEY, + getDepFromReactive, + trackDep, + triggerDep, +} from '../src/dep' import { onEffectCleanup, pauseTracking, resetTracking } from '../src/effect' import { type DebuggerEvent, @@ -1430,4 +1436,145 @@ describe('reactivity/effect', () => { expect(reads).toEqual(__DEV__ ? ['onTrack', 'onTrigger'] : []) stop(runner) }) + + it('should notify remaining Dep subscribers when onTrigger removes the saved head', () => { + const dep = new Dep() + const seen: string[] = [] + let value = 0 + const first = effect(() => { + trackDep(dep) + seen.push(`first ${value}`) + }) + const second = effect( + () => { + trackDep(dep) + seen.push(`second ${value}`) + }, + { onTrigger: () => stop(first) }, + ) + const third = effect(() => { + trackDep(dep) + seen.push(`third ${value}`) + }) + + value = 1 + triggerDep(dep) + expect(seen).toEqual([ + 'first 0', + 'second 0', + 'third 0', + 'second 1', + 'third 1', + ]) + ;[first, second, third].forEach(stop) + }) + + it('should notify remaining computed subscribers when evaluation stops the first', () => { + const source = ref(0) + const seen: string[] = [] + const value = computed(() => { + if (source.value) stop(first) + return source.value + }) + const first = effect(() => seen.push(`first ${value.value}`)) + const second = effect(() => seen.push(`second ${value.value}`)) + source.value++ + expect(seen).toEqual(['first 0', 'second 0', 'first 1', 'second 1']) + stop(second) + }) + + it.each([false, true])( + 'should notify remaining subscribers when onTrigger stops the next effect (nested: %s)', + nested => { + const source = ref(0) + const other = ref(0) + const seen: string[] = [] + const nestedRunner = effect(() => other.value) + const first = effect(() => seen.push(`first ${source.value}`), { + onTrigger() { + stop(second) + if (nested) other.value++ + }, + }) + const second = effect(() => seen.push(`second ${source.value}`)) + const third = effect(() => seen.push(`third ${source.value}`)) + + source.value++ + + expect(seen).toEqual([ + 'first 0', + 'second 0', + 'third 0', + 'first 1', + 'third 1', + ]) + ;[first, second, third, nestedRunner].forEach(stop) + }, + ) + + describe.each([ + ['first', 0, 3], + ['middle', 1, 3], + ['last', 2, 3], + ['only', 0, 1], + ] as const)('removing the %s subscriber', (_, index, count) => { + it('should detach stopped effects and keep remaining effects subscribed', () => { + const source = ref(0) + const other = ref(0) + const spies = Array.from({ length: count }, () => + vi.fn(() => source.value + other.value), + ) + const runners = spies.map(spy => effect(spy)) + const removed = runners[index].effect.deps! + + stop(runners[index]) + + source.value++ + other.value++ + spies.forEach((spy, i) => { + expect(spy).toHaveBeenCalledTimes(i === index ? 1 : 3) + expect(spy).toHaveLastReturnedWith(i === index ? 0 : 2) + }) + expect(removed.prevSub).toBeUndefined() + expect(removed.nextSub).toBeUndefined() + expect(removed.nextDep!.prevSub).toBeUndefined() + expect(removed.nextDep!.nextSub).toBeUndefined() + + const appended = vi.fn(() => source.value) + const runner = effect(appended) + source.value++ + expect(appended).toHaveLastReturnedWith(2) + expect(appended).toHaveBeenCalledTimes(2) + runners.forEach(stop) + stop(runner) + }) + + it('should detach stale dependencies and keep remaining effects subscribed', () => { + const source = ref(0) + const enabled = Array.from({ length: count }, () => ref(true)) + const spies = enabled.map(flag => + vi.fn(() => (flag.value ? source.value : -1)), + ) + const runners = spies.map(spy => effect(spy)) + const removed = runners[index].effect.deps!.nextDep! + + enabled[index].value = false + source.value++ + + spies.forEach((spy, i) => { + expect(spy).toHaveBeenCalledTimes(2) + expect(spy).toHaveLastReturnedWith(i === index ? -1 : 1) + }) + expect(removed.prevSub).toBeUndefined() + expect(removed.nextSub).toBeUndefined() + + enabled[index].value = true + source.value++ + spies.forEach((spy, i) => { + expect(spy).toHaveBeenCalledTimes(i === index ? 4 : 3) + expect(spy).toHaveLastReturnedWith(2) + }) + runners.forEach(stop) + }) + }) }) diff --git a/packages/reactivity/__tests__/effectScope.spec.ts b/packages/reactivity/__tests__/effectScope.spec.ts index 2bc03c3db09..95ea631ccb6 100644 --- a/packages/reactivity/__tests__/effectScope.spec.ts +++ b/packages/reactivity/__tests__/effectScope.spec.ts @@ -521,6 +521,35 @@ describe('reactivity/effect/scope', () => { expect(secondSpy).not.toHaveBeenCalled() expect(getEffectsCount(scope)).toBe(1) }) + + it('should resume every effect when the first effect stops itself', () => { + const scope = effectScope() + const source = ref(0) + const seen: string[] = [] + + scope.run(() => { + const first = effect(() => { + seen.push(`first ${source.value}`) + if (source.value > 0) first.effect.stop() + }) + effect(() => seen.push(`second ${source.value}`)) + }) + + scope.pause() + source.value = 1 + scope.resume() + expect(seen).toEqual(['first 0', 'second 0', 'first 1', 'second 1']) + + source.value = 2 + expect(seen).toEqual([ + 'first 0', + 'second 0', + 'first 1', + 'second 1', + 'second 2', + ]) + scope.stop() + }) }) function getEffectsCount(scope: EffectScope): number { diff --git a/packages/reactivity/__tests__/gc.spec.ts b/packages/reactivity/__tests__/gc.spec.ts index 55499ec0a5e..2223f3520d5 100644 --- a/packages/reactivity/__tests__/gc.spec.ts +++ b/packages/reactivity/__tests__/gc.spec.ts @@ -112,4 +112,50 @@ describe.skipIf(!global.gc)('reactivity/gc', () => { await gc() expect(c.deref()).toBeUndefined() }) + + it('should release stopped effects across minor collections during subscriber churn', async ({ + skip, + }) => { + const minorGC = () => + new Promise(resolve => { + setTimeout(() => { + global.gc!({ type: 'minor' }) + resolve() + }) + }) + // Some V8 versions treat WeakRef targets as strong during minor GC. + // @ts-expect-error ES2021 API + const probe = new WeakRef({}) + await minorGC() + if (probe.deref() !== undefined) { + skip('This runtime keeps WeakRef targets alive during minor GC') + } + + const source = ref(0) + const live = Array.from({ length: 32 }, () => effect(() => source.value)) + // Promote the initial links so stale pointers from them would keep younger + // effects alive even after those effects are stopped. + await gc() + + const payloads = Array.from({ length: 256 }, (_, value) => { + const payload = { value } + live.push(effect(() => source.value + payload.value)) + live.shift()!.effect.stop() + // @ts-expect-error ES2021 API + return new WeakRef(payload) + }) + + // WeakRef targets are kept alive until the end of the current job. Collect + // in a new task, without a major GC that would also reclaim the old links. + await minorGC() + + payloads.forEach((payload, index) => { + if (index < payloads.length - live.length) { + expect(payload.deref()).toBeUndefined() + } else { + expect(payload.deref()).toBeDefined() + } + }) + live.forEach(runner => runner.effect.stop()) + }) }) diff --git a/packages/reactivity/src/dep.ts b/packages/reactivity/src/dep.ts index 40d895a3a63..18331079571 100644 --- a/packages/reactivity/src/dep.ts +++ b/packages/reactivity/src/dep.ts @@ -77,7 +77,10 @@ export function triggerDep( }) } propagate(subs) - shallowPropagate(subs) + const currentSubs = dep.subs + if (currentSubs !== undefined) { + shallowPropagate(currentSubs) + } if (!batchDepth) { flush() } diff --git a/packages/reactivity/src/system.ts b/packages/reactivity/src/system.ts index 4a1729169da..080c2503568 100644 --- a/packages/reactivity/src/system.ts +++ b/packages/reactivity/src/system.ts @@ -56,6 +56,8 @@ export function decRunDepth(): void { let globalVersion = 0 let notifyIndex = 0 let notifyBufferLength = 0 +let propagationDepth = 0 +const unlinkedDuringPropagation: Link[] = [] export function setActiveSub(sub?: ReactiveNode): ReactiveNode | undefined { try { @@ -156,10 +158,31 @@ export function unlink( dep.flags |= ReactiveFlags.Dirty } } + // An active traversal may still hold this link as its saved successor. + if (propagationDepth) { + unlinkedDuringPropagation.push(link) + } else { + link.prevSub = link.nextSub = undefined + } return nextDep } export function propagate(link: Link): void { + ++propagationDepth + try { + propagateLinks(link) + } finally { + if (!--propagationDepth) { + // Removed links must not retain younger subscribers across minor GCs. + for (const removed of unlinkedDuringPropagation) { + removed.prevSub = removed.nextSub = undefined + } + unlinkedDuringPropagation.length = 0 + } + } +} + +function propagateLinks(link: Link): void { let next = link.nextSub let stack: Stack | undefined @@ -302,10 +325,14 @@ export function checkDirty(link: Link, sub: ReactiveNode): boolean { (depFlags & (ReactiveFlags.Mutable | ReactiveFlags.Dirty)) === (ReactiveFlags.Mutable | ReactiveFlags.Dirty) ) { - const subs = dep.subs! + const subs = dep.subs if ((dep as Computed).update()) { - if (subs.nextSub !== undefined) { - shallowPropagate(subs) + const currentSubs = dep.subs + if ( + currentSubs !== undefined && + (currentSubs !== subs || currentSubs.nextSub !== undefined) + ) { + shallowPropagate(currentSubs) } dirty = true } @@ -330,10 +357,14 @@ export function checkDirty(link: Link, sub: ReactiveNode): boolean { link = stack!.value stack = stack!.prev if (dirty) { - const subs = sub.subs! + const subs = sub.subs if ((sub as Computed).update()) { - if (subs.nextSub !== undefined) { - shallowPropagate(subs) + const currentSubs = sub.subs + if ( + currentSubs !== undefined && + (currentSubs !== subs || currentSubs.nextSub !== undefined) + ) { + shallowPropagate(currentSubs) } sub = link.sub continue