From 06be8034bb9b9adfc07ab0d40cd87c6ae5de02de Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Wed, 3 Dec 2025 16:32:18 -0800 Subject: [PATCH] fix(core): Microtask scheduling should be used after any application synchronization Previously, Angular would switch from the macrotask to a microtask scheduler _only_ when the scheduler was the trigger for the synchronization. This microtask scheduling is to ensure patterns such as `Promise.resolve().then(() => updateAppStateAgain())` _during_ synchronization are caught and synchronized again within the same event loop (guaranteeing that they aren't split across multiple browser paints). The microtask scheduler should be used after any tick, not just from those than run within the scheduler to always account for the promises within synchronization. This is encountered most frequently during bootstrap, which triggers the tick directly. In this change we exempt `TestBed.tick` and `ComponentFixture.detectChanges` from this behavior. Doing so would affect the timing of stability and tests are quite sensitive to this (e.g. `fixture.whenStable`). It is somewhat unfortunate that we have "special" test-only behavior. However, it is important to acknowledge that this only affects the test-only APIs as well. Any code in the application under test that triggers `ApplicationRef.tick` directly would still use the microtask scheduling behavior. fixes #65444 --- .../core/src/application/application_ref.ts | 3 +- .../scheduling/zoneless_scheduling_impl.ts | 44 ++++++++++++++----- .../test/acceptance/pending_tasks_spec.ts | 3 ++ packages/core/test/render3/reactivity_spec.ts | 1 + packages/core/testing/src/test_bed.ts | 2 + 5 files changed, 41 insertions(+), 12 deletions(-) diff --git a/packages/core/src/application/application_ref.ts b/packages/core/src/application/application_ref.ts index 6d278597c7a..b52c6d7b33c 100644 --- a/packages/core/src/application/application_ref.ts +++ b/packages/core/src/application/application_ref.ts @@ -293,7 +293,8 @@ export class ApplicationRef { // Eventually the hostView of the fixture should just attach to ApplicationRef. private allTestViews: Set> = new Set(); private autoDetectTestViews: Set> = new Set(); - private includeAllTestViews = false; + /** @internal */ + includeAllTestViews = false; /** @internal */ afterTick = new Subject(); /** @internal */ diff --git a/packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts b/packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts index 4b53e0d5aa3..dcc0de84113 100644 --- a/packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts +++ b/packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts @@ -82,12 +82,28 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler { constructor() { this.subscriptions.add( this.appRef.afterTick.subscribe(() => { + // Prevent stabilization if cleanup causes the last task to be removed + // before we can switch to the microtask scheduler. + const task = this.taskService.add(); // If the scheduler isn't running a tick but the application ticked, that means // someone called ApplicationRef.tick manually. In this case, we should cancel // any change detections that had been scheduled so we don't run an extra one. if (!this.runningTick) { this.cleanup(); + // Ticks that happen when ZoneJS is present do not get the microtask scheduling treatment. + // ZoneJS is responsible for rerunning change detection on microtask queue empty. + // Ticks initiated from tests also do not get microtask treatment so those ticks + // do not affect stability timing, which tests are quite sensitive to. + // TODO(atscott): we really should not use microtask scheduler + // _ever_ when ZoneJS is enabled because ZoneJS is responsible for rerunning change + // detection on microtask queue empty. This change breaks some tests + if (!this.zonelessEnabled || this.appRef.includeAllTestViews) { + this.taskService.remove(task); + return; + } } + this.switchToMicrotaskScheduler(); + this.taskService.remove(task); }), ); this.subscriptions.add( @@ -102,6 +118,22 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler { ); } + // If we're notified of a change within 1 microtask of running change + // detection, run another round in the same event loop. This allows code + // which uses Promise.resolve (see NgModel) to avoid + // ExpressionChanged...Error to still be reflected in a single browser + // paint, even if that spans multiple rounds of change detection. + private switchToMicrotaskScheduler(): void { + this.ngZone.runOutsideAngular(() => { + const task = this.taskService.add(); + this.useMicrotaskScheduler = true; + queueMicrotask(() => { + this.useMicrotaskScheduler = false; + this.taskService.remove(task); + }); + }); + } + notify(source: NotificationSource): void { if (!this.zonelessEnabled && source === NotificationSource.Listener) { // When the notification comes from a listener, we skip the notification unless the @@ -264,21 +296,11 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler { this.schedulerTickApplyArgs, ); } catch (e: unknown) { - this.taskService.remove(task); this.applicationErrorHandler(e); } finally { + this.taskService.remove(task); this.cleanup(); } - // If we're notified of a change within 1 microtask of running change - // detection, run another round in the same event loop. This allows code - // which uses Promise.resolve (see NgModel) to avoid - // ExpressionChanged...Error to still be reflected in a single browser - // paint, even if that spans multiple rounds of change detection. - this.useMicrotaskScheduler = true; - scheduleCallbackWithMicrotask(() => { - this.useMicrotaskScheduler = false; - this.taskService.remove(task); - }); } ngOnDestroy() { diff --git a/packages/core/test/acceptance/pending_tasks_spec.ts b/packages/core/test/acceptance/pending_tasks_spec.ts index f706645cc44..be6220f5ce8 100644 --- a/packages/core/test/acceptance/pending_tasks_spec.ts +++ b/packages/core/test/acceptance/pending_tasks_spec.ts @@ -78,6 +78,9 @@ describe('public PendingTasks', () => { // stability is delayed until a tick happens await expectAsync(applicationRefIsStable(appRef)).toBeResolvedTo(false); TestBed.inject(ApplicationRef).tick(); + // Stability is not synchronous after a tick. We wait for a microtask + // in case there is a Promise inside tick that requires tick again + await Promise.resolve(); await expectAsync(applicationRefIsStable(appRef)).toBeResolvedTo(true); }); diff --git a/packages/core/test/render3/reactivity_spec.ts b/packages/core/test/render3/reactivity_spec.ts index 3de8867ec9b..062fdcd3207 100644 --- a/packages/core/test/render3/reactivity_spec.ts +++ b/packages/core/test/render3/reactivity_spec.ts @@ -91,6 +91,7 @@ describe('reactivity', () => { expect(isStable).toEqual([true, false]); appRef.tick(); + await appRef.whenStable(); expect(isStable).toEqual([true, false, true]); }); diff --git a/packages/core/testing/src/test_bed.ts b/packages/core/testing/src/test_bed.ts index 582fa52dda3..14e67c8de61 100644 --- a/packages/core/testing/src/test_bed.ts +++ b/packages/core/testing/src/test_bed.ts @@ -880,6 +880,8 @@ export class TestBedImpl implements TestBed { // The behavior should be that TestBed.tick, ComponentFixture.detectChanges, and ApplicationRef.tick all result in the test fixtures // getting synchronized, regardless of whether they are autoDetect: true. // Automatic scheduling (zone or zoneless) will call _tick which will _not_ include fixtures with autoDetect: false + // If this does get changed, we will need a new flag for the scheduler to use to omit the microtask scheduling + // from a tick initiated by tests. (appRef as any).includeAllTestViews = true; appRef.tick(); } finally {