Skip to content

Commit 9de30a7

Browse files
atscottdylhunn
atscott
authored and
dylhunn
committed
fix(core): Allow zoneless scheduler to run inside fakeAsync (#56932)
The zoneless scheduler callback was executed in the root zone rather than simply in `runOutsideAngular` to allow us to land the hybrid mode change detection (scheduler always enabled, even for zones) without breaking a ton of existing `fakeAsync` tests that could/would fail with the "timer(s) still in queue" error. However, this caused another problem: when a test executes inside `fakeAsync`, it cannot flush the scheduled time. A similar problem exists with event and run coalescing (#56767). This change would allow `fakeAsync` to flush the zoneless-scheduled change detections and minimize breaking existing tests by flushing pending timers at the end of the test, which actually now matches what's done internally. PR Close #56932
1 parent b1ed7e2 commit 9de30a7

File tree

20 files changed

+233
-31
lines changed

20 files changed

+233
-31
lines changed

‎goldens/public-api/core/index.api.md‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1223,10 +1223,10 @@ export class NgProbeToken {
12231223

12241224
// @public
12251225
export class NgZone {
1226-
constructor({ enableLongStackTrace, shouldCoalesceEventChangeDetection, shouldCoalesceRunChangeDetection, }: {
1227-
enableLongStackTrace?: boolean | undefined;
1228-
shouldCoalesceEventChangeDetection?: boolean | undefined;
1229-
shouldCoalesceRunChangeDetection?: boolean | undefined;
1226+
constructor(options: {
1227+
enableLongStackTrace?: boolean;
1228+
shouldCoalesceEventChangeDetection?: boolean;
1229+
shouldCoalesceRunChangeDetection?: boolean;
12301230
});
12311231
static assertInAngularZone(): void;
12321232
static assertNotInAngularZone(): void;

‎packages/core/src/change_detection/scheduling/flags.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
export const SCHEDULE_IN_ROOT_ZONE_DEFAULT = true;

‎packages/core/src/change_detection/scheduling/ng_zone_scheduling.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ import {
2828
ChangeDetectionScheduler,
2929
ZONELESS_SCHEDULER_DISABLED,
3030
ZONELESS_ENABLED,
31+
SCHEDULE_IN_ROOT_ZONE,
3132
} from './zoneless_scheduling';
33+
import {SCHEDULE_IN_ROOT_ZONE_DEFAULT} from './flags';
3234

3335
@Injectable({providedIn: 'root'})
3436
export class NgZoneChangeDetectionScheduler {
@@ -75,11 +77,14 @@ export const PROVIDED_NG_ZONE = new InjectionToken<boolean>(
7577
export function internalProvideZoneChangeDetection({
7678
ngZoneFactory,
7779
ignoreChangesOutsideZone,
80+
scheduleInRootZone,
7881
}: {
7982
ngZoneFactory?: () => NgZone;
8083
ignoreChangesOutsideZone?: boolean;
84+
scheduleInRootZone?: boolean;
8185
}): StaticProvider[] {
82-
ngZoneFactory ??= () => new NgZone(getNgZoneOptions());
86+
ngZoneFactory ??= () =>
87+
new NgZone({...getNgZoneOptions(), scheduleInRootZone} as InternalNgZoneOptions);
8388
return [
8489
{provide: NgZone, useFactory: ngZoneFactory},
8590
{
@@ -115,6 +120,10 @@ export function internalProvideZoneChangeDetection({
115120
// Always disable scheduler whenever explicitly disabled, even if another place called
116121
// `provideZoneChangeDetection` without the 'ignore' option.
117122
ignoreChangesOutsideZone === true ? {provide: ZONELESS_SCHEDULER_DISABLED, useValue: true} : [],
123+
{
124+
provide: SCHEDULE_IN_ROOT_ZONE,
125+
useValue: scheduleInRootZone ?? SCHEDULE_IN_ROOT_ZONE_DEFAULT,
126+
},
118127
];
119128
}
120129

@@ -140,15 +149,18 @@ export function internalProvideZoneChangeDetection({
140149
*/
141150
export function provideZoneChangeDetection(options?: NgZoneOptions): EnvironmentProviders {
142151
const ignoreChangesOutsideZone = options?.ignoreChangesOutsideZone;
152+
const scheduleInRootZone = (options as any)?.scheduleInRootZone;
143153
const zoneProviders = internalProvideZoneChangeDetection({
144154
ngZoneFactory: () => {
145155
const ngZoneOptions = getNgZoneOptions(options);
156+
ngZoneOptions.scheduleInRootZone = scheduleInRootZone;
146157
if (ngZoneOptions.shouldCoalesceEventChangeDetection) {
147158
performanceMarkFeature('NgZone_CoalesceEvent');
148159
}
149160
return new NgZone(ngZoneOptions);
150161
},
151162
ignoreChangesOutsideZone,
163+
scheduleInRootZone,
152164
});
153165
return makeEnvironmentProviders([
154166
{provide: PROVIDED_NG_ZONE, useValue: true},

‎packages/core/src/change_detection/scheduling/zoneless_scheduling.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,3 +68,8 @@ export const PROVIDED_ZONELESS = new InjectionToken<boolean>(
6868
export const ZONELESS_SCHEDULER_DISABLED = new InjectionToken<boolean>(
6969
typeof ngDevMode === 'undefined' || ngDevMode ? 'scheduler disabled' : '',
7070
);
71+
72+
// TODO(atscott): Remove in v19. Scheduler should be done with runOutsideAngular.
73+
export const SCHEDULE_IN_ROOT_ZONE = new InjectionToken<boolean>(
74+
typeof ngDevMode === 'undefined' || ngDevMode ? 'run changes outside zone in root' : '',
75+
);

‎packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
ZONELESS_ENABLED,
2929
PROVIDED_ZONELESS,
3030
ZONELESS_SCHEDULER_DISABLED,
31+
SCHEDULE_IN_ROOT_ZONE,
3132
} from './zoneless_scheduling';
3233

3334
const CONSECUTIVE_MICROTASK_NOTIFICATION_LIMIT = 100;
@@ -67,6 +68,10 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler {
6768
private readonly angularZoneId = this.zoneIsDefined
6869
? (this.ngZone as NgZonePrivate)._inner?.get(angularZoneInstanceIdProperty)
6970
: null;
71+
private readonly scheduleInRootZone =
72+
!this.zonelessEnabled &&
73+
this.zoneIsDefined &&
74+
(inject(SCHEDULE_IN_ROOT_ZONE, {optional: true}) ?? false);
7075

7176
private cancelScheduledCallback: null | (() => void) = null;
7277
private shouldRefreshViews = false;
@@ -156,16 +161,14 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler {
156161
? scheduleCallbackWithMicrotask
157162
: scheduleCallbackWithRafRace;
158163
this.pendingRenderTaskId = this.taskService.add();
159-
if (this.zoneIsDefined) {
160-
Zone.root.run(() => {
161-
this.cancelScheduledCallback = scheduleCallback(() => {
162-
this.tick(this.shouldRefreshViews);
163-
});
164-
});
164+
if (this.scheduleInRootZone) {
165+
this.cancelScheduledCallback = Zone.root.run(() =>
166+
scheduleCallback(() => this.tick(this.shouldRefreshViews)),
167+
);
165168
} else {
166-
this.cancelScheduledCallback = scheduleCallback(() => {
167-
this.tick(this.shouldRefreshViews);
168-
});
169+
this.cancelScheduledCallback = this.ngZone.runOutsideAngular(() =>
170+
scheduleCallback(() => this.tick(this.shouldRefreshViews)),
171+
);
169172
}
170173
}
171174

@@ -316,6 +319,7 @@ export function provideExperimentalZonelessChangeDetection(): EnvironmentProvide
316319
{provide: ChangeDetectionScheduler, useExisting: ChangeDetectionSchedulerImpl},
317320
{provide: NgZone, useClass: NoopNgZone},
318321
{provide: ZONELESS_ENABLED, useValue: true},
322+
{provide: SCHEDULE_IN_ROOT_ZONE, useValue: false},
319323
typeof ngDevMode === 'undefined' || ngDevMode
320324
? [{provide: PROVIDED_ZONELESS, useValue: true}]
321325
: [],

‎packages/core/src/platform/platform_ref.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,15 @@ export class PlatformRef {
5656
moduleFactory: NgModuleFactory<M>,
5757
options?: BootstrapOptions,
5858
): Promise<NgModuleRef<M>> {
59+
const scheduleInRootZone = (options as any)?.scheduleInRootZone;
5960
const ngZoneFactory = () =>
60-
getNgZone(
61-
options?.ngZone,
62-
getNgZoneOptions({
61+
getNgZone(options?.ngZone, {
62+
...getNgZoneOptions({
6363
eventCoalescing: options?.ngZoneEventCoalescing,
6464
runCoalescing: options?.ngZoneRunCoalescing,
6565
}),
66-
);
66+
scheduleInRootZone,
67+
});
6768
const ignoreChangesOutsideZone = options?.ignoreChangesOutsideZone;
6869
const allAppProviders = [
6970
internalProvideZoneChangeDetection({

‎packages/core/src/zone/ng_zone.ts‎

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

9+
import {SCHEDULE_IN_ROOT_ZONE_DEFAULT} from '../change_detection/scheduling/flags';
910
import {RuntimeError, RuntimeErrorCode} from '../errors';
1011
import {EventEmitter} from '../event_emitter';
1112
import {scheduleCallbackWithRafRace} from '../util/callback_scheduler';
12-
import {global} from '../util/global';
1313
import {noop} from '../util/noop';
1414

1515
import {AsyncStackTaggingZoneSpec} from './async-stack-tagging';
@@ -130,11 +130,18 @@ export class NgZone {
130130
*/
131131
readonly onError: EventEmitter<any> = new EventEmitter(false);
132132

133-
constructor({
134-
enableLongStackTrace = false,
135-
shouldCoalesceEventChangeDetection = false,
136-
shouldCoalesceRunChangeDetection = false,
133+
constructor(options: {
134+
enableLongStackTrace?: boolean;
135+
shouldCoalesceEventChangeDetection?: boolean;
136+
shouldCoalesceRunChangeDetection?: boolean;
137137
}) {
138+
const {
139+
enableLongStackTrace = false,
140+
shouldCoalesceEventChangeDetection = false,
141+
shouldCoalesceRunChangeDetection = false,
142+
scheduleInRootZone = SCHEDULE_IN_ROOT_ZONE_DEFAULT,
143+
} = options as InternalNgZoneOptions;
144+
138145
if (typeof Zone == 'undefined') {
139146
throw new RuntimeError(
140147
RuntimeErrorCode.MISSING_ZONEJS,
@@ -170,6 +177,7 @@ export class NgZone {
170177
!shouldCoalesceRunChangeDetection && shouldCoalesceEventChangeDetection;
171178
self.shouldCoalesceRunChangeDetection = shouldCoalesceRunChangeDetection;
172179
self.callbackScheduled = false;
180+
self.scheduleInRootZone = scheduleInRootZone;
173181
forkInnerZoneWithAngularBehavior(self);
174182
}
175183

@@ -330,6 +338,11 @@ export interface NgZonePrivate extends NgZone {
330338
*
331339
*/
332340
shouldCoalesceRunChangeDetection: boolean;
341+
342+
/**
343+
* Whether to schedule the coalesced change detection in the root zone
344+
*/
345+
scheduleInRootZone: boolean;
333346
}
334347

335348
function checkStable(zone: NgZonePrivate) {
@@ -383,15 +396,24 @@ function delayChangeDetectionForEvents(zone: NgZonePrivate) {
383396
return;
384397
}
385398
zone.callbackScheduled = true;
386-
Zone.root.run(() => {
399+
function scheduleCheckStable() {
387400
scheduleCallbackWithRafRace(() => {
388401
zone.callbackScheduled = false;
389402
updateMicroTaskStatus(zone);
390403
zone.isCheckStableRunning = true;
391404
checkStable(zone);
392405
zone.isCheckStableRunning = false;
393406
});
394-
});
407+
}
408+
if (zone.scheduleInRootZone) {
409+
Zone.root.run(() => {
410+
scheduleCheckStable();
411+
});
412+
} else {
413+
zone._outer.run(() => {
414+
scheduleCheckStable();
415+
});
416+
}
395417
updateMicroTaskStatus(zone);
396418
}
397419

@@ -574,9 +596,10 @@ function hasApplyArgsData(applyArgs: unknown, key: string) {
574596

575597
// Set of options recognized by the NgZone.
576598
export interface InternalNgZoneOptions {
577-
enableLongStackTrace: boolean;
578-
shouldCoalesceEventChangeDetection: boolean;
579-
shouldCoalesceRunChangeDetection: boolean;
599+
enableLongStackTrace?: boolean;
600+
shouldCoalesceEventChangeDetection?: boolean;
601+
shouldCoalesceRunChangeDetection?: boolean;
602+
scheduleInRootZone?: boolean;
580603
}
581604

582605
export function getNgZone(

‎packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,9 @@
458458
{
459459
"name": "RuntimeError"
460460
},
461+
{
462+
"name": "SCHEDULE_IN_ROOT_ZONE"
463+
},
461464
{
462465
"name": "SELF_TOKEN_REGEX"
463466
},

‎packages/core/test/bundling/animations/bundle.golden_symbols.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,9 @@
497497
{
498498
"name": "RuntimeError"
499499
},
500+
{
501+
"name": "SCHEDULE_IN_ROOT_ZONE"
502+
},
500503
{
501504
"name": "SELF_TOKEN_REGEX"
502505
},

‎packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,9 @@
380380
{
381381
"name": "RuntimeError"
382382
},
383+
{
384+
"name": "SCHEDULE_IN_ROOT_ZONE"
385+
},
383386
{
384387
"name": "SIGNAL"
385388
},

0 commit comments

Comments
 (0)