mirror of
https://github.com/angular/angular
synced 2026-05-24 09:28:37 +00:00
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
This commit is contained in:
parent
ae0c59028a
commit
06be8034bb
5 changed files with 41 additions and 12 deletions
|
|
@ -293,7 +293,8 @@ export class ApplicationRef {
|
|||
// Eventually the hostView of the fixture should just attach to ApplicationRef.
|
||||
private allTestViews: Set<InternalViewRef<unknown>> = new Set();
|
||||
private autoDetectTestViews: Set<InternalViewRef<unknown>> = new Set();
|
||||
private includeAllTestViews = false;
|
||||
/** @internal */
|
||||
includeAllTestViews = false;
|
||||
/** @internal */
|
||||
afterTick = new Subject<void>();
|
||||
/** @internal */
|
||||
|
|
|
|||
|
|
@ -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() {
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -91,6 +91,7 @@ describe('reactivity', () => {
|
|||
expect(isStable).toEqual([true, false]);
|
||||
|
||||
appRef.tick();
|
||||
await appRef.whenStable();
|
||||
|
||||
expect(isStable).toEqual([true, false, true]);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Reference in a new issue