diff --git a/packages/core/src/render3/reactivity/view_effect_runner.ts b/packages/core/src/render3/reactivity/view_effect_runner.ts index e81e75ca22bc..15f18333b680 100644 --- a/packages/core/src/render3/reactivity/view_effect_runner.ts +++ b/packages/core/src/render3/reactivity/view_effect_runner.ts @@ -32,6 +32,11 @@ export function runEffectsInView(view: LView): void { } else { effect.zone.run(() => effect.run()); } + + // Stop immediately if the view was destroyed during effect execution. + if (view[EFFECTS] === null) { + return; + } } // Check if we need to continue flushing. If we didn't find any dirty effects, then there's diff --git a/packages/core/test/render3/reactivity_spec.ts b/packages/core/test/render3/reactivity_spec.ts index 560a01dcfcef..e6196903371c 100644 --- a/packages/core/test/render3/reactivity_spec.ts +++ b/packages/core/test/render3/reactivity_spec.ts @@ -44,7 +44,7 @@ import { ViewContainerRef, } from '../../src/core'; import {EffectNode} from '../../src/render3/reactivity/effect'; -import {TestBed} from '../../testing'; +import {type ComponentFixture, TestBed} from '../../testing'; describe('reactivity', () => { describe('effects', () => { @@ -487,6 +487,87 @@ describe('reactivity', () => { expect(destroyed).toBeTrue(); }); + it("should stop running a view's remaining effects once an earlier one destroys the view", async () => { + const recorder: string[] = []; + let fixture: ComponentFixture; + + @Component({}) + class TestCmp { + readonly counter = signal(0); + + constructor() { + // Added first, so it's visited first when the view's effects are flushed. + effect(() => { + recorder.push(`a: ${this.counter()}`); + if (this.counter() === 1) { + fixture.destroy(); + } + }); + + // Added second. Also dirty in the same flush pass as "a" above, so it must not + // run once "a" has destroyed the view partway through that pass. + effect(() => { + recorder.push(`b: ${this.counter()}`); + }); + } + } + + fixture = TestBed.createComponent(TestCmp); + fixture.detectChanges(); + await fixture.whenStable(); + expect(recorder).toEqual(['a: 0', 'b: 0']); + + fixture.componentInstance.counter.set(1); + fixture.detectChanges(); + await fixture.whenStable(); + + expect(recorder).toEqual(['a: 0', 'b: 0', 'a: 1']); + }); + + it('should not restart the flush loop once an effect dirties a sibling and destroys the view in the same run', async () => { + const recorder: string[] = []; + const trigger = signal(0); + let fixture: ComponentFixture; + + @Component({}) + class TestCmp { + readonly counter = signal(0); + + constructor() { + // Added first, so it's already had its turn (and was not dirty) earlier in the + // same flush pass by the time "a" below dirties it. + effect(() => { + trigger(); + recorder.push('b'); + }); + + // Added second. On its second run, dirties "b" above (which sets + // HasChildViewsToRefresh on this view) and destroys the view in the same call. + // That combination makes the outer while loop want to restart even though the + // view is already gone. + effect(() => { + const value = this.counter(); + recorder.push(`a: ${value}`); + if (value === 1) { + trigger.update((v) => v + 1); + fixture.destroy(); + } + }); + } + } + + fixture = TestBed.createComponent(TestCmp); + fixture.detectChanges(); + await fixture.whenStable(); + expect(recorder).toEqual(['b', 'a: 0']); + + fixture.componentInstance.counter.set(1); + fixture.detectChanges(); + await fixture.whenStable(); + + expect(recorder).toEqual(['b', 'a: 0', 'a: 1']); + }); + it('should destroy effects when their DestroyRef is separately destroyed', () => { let destroyed = false; @Component({})