Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): TestBed should still use the microtask queue to schedule effects #53843

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/core/src/core_reactivity_export_internal.ts
Expand Up @@ -31,7 +31,6 @@ export {
EffectCleanupFn,
EffectCleanupRegisterFn,
EffectScheduler as ɵEffectScheduler,
ZoneAwareQueueingScheduler as ɵZoneAwareQueueingScheduler
} from './render3/reactivity/effect';
export {
assertNotInReactiveContext,
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/render3/interfaces/view.ts
Expand Up @@ -12,7 +12,7 @@ import {DehydratedView} from '../../hydration/interfaces';
import {SchemaMetadata} from '../../metadata/schema';
import {Sanitizer} from '../../sanitization/sanitizer';
import type {ReactiveLViewConsumer} from '../reactive_lview_consumer';
import type {FlushableEffectRunner} from '../reactivity/effect';
import type {EffectScheduler} from '../reactivity/effect';
import type {AfterRenderEventManager} from '../after_render_hooks';

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

/** Container for reactivity system `effect`s. */
inlineEffectRunner: FlushableEffectRunner|null;
inlineEffectRunner: EffectScheduler|null;

/** Container for after render hooks */
afterRenderEventManager: AfterRenderEventManager|null;
Expand Down
70 changes: 21 additions & 49 deletions packages/core/src/render3/reactivity/effect.ts
Expand Up @@ -63,33 +63,40 @@ export abstract class EffectScheduler {
*/
abstract scheduleEffect(e: SchedulableEffect): void;

/**
* Run any scheduled effects.
*/
abstract flush(): void;

/** @nocollapse */
static ɵprov = /** @pureOrBreakMyCode */ ɵɵdefineInjectable({
token: EffectScheduler,
providedIn: 'root',
factory: () => new ZoneAwareMicrotaskScheduler(),
factory: () => new ZoneAwareEffectScheduler(),
});
}

/**
* Interface to an `EffectScheduler` capable of running scheduled effects synchronously.
*/
export interface FlushableEffectRunner {
/**
* Run any scheduled effects.
*/
flush(): void;
}

/**
* An `EffectScheduler` which is capable of queueing scheduled effects per-zone, and flushing them
* as an explicit operation.
* A wrapper around `ZoneAwareQueueingScheduler` that schedules flushing via the microtask queue
* when.
*/
export class ZoneAwareQueueingScheduler implements EffectScheduler, FlushableEffectRunner {
export class ZoneAwareEffectScheduler implements EffectScheduler {
private hasQueuedFlush = false;
private queuedEffectCount = 0;
private queues = new Map<Zone|null, Set<SchedulableEffect>>();

scheduleEffect(handle: SchedulableEffect): void {
this.enqueue(handle);

if (!this.hasQueuedFlush) {
queueMicrotask(() => this.flush());
// Leave `hasQueuedFlush` as `true` so we don't queue another microtask if more effects are
// scheduled during flushing. We are guaranteed to empty the whole queue during flush.
this.hasQueuedFlush = false;
}
}

private enqueue(handle: SchedulableEffect): void {
const zone = handle.creationZone as Zone | null;
if (!this.queues.has(zone)) {
this.queues.set(zone, new Set());
Expand Down Expand Up @@ -131,41 +138,6 @@ export class ZoneAwareQueueingScheduler implements EffectScheduler, FlushableEff
handle.run();
}
}

/** @nocollapse */
static ɵprov = /** @pureOrBreakMyCode */ ɵɵdefineInjectable({
token: ZoneAwareQueueingScheduler,
providedIn: 'root',
factory: () => new ZoneAwareQueueingScheduler(),
});
}

/**
* A wrapper around `ZoneAwareQueueingScheduler` that schedules flushing via the microtask queue
* when.
*/
export class ZoneAwareMicrotaskScheduler implements EffectScheduler {
private hasQueuedFlush = false;
private delegate = new ZoneAwareQueueingScheduler();
private flushTask = () => {
// Leave `hasQueuedFlush` as `true` so we don't queue another microtask if more effects are
// scheduled during flushing. The flush of the `ZoneAwareQueueingScheduler` delegate is
// guaranteed to empty the queue.
this.delegate.flush();
this.hasQueuedFlush = false;

// This is a variable initialization, not a method.
// tslint:disable-next-line:semicolon
};

scheduleEffect(handle: SchedulableEffect): void {
this.delegate.scheduleEffect(handle);

if (!this.hasQueuedFlush) {
queueMicrotask(this.flushTask);
this.hasQueuedFlush = true;
}
}
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/core/test/render3/global_utils_spec.ts
Expand Up @@ -7,6 +7,7 @@
*/

import {setProfiler} from '@angular/core/src/render3/profiler';

import {applyChanges} from '../../src/render3/util/change_detection_utils';
import {getComponent, getContext, getDirectiveMetadata, getDirectives, getHostElement, getInjector, getListeners, getOwningComponent, getRootComponents} from '../../src/render3/util/discovery_utils';
import {GLOBAL_PUBLISH_EXPANDO_KEY, GlobalDevModeContainer, publishDefaultGlobalUtils, publishGlobalUtil} from '../../src/render3/util/global_utils';
Expand Down
28 changes: 27 additions & 1 deletion packages/core/test/test_bed_effect_spec.ts
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Component, effect, inject, Injector} from '@angular/core';
import {Component, effect, inject, Injector, NgZone, signal} from '@angular/core';
import {TestBed} from '@angular/core/testing';

describe('effects in TestBed', () => {
Expand Down Expand Up @@ -85,4 +85,30 @@ describe('effects in TestBed', () => {
'Effect',
]);
});

it('will flush effects automatically when using autoDetectChanges', async () => {
const val = signal('initial');
let observed = '';
@Component({
selector: 'test-cmp',
standalone: true,
template: '',
})
class Cmp {
constructor() {
effect(() => {
observed = val();
});
}
}

const fixture = TestBed.createComponent(Cmp);
fixture.autoDetectChanges();

expect(observed).toBe('initial');
val.set('new');
expect(observed).toBe('initial');
await fixture.whenStable();
expect(observed).toBe('new');
});
});
8 changes: 4 additions & 4 deletions packages/core/testing/src/component_fixture.ts
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

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';
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';
import {Subscription} from 'rxjs';

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

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/core/testing/src/test_bed.ts
Expand Up @@ -27,6 +27,7 @@ import {
Type,
ɵconvertToBitFlags as convertToBitFlags,
ɵDeferBlockBehavior as DeferBlockBehavior,
ɵEffectScheduler as EffectScheduler,
ɵflushModuleScopingQueueAsMuchAsPossible as flushModuleScopingQueueAsMuchAsPossible,
ɵgetAsyncClassMetadataFn as getAsyncClassMetadataFn,
ɵgetUnknownElementStrictMode as getUnknownElementStrictMode,
Expand All @@ -38,7 +39,6 @@ import {
ɵsetUnknownElementStrictMode as setUnknownElementStrictMode,
ɵsetUnknownPropertyStrictMode as setUnknownPropertyStrictMode,
ɵstringify as stringify,
ɵZoneAwareQueueingScheduler as ZoneAwareQueueingScheduler,
} from '@angular/core';

/* clang-format on */
Expand Down Expand Up @@ -782,7 +782,7 @@ export class TestBedImpl implements TestBed {
* @developerPreview
*/
flushEffects(): void {
this.inject(ZoneAwareQueueingScheduler).flush();
this.inject(EffectScheduler).flush();
}
}

Expand Down
Expand Up @@ -7,6 +7,7 @@
*/

import {browser, by, element} from 'protractor';

import {verifyNoBrowserErrors} from '../../../../../test-utils';

// Declare the global "window" and "document" constant since we don't want to add the "dom"
Expand Down
4 changes: 1 addition & 3 deletions packages/platform-browser/testing/src/browser.ts
Expand Up @@ -7,7 +7,7 @@
*/
import {PlatformLocation} from '@angular/common';
import {MockPlatformLocation} from '@angular/common/testing';
import {APP_ID, createPlatformFactory, NgModule, PLATFORM_INITIALIZER, platformCore, provideZoneChangeDetection, StaticProvider, ɵEffectScheduler as EffectScheduler, ɵZoneAwareQueueingScheduler as ZoneAwareQueueingScheduler} from '@angular/core';
import {APP_ID, createPlatformFactory, NgModule, PLATFORM_INITIALIZER, platformCore, provideZoneChangeDetection, StaticProvider} from '@angular/core';
import {BrowserModule, ɵBrowserDomAdapter as BrowserDomAdapter} from '@angular/platform-browser';

function initBrowserTests() {
Expand Down Expand Up @@ -36,8 +36,6 @@ export const platformBrowserTesting =
{provide: APP_ID, useValue: 'a'},
provideZoneChangeDetection(),
{provide: PlatformLocation, useClass: MockPlatformLocation},
{provide: ZoneAwareQueueingScheduler},
{provide: EffectScheduler, useExisting: ZoneAwareQueueingScheduler},
]
})
export class BrowserTestingModule {
Expand Down