Skip to content

Commit

Permalink
Revert "fix(core): Ensure that a destroyed effect never run. (angul…
Browse files Browse the repository at this point in the history
…ar#59415)"

This reverts commit 5c0d688.
  • Loading branch information
kirjs committed Jan 15, 2025
1 parent ed705a8 commit 3c9fa53
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 64 deletions.
2 changes: 1 addition & 1 deletion goldens/size-tracking/integration-payloads.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
},
"forms": {
"uncompressed": {
"main": 181773,
"main": 176726,
"polyfills": 33772
}
},
Expand Down
1 change: 0 additions & 1 deletion packages/core/src/render3/reactivity/effect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,6 @@ export const ROOT_EFFECT_NODE: Omit<RootEffectNode, 'fn' | 'scheduler' | 'notifi
consumerDestroy(this);
this.onDestroyFn();
this.maybeCleanup();
this.scheduler.remove(this);
},
}))();

Expand Down
14 changes: 0 additions & 14 deletions packages/core/src/render3/reactivity/root_effect_scheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,6 @@ export abstract class EffectScheduler {
*/
abstract flush(): void;

/** Remove a scheduled effect */
abstract remove(e: SchedulableEffect): void;

/** @nocollapse */
static ɵprov = /** @pureOrBreakMyCode */ /* @__PURE__ */ ɵɵdefineInjectable({
token: EffectScheduler,
Expand All @@ -59,17 +56,6 @@ export class ZoneAwareEffectScheduler implements EffectScheduler {
this.enqueue(handle);
}

remove(handle: SchedulableEffect): void {
const zone = handle.zone as Zone | null;
const queue = this.queues.get(zone)!;
if (!queue.has(handle)) {
return;
}

queue.delete(handle);
this.queuedEffectCount--;
}

private enqueue(handle: SchedulableEffect): void {
const zone = handle.zone as Zone | null;
if (!this.queues.has(zone)) {
Expand Down
52 changes: 4 additions & 48 deletions packages/core/test/render3/reactivity_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
computed,
ContentChildren,
createComponent,
createEnvironmentInjector,
destroyPlatform,
Directive,
effect,
Expand All @@ -36,14 +37,16 @@ import {
ViewContainerRef,
} from '@angular/core';
import {SIGNAL} from '@angular/core/primitives/signals';
import {toObservable} from '@angular/core/rxjs-interop';
import {takeUntilDestroyed, toObservable} from '@angular/core/rxjs-interop';
import {createInjector} from '@angular/core/src/di/create_injector';
import {
EffectNode,
setUseMicrotaskEffectsByDefault,
} from '@angular/core/src/render3/reactivity/effect';
import {TestBed} from '@angular/core/testing';
import {bootstrapApplication} from '@angular/platform-browser';
import {withBody} from '@angular/private/testing';
import {filter, firstValueFrom, map} from 'rxjs';

describe('reactivity', () => {
let prev: boolean;
Expand Down Expand Up @@ -484,53 +487,6 @@ describe('reactivity', () => {
(fix.componentInstance.injector as Injector & {destroy(): void}).destroy();
expect(destroyed).toBeTrue();
});

it('should not run root effects after it has been destroyed', async () => {
let effectCounter = 0;
const counter = signal(1);
const effectRef = TestBed.runInInjectionContext(() =>
effect(
() => {
counter();
effectCounter++;
},
{injector: TestBed.inject(EnvironmentInjector)},
),
);
expect(effectCounter).toBe(0);
effectRef.destroy();
TestBed.flushEffects();
expect(effectCounter).toBe(0);

counter.set(2);
TestBed.flushEffects();
expect(effectCounter).toBe(0);
});

it('should not run view effects after it has been destroyed', async () => {
let effectCounter = 0;

@Component({template: ''})
class TestCmp {
counter = signal(1);
effectRef = effect(() => {
this.counter();
effectCounter++;
});
}

const fixture = TestBed.createComponent(TestCmp);
fixture.componentInstance.effectRef.destroy();
fixture.detectChanges();
expect(effectCounter).toBe(0);

TestBed.flushEffects();
expect(effectCounter).toBe(0);

fixture.componentInstance.counter.set(2);
TestBed.flushEffects();
expect(effectCounter).toBe(0);
});
});
});

Expand Down

0 comments on commit 3c9fa53

Please sign in to comment.