Skip to content

Commit 36d8d19

Browse files
atscottAndrewKushnir
authored andcommitted
refactor(core): removing a pending task delays stability until the next tick (angular#57570)
This commit updates the public API for pending tasks to schedule an application tick, effectively making the stability async when the last task is removed. PR Close angular#57570
1 parent 9e83ca8 commit 36d8d19

File tree

4 files changed

+35
-3
lines changed

4 files changed

+35
-3
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ export const enum NotificationSource {
4747
ViewDetachedFromDOM,
4848
// Applying animations might result in new DOM state and should rerun render hooks
4949
AsyncAnimationsLoaded,
50+
// The scheduler is notified when a pending task is removed via the public API.
51+
// This allows us to make stability async, delayed until the next application tick.
52+
PendingTaskRemoved,
5053
}
5154

5255
/**

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,15 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler {
153153
force = true;
154154
break;
155155
}
156+
case NotificationSource.PendingTaskRemoved: {
157+
// Removing a pending task via the public API forces a scheduled tick, ensuring that
158+
// stability is async and delayed until there was at least an opportunity to run
159+
// application synchronization. This prevents some footguns when working with the
160+
// public API for pending tasks where developers attempt to update application state
161+
// immediately after removing the last task.
162+
force = true;
163+
break;
164+
}
156165
case NotificationSource.ViewDetachedFromDOM:
157166
case NotificationSource.ViewAttached:
158167
case NotificationSource.RenderHook:
@@ -229,6 +238,14 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler {
229238
return;
230239
}
231240

241+
// If we reach the tick and there is no work to be done in ApplicationRef.tick,
242+
// skip it altogether and clean up. There may be no work if, for example, the only
243+
// event that notified the scheduler was the removal of a pending task.
244+
if (this.appRef.dirtyFlags === ApplicationRefDirtyFlags.None) {
245+
this.cleanup();
246+
return;
247+
}
248+
232249
// The scheduler used to pass "whether to check views" as a boolean flag instead of setting
233250
// fine-grained dirtiness flags, and global checking was always used on the first pass. This
234251
// created an interesting edge case: if a notification made a view dirty and then ticked via the

packages/core/src/pending_tasks.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ import {BehaviorSubject} from 'rxjs';
1111
import {inject} from './di/injector_compatibility';
1212
import {ɵɵdefineInjectable} from './di/interface/defs';
1313
import {OnDestroy} from './interface/lifecycle_hooks';
14+
import {
15+
ChangeDetectionScheduler,
16+
NotificationSource,
17+
} from './change_detection/scheduling/zoneless_scheduling';
1418

1519
/**
1620
* Internal implementation of the pending tasks service.
@@ -82,13 +86,18 @@ export class PendingTasks implements OnDestroy {
8286
*/
8387
export class ExperimentalPendingTasks {
8488
private internalPendingTasks = inject(PendingTasks);
89+
private scheduler = inject(ChangeDetectionScheduler);
8590
/**
8691
* Adds a new task that should block application's stability.
8792
* @returns A cleanup function that removes a task when called.
8893
*/
8994
add(): () => void {
9095
const taskId = this.internalPendingTasks.add();
91-
return () => this.internalPendingTasks.remove(taskId);
96+
return () => {
97+
// Notifying the scheduler will hold application stability open until the next tick.
98+
this.scheduler.notify(NotificationSource.PendingTaskRemoved);
99+
this.internalPendingTasks.remove(taskId);
100+
};
92101
}
93102

94103
/** @nocollapse */

packages/core/test/acceptance/pending_tasks_spec.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,12 @@ describe('public ExperimentalPendingTasks', () => {
7272
const appRef = TestBed.inject(ApplicationRef);
7373
const pendingTasks = TestBed.inject(ExperimentalPendingTasks);
7474

75-
const taskA = pendingTasks.add();
75+
const removeTaskA = pendingTasks.add();
76+
await expectAsync(applicationRefIsStable(appRef)).toBeResolvedTo(false);
77+
removeTaskA();
78+
// stability is delayed until a tick happens
7679
await expectAsync(applicationRefIsStable(appRef)).toBeResolvedTo(false);
77-
taskA();
80+
TestBed.inject(ApplicationRef).tick();
7881
await expectAsync(applicationRefIsStable(appRef)).toBeResolvedTo(true);
7982
});
8083
});

0 commit comments

Comments
 (0)