Skip to content

Commit 0daca45

Browse files
atscottatscott
committed
fix(core): TestBed should still use the microtask queue to schedule effects (#53843)
Prior to this commit, `TestBed` would require tests call `flushEffects` or `fixture.detectChanges` in order to execute effects. In general, we want to discourage authoring tests like this because it makes the timing of change detection and effects differ from what happens in the application. Instead, developers should perform actions and `await` (or `flush`/`tick` when using `fakeAsync`) some `Promise` so that Angular can react to the changes in the same way that it does in the application. Note that this still _allows_ developers to flush effects synchronously with `flushEffects` and `detectChanges` but also enables the <action>, `await` pattern described above. PR Close #53843
1 parent 12f9ed0 commit 0daca45

File tree

7 files changed

+57
-62
lines changed

7 files changed

+57
-62
lines changed

‎packages/core/src/core_reactivity_export_internal.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ export {
3131
EffectCleanupFn,
3232
EffectCleanupRegisterFn,
3333
EffectScheduler as ɵEffectScheduler,
34-
ZoneAwareQueueingScheduler as ɵZoneAwareQueueingScheduler
3534
} from './render3/reactivity/effect';
3635
export {
3736
assertNotInReactiveContext,

‎packages/core/src/render3/interfaces/view.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {DehydratedView} from '../../hydration/interfaces';
1212
import {SchemaMetadata} from '../../metadata/schema';
1313
import {Sanitizer} from '../../sanitization/sanitizer';
1414
import type {ReactiveLViewConsumer} from '../reactive_lview_consumer';
15-
import type {FlushableEffectRunner} from '../reactivity/effect';
15+
import type {EffectScheduler} from '../reactivity/effect';
1616
import type {AfterRenderEventManager} from '../after_render_hooks';
1717

1818
import {LContainer} from './container';
@@ -369,7 +369,7 @@ export interface LViewEnvironment {
369369
sanitizer: Sanitizer|null;
370370

371371
/** Container for reactivity system `effect`s. */
372-
inlineEffectRunner: FlushableEffectRunner|null;
372+
inlineEffectRunner: EffectScheduler|null;
373373

374374
/** Container for after render hooks */
375375
afterRenderEventManager: AfterRenderEventManager|null;

‎packages/core/src/render3/reactivity/effect.ts‎

Lines changed: 21 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -63,33 +63,40 @@ export abstract class EffectScheduler {
6363
*/
6464
abstract scheduleEffect(e: SchedulableEffect): void;
6565

66+
/**
67+
* Run any scheduled effects.
68+
*/
69+
abstract flush(): void;
70+
6671
/** @nocollapse */
6772
static ɵprov = /** @pureOrBreakMyCode */ ɵɵdefineInjectable({
6873
token: EffectScheduler,
6974
providedIn: 'root',
70-
factory: () => new ZoneAwareMicrotaskScheduler(),
75+
factory: () => new ZoneAwareEffectScheduler(),
7176
});
7277
}
7378

7479
/**
75-
* Interface to an `EffectScheduler` capable of running scheduled effects synchronously.
76-
*/
77-
export interface FlushableEffectRunner {
78-
/**
79-
* Run any scheduled effects.
80-
*/
81-
flush(): void;
82-
}
83-
84-
/**
85-
* An `EffectScheduler` which is capable of queueing scheduled effects per-zone, and flushing them
86-
* as an explicit operation.
80+
* A wrapper around `ZoneAwareQueueingScheduler` that schedules flushing via the microtask queue
81+
* when.
8782
*/
88-
export class ZoneAwareQueueingScheduler implements EffectScheduler, FlushableEffectRunner {
83+
export class ZoneAwareEffectScheduler implements EffectScheduler {
84+
private hasQueuedFlush = false;
8985
private queuedEffectCount = 0;
9086
private queues = new Map<Zone|null, Set<SchedulableEffect>>();
9187

9288
scheduleEffect(handle: SchedulableEffect): void {
89+
this.enqueue(handle);
90+
91+
if (!this.hasQueuedFlush) {
92+
queueMicrotask(() => this.flush());
93+
// Leave `hasQueuedFlush` as `true` so we don't queue another microtask if more effects are
94+
// scheduled during flushing. We are guaranteed to empty the whole queue during flush.
95+
this.hasQueuedFlush = false;
96+
}
97+
}
98+
99+
private enqueue(handle: SchedulableEffect): void {
93100
const zone = handle.creationZone as Zone | null;
94101
if (!this.queues.has(zone)) {
95102
this.queues.set(zone, new Set());
@@ -131,41 +138,6 @@ export class ZoneAwareQueueingScheduler implements EffectScheduler, FlushableEff
131138
handle.run();
132139
}
133140
}
134-
135-
/** @nocollapse */
136-
static ɵprov = /** @pureOrBreakMyCode */ ɵɵdefineInjectable({
137-
token: ZoneAwareQueueingScheduler,
138-
providedIn: 'root',
139-
factory: () => new ZoneAwareQueueingScheduler(),
140-
});
141-
}
142-
143-
/**
144-
* A wrapper around `ZoneAwareQueueingScheduler` that schedules flushing via the microtask queue
145-
* when.
146-
*/
147-
export class ZoneAwareMicrotaskScheduler implements EffectScheduler {
148-
private hasQueuedFlush = false;
149-
private delegate = new ZoneAwareQueueingScheduler();
150-
private flushTask = () => {
151-
// Leave `hasQueuedFlush` as `true` so we don't queue another microtask if more effects are
152-
// scheduled during flushing. The flush of the `ZoneAwareQueueingScheduler` delegate is
153-
// guaranteed to empty the queue.
154-
this.delegate.flush();
155-
this.hasQueuedFlush = false;
156-
157-
// This is a variable initialization, not a method.
158-
// tslint:disable-next-line:semicolon
159-
};
160-
161-
scheduleEffect(handle: SchedulableEffect): void {
162-
this.delegate.scheduleEffect(handle);
163-
164-
if (!this.hasQueuedFlush) {
165-
queueMicrotask(this.flushTask);
166-
this.hasQueuedFlush = true;
167-
}
168-
}
169141
}
170142

171143
/**

‎packages/core/test/test_bed_effect_spec.ts‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {Component, effect, inject, Injector} from '@angular/core';
9+
import {Component, effect, inject, Injector, NgZone, signal} from '@angular/core';
1010
import {TestBed} from '@angular/core/testing';
1111

1212
describe('effects in TestBed', () => {
@@ -85,4 +85,30 @@ describe('effects in TestBed', () => {
8585
'Effect',
8686
]);
8787
});
88+
89+
it('will flush effects automatically when using autoDetectChanges', async () => {
90+
const val = signal('initial');
91+
let observed = '';
92+
@Component({
93+
selector: 'test-cmp',
94+
standalone: true,
95+
template: '',
96+
})
97+
class Cmp {
98+
constructor() {
99+
effect(() => {
100+
observed = val();
101+
});
102+
}
103+
}
104+
105+
const fixture = TestBed.createComponent(Cmp);
106+
fixture.autoDetectChanges();
107+
108+
expect(observed).toBe('initial');
109+
val.set('new');
110+
expect(observed).toBe('initial');
111+
await fixture.whenStable();
112+
expect(observed).toBe('new');
113+
});
88114
});

‎packages/core/testing/src/component_fixture.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {ApplicationRef, ChangeDetectorRef, ComponentRef, DebugElement, ElementRef, getDebugNode, inject, NgZone, RendererFactory2, ɵDeferBlockDetails as DeferBlockDetails, ɵgetDeferBlocks as getDeferBlocks, ɵNoopNgZone as NoopNgZone, ɵZoneAwareQueueingScheduler as ZoneAwareQueueingScheduler} from '@angular/core';
9+
import {ApplicationRef, ChangeDetectorRef, ComponentRef, DebugElement, ElementRef, getDebugNode, inject, NgZone, RendererFactory2, ɵDeferBlockDetails as DeferBlockDetails, ɵEffectScheduler as EffectScheduler, ɵgetDeferBlocks as getDeferBlocks, ɵNoopNgZone as NoopNgZone} from '@angular/core';
1010
import {Subscription} from 'rxjs';
1111

1212
import {DeferBlockFixture} from './defer';
@@ -52,7 +52,7 @@ export class ComponentFixture<T> {
5252
private readonly noZoneOptionIsSet = inject(ComponentFixtureNoNgZone, {optional: true});
5353
private _ngZone: NgZone = this.noZoneOptionIsSet ? new NoopNgZone() : inject(NgZone);
5454
private _autoDetect = inject(ComponentFixtureAutoDetect, {optional: true}) ?? false;
55-
private effectRunner = inject(ZoneAwareQueueingScheduler, {optional: true});
55+
private effectRunner = inject(EffectScheduler);
5656
private _subscriptions = new Subscription();
5757
// Inject ApplicationRef to ensure NgZone stableness causes after render hooks to run
5858
// This will likely happen as a result of fixture.detectChanges because it calls ngZone.run
@@ -134,15 +134,15 @@ export class ComponentFixture<T> {
134134
* Trigger a change detection cycle for the component.
135135
*/
136136
detectChanges(checkNoChanges: boolean = true): void {
137-
this.effectRunner?.flush();
137+
this.effectRunner.flush();
138138
// Run the change detection inside the NgZone so that any async tasks as part of the change
139139
// detection are captured by the zone and can be waited for in isStable.
140140
this._ngZone.run(() => {
141141
this._tick(checkNoChanges);
142142
});
143143
// Run any effects that were created/dirtied during change detection. Such effects might become
144144
// dirty in response to input signals changing.
145-
this.effectRunner?.flush();
145+
this.effectRunner.flush();
146146
}
147147

148148
/**

‎packages/core/testing/src/test_bed.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
Type,
2828
ɵconvertToBitFlags as convertToBitFlags,
2929
ɵDeferBlockBehavior as DeferBlockBehavior,
30+
ɵEffectScheduler as EffectScheduler,
3031
ɵflushModuleScopingQueueAsMuchAsPossible as flushModuleScopingQueueAsMuchAsPossible,
3132
ɵgetAsyncClassMetadataFn as getAsyncClassMetadataFn,
3233
ɵgetUnknownElementStrictMode as getUnknownElementStrictMode,
@@ -38,7 +39,6 @@ import {
3839
ɵsetUnknownElementStrictMode as setUnknownElementStrictMode,
3940
ɵsetUnknownPropertyStrictMode as setUnknownPropertyStrictMode,
4041
ɵstringify as stringify,
41-
ɵZoneAwareQueueingScheduler as ZoneAwareQueueingScheduler,
4242
} from '@angular/core';
4343

4444
/* clang-format on */
@@ -782,7 +782,7 @@ export class TestBedImpl implements TestBed {
782782
* @developerPreview
783783
*/
784784
flushEffects(): void {
785-
this.inject(ZoneAwareQueueingScheduler).flush();
785+
this.inject(EffectScheduler).flush();
786786
}
787787
}
788788

‎packages/platform-browser/testing/src/browser.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88
import {PlatformLocation} from '@angular/common';
99
import {MockPlatformLocation} from '@angular/common/testing';
10-
import {APP_ID, createPlatformFactory, NgModule, PLATFORM_INITIALIZER, platformCore, provideZoneChangeDetection, StaticProvider, ɵEffectScheduler as EffectScheduler, ɵZoneAwareQueueingScheduler as ZoneAwareQueueingScheduler} from '@angular/core';
10+
import {APP_ID, createPlatformFactory, NgModule, PLATFORM_INITIALIZER, platformCore, provideZoneChangeDetection, StaticProvider} from '@angular/core';
1111
import {BrowserModule, ɵBrowserDomAdapter as BrowserDomAdapter} from '@angular/platform-browser';
1212

1313
function initBrowserTests() {
@@ -36,8 +36,6 @@ export const platformBrowserTesting =
3636
{provide: APP_ID, useValue: 'a'},
3737
provideZoneChangeDetection(),
3838
{provide: PlatformLocation, useClass: MockPlatformLocation},
39-
{provide: ZoneAwareQueueingScheduler},
40-
{provide: EffectScheduler, useExisting: ZoneAwareQueueingScheduler},
4139
]
4240
})
4341
export class BrowserTestingModule {

0 commit comments

Comments
 (0)