Skip to content

Patch branch version of #57453 #57504

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.

Sign up for GitHub

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

Closed
alxhub wants to merge 2 commits into from
Closed

Patch branch version of #57453 #57504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
642f783
refactor(core): track dirtiness bits in `ApplicationRef` (#57453)
alxhub Jul 10, 2024
1a58b45
refactor(core): restructure `AfterRenderManager` to connect related p…
alxhub Aug 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
196 changes: 152 additions & 44 deletions packages/core/src/application/application_ref.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ import {NgModuleRef} from '../linker/ng_module_factory';
import {ViewRef} from '../linker/view_ref';
import {PendingTasks} from '../pending_tasks';
import {RendererFactory2} from '../render/api';
import {AfterRenderEventManager} from '../render3/after_render_hooks';
import {AfterRenderManager} from '../render3/after_render/manager';
import {ComponentFactory as R3ComponentFactory} from '../render3/component_ref';
import {isStandalone} from '../render3/definition';
import {ChangeDetectionMode, detectChangesInternal} from '../render3/instructions/change_detection';
import {FLAGS, LView, LViewFlags} from '../render3/interfaces/view';
import {publishDefaultGlobalUtils as _publishDefaultGlobalUtils} from '../render3/util/global_utils';
import {requiresRefreshOrTraversal} from '../render3/util/view_utils';
import {removeLViewOnDestroy, requiresRefreshOrTraversal} from '../render3/util/view_utils';
import {ViewRef as InternalViewRef} from '../render3/view_ref';
import {TESTABILITY} from '../testability/testability';
import {isPromise} from '../util/lang';
Expand Down Expand Up @@ -308,9 +308,26 @@ export class ApplicationRef {
/** @internal */
_views: InternalViewRef<unknown>[] = [];
private readonly internalErrorHandler = inject(INTERNAL_APPLICATION_ERROR_HANDLER);
private readonly afterRenderEffectManager = inject(AfterRenderEventManager);
private readonly afterRenderManager = inject(AfterRenderManager);
private readonly zonelessEnabled = inject(ZONELESS_ENABLED);

/**
* Current dirty state of the application across a number of dimensions (views, afterRender hooks,
* etc).
*
* A flag set here means that `tick()` will attempt to resolve the dirtiness when executed.
*
* @internal
*/
dirtyFlags = ApplicationRefDirtyFlags.None;

/**
* Like `dirtyFlags` but don't cause `tick()` to loop.
*
* @internal
*/
deferredDirtyFlags = ApplicationRefDirtyFlags.None;

// Needed for ComponentFixture temporarily during migration of autoDetect behavior
// Eventually the hostView of the fixture should just attach to ApplicationRef.
private externalTestViews: Set<InternalViewRef<unknown>> = new Set();
Expand Down Expand Up @@ -557,11 +574,14 @@ export class ApplicationRef {
* detection pass during which all change detection must complete.
*/
tick(): void {
this._tick(true);
if (!this.zonelessEnabled) {
this.dirtyFlags |= ApplicationRefDirtyFlags.ViewTreeGlobal;
}
this._tick();
}

/** @internal */
_tick(refreshViews: boolean): void {
_tick(): void {
(typeof ngDevMode === 'undefined' || ngDevMode) && this.warnIfDestroyed();
if (this._runningTick) {
throw new RuntimeError(
Expand All @@ -573,8 +593,7 @@ export class ApplicationRef {
const prevConsumer = setActiveConsumer(null);
try {
this._runningTick = true;

this.detectChangesInAttachedViews(refreshViews);
this.synchronize();

if (typeof ngDevMode === 'undefined' || ngDevMode) {
for (let view of this._views) {
Expand All @@ -591,49 +610,23 @@ export class ApplicationRef {
}
}

private detectChangesInAttachedViews(refreshViews: boolean) {
/**
* Performs the core work of synchronizing the application state with the UI, resolving any
* pending dirtiness (potentially in a loop).
*/
private synchronize(): void {
let rendererFactory: RendererFactory2 | null = null;
if (!(this._injector as R3Injector).destroyed) {
rendererFactory = this._injector.get(RendererFactory2, null, {optional: true});
}

let runs = 0;
const afterRenderEffectManager = this.afterRenderEffectManager;
while (runs < MAXIMUM_REFRESH_RERUNS) {
const isFirstPass = runs === 0;
// Some notifications to run a `tick` will only trigger render hooks. so we skip refreshing views the first time through.
// After the we execute render hooks in the first pass, we loop while views are marked dirty and should refresh them.
if (refreshViews || !isFirstPass) {
this.beforeRender.next(isFirstPass);
for (let {_lView, notifyErrorHandler} of this._views) {
detectChangesInViewIfRequired(
_lView,
notifyErrorHandler,
isFirstPass,
this.zonelessEnabled,
);
}
} else {
// If we skipped refreshing views above, there might still be unflushed animations
// because we never called `detectChangesInternal` on the views.
rendererFactory?.begin?.();
rendererFactory?.end?.();
}
runs++;

afterRenderEffectManager.executeInternalCallbacks();
// If we have a newly dirty view after running internal callbacks, recheck the views again
// before running user-provided callbacks
if (this.allViews.some(({_lView}) => requiresRefreshOrTraversal(_lView))) {
continue;
}
// When beginning synchronization, all deferred dirtiness becomes active dirtiness.
this.dirtyFlags |= this.deferredDirtyFlags;
this.deferredDirtyFlags = ApplicationRefDirtyFlags.None;

afterRenderEffectManager.execute();
// If after running all afterRender callbacks we have no more views that need to be refreshed,
// we can break out of the loop
if (!this.allViews.some(({_lView}) => requiresRefreshOrTraversal(_lView))) {
break;
}
let runs = 0;
while (this.dirtyFlags !== ApplicationRefDirtyFlags.None && runs++ < MAXIMUM_REFRESH_RERUNS) {
this.synchronizeOnce(rendererFactory);
}

if ((typeof ngDevMode === 'undefined' || ngDevMode) && runs >= MAXIMUM_REFRESH_RERUNS) {
Expand All @@ -647,6 +640,92 @@ export class ApplicationRef {
}
}

/**
* Perform a single synchronization pass.
*/
private synchronizeOnce(rendererFactory: RendererFactory2 | null): void {
// If we happened to loop, deferred dirtiness can be processed as active dirtiness again.
this.dirtyFlags |= this.deferredDirtyFlags;
this.deferredDirtyFlags = ApplicationRefDirtyFlags.None;

// First check dirty views, if there are any.
if (this.dirtyFlags & ApplicationRefDirtyFlags.ViewTreeAny) {
// Change detection on views starts in targeted mode (only check components if they're
// marked as dirty) unless global checking is specifically requested via APIs like
// `ApplicationRef.tick()` and the `NgZone` integration.
const useGlobalCheck = Boolean(this.dirtyFlags & ApplicationRefDirtyFlags.ViewTreeGlobal);

// Clear the view-related dirty flags.
this.dirtyFlags &= ~ApplicationRefDirtyFlags.ViewTreeAny;

// Set the AfterRender bit, as we're checking views and will need to run afterRender hooks.
this.dirtyFlags |= ApplicationRefDirtyFlags.AfterRender;

// Check all potentially dirty views.
this.beforeRender.next(useGlobalCheck);
for (let {_lView, notifyErrorHandler} of this._views) {
detectChangesInViewIfRequired(
_lView,
notifyErrorHandler,
useGlobalCheck,
this.zonelessEnabled,
);
}

// If `markForCheck()` was called during view checking, it will have set the `ViewTreeCheck`
// flag. We clear the flag here because, for backwards compatibility, `markForCheck()`
// during view checking doesn't cause the view to be re-checked.
this.dirtyFlags &= ~ApplicationRefDirtyFlags.ViewTreeCheck;

// Check if any views are still dirty after checking and we need to loop back.
this.syncDirtyFlagsWithViews();
if (this.dirtyFlags & ApplicationRefDirtyFlags.ViewTreeAny) {
// If any views are still dirty after checking, loop back before running render hooks.
return;
}
} else {
// If we skipped refreshing views above, there might still be unflushed animations
// because we never called `detectChangesInternal` on the views.
rendererFactory?.begin?.();
rendererFactory?.end?.();
}

// Even if there were no dirty views, afterRender hooks might still be dirty.
if (this.dirtyFlags & ApplicationRefDirtyFlags.AfterRender) {
this.dirtyFlags &= ~ApplicationRefDirtyFlags.AfterRender;
this.afterRenderManager.execute();

// afterRender hooks might influence dirty flags.
}
this.syncDirtyFlagsWithViews();
}

/**
* Checks `allViews` for views which require refresh/traversal, and updates `dirtyFlags`
* accordingly, with two potential behaviors:
*
* 1. If any of our views require updating, then this adds the `ViewTreeTraversal` dirty flag.
* This _should_ be a no-op, since the scheduler should've added the flag at the same time the
* view was marked as needing updating.
*
* TODO(alxhub): figure out if this behavior is still needed for edge cases.
*
* 2. If none of our views require updating, then clear the view-related `dirtyFlag`s. This
* happens when the scheduler is notified of a view becoming dirty, but the view itself isn't
* reachable through traversal from our roots (e.g. it's detached from the CD tree).
*/
private syncDirtyFlagsWithViews(): void {
if (this.allViews.some(({_lView}) => requiresRefreshOrTraversal(_lView))) {
// If after running all afterRender callbacks new views are dirty, ensure we loop back.
this.dirtyFlags |= ApplicationRefDirtyFlags.ViewTreeTraversal;
return;
} else {
// Even though this flag may be set, none of _our_ views require traversal, and so the
// `ApplicationRef` doesn't require any repeated checking.
this.dirtyFlags &= ~ApplicationRefDirtyFlags.ViewTreeAny;
}
}

/**
* Attaches a view so that it will be dirty checked.
* The view will be automatically detached when it is destroyed.
Expand Down Expand Up @@ -769,6 +848,35 @@ export function remove<T>(list: T[], el: T): void {
}
}

export const enum ApplicationRefDirtyFlags {
None = 0,

/**
* A global change detection round has been requested.
*/
ViewTreeGlobal = 0b00000001,

/**
* Part of the view tree is marked for traversal.
*/
ViewTreeTraversal = 0b00000010,

/**
* Part of the view tree is marked to be checked (dirty).
*/
ViewTreeCheck = 0b00000100,

/**
* Helper for any view tree bit being set.
*/
ViewTreeAny = ViewTreeGlobal | ViewTreeTraversal | ViewTreeCheck,

/**
* After render hooks need to run.
*/
AfterRender = 0b00001000,
}

let whenStableStore: WeakMap<ApplicationRef, Promise<void>> | undefined;
/**
* Returns a Promise that resolves when the application becomes stable after this method is called
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/change_detection/scheduling/zoneless_scheduling.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ export const enum NotificationSource {
// The following notifications do not require views to be refreshed
// but we should execute render hooks:
// Render hooks are guaranteed to execute with the schedulers timing.
NewRenderHook,
RenderHook,
DeferredRenderHook,
// Views might be created outside and manipulated in ways that
// we cannot be aware of. When a view is attached, Angular now "knows"
// about it and we now know that DOM might have changed (and we should
Expand Down
47 changes: 35 additions & 12 deletions packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import {Subscription} from 'rxjs';

import {ApplicationRef} from '../../application/application_ref';
import {ApplicationRef, ApplicationRefDirtyFlags} from '../../application/application_ref';
import {Injectable} from '../../di/injectable';
import {inject} from '../../di/injector_compatibility';
import {EnvironmentProviders} from '../../di/interface/provider';
Expand Down Expand Up @@ -74,7 +74,6 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler {
(inject(SCHEDULE_IN_ROOT_ZONE, {optional: true}) ?? false);

private cancelScheduledCallback: null | (() => void) = null;
private shouldRefreshViews = false;
private useMicrotaskScheduler = false;
runningTick = false;
pendingRenderTaskId: number | null = null;
Expand Down Expand Up @@ -124,23 +123,34 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler {
return;
}
switch (source) {
case NotificationSource.MarkAncestorsForTraversal: {
this.appRef.dirtyFlags |= ApplicationRefDirtyFlags.ViewTreeTraversal;
break;
}
case NotificationSource.DebugApplyChanges:
case NotificationSource.DeferBlockStateUpdate:
case NotificationSource.MarkAncestorsForTraversal:
case NotificationSource.MarkForCheck:
case NotificationSource.Listener:
case NotificationSource.SetInput: {
this.shouldRefreshViews = true;
this.appRef.dirtyFlags |= ApplicationRefDirtyFlags.ViewTreeCheck;
break;
}
case NotificationSource.DeferredRenderHook: {
// Render hooks are "deferred" when they're triggered from other render hooks. Using the
// deferred dirty flags ensures that adding new hooks doesn't automatically trigger a loop
// inside tick().
this.appRef.deferredDirtyFlags |= ApplicationRefDirtyFlags.AfterRender;
break;
}
case NotificationSource.ViewDetachedFromDOM:
case NotificationSource.ViewAttached:
case NotificationSource.NewRenderHook:
case NotificationSource.RenderHook:
case NotificationSource.AsyncAnimationsLoaded:
default: {
// These notifications only schedule a tick but do not change whether we should refresh
// views. Instead, we only need to run render hooks unless another notification from the
// other set is also received before `tick` happens.
this.appRef.dirtyFlags |= ApplicationRefDirtyFlags.AfterRender;
}
}

Expand All @@ -162,12 +172,10 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler {
: scheduleCallbackWithRafRace;
this.pendingRenderTaskId = this.taskService.add();
if (this.scheduleInRootZone) {
this.cancelScheduledCallback = Zone.root.run(() =>
scheduleCallback(() => this.tick(this.shouldRefreshViews)),
);
this.cancelScheduledCallback = Zone.root.run(() => scheduleCallback(() => this.tick()));
} else {
this.cancelScheduledCallback = this.ngZone.runOutsideAngular(() =>
scheduleCallback(() => this.tick(this.shouldRefreshViews)),
scheduleCallback(() => this.tick()),
);
}
}
Expand Down Expand Up @@ -202,20 +210,36 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler {
* @param shouldRefreshViews Passed directly to `ApplicationRef._tick` and skips straight to
* render hooks when `false`.
*/
private tick(shouldRefreshViews: boolean): void {
private tick(): void {
// When ngZone.run below exits, onMicrotaskEmpty may emit if the zone is
// stable. We want to prevent double ticking so we track whether the tick is
// already running and skip it if so.
if (this.runningTick || this.appRef.destroyed) {
return;
}

// The scheduler used to pass "whether to check views" as a boolean flag instead of setting
// fine-grained dirtiness flags, and global checking was always used on the first pass. This
// created an interesting edge case: if a notification made a view dirty and then ticked via the
// scheduler (and not the zone) a global check was still performed.
//
// Ideally, this would not be the case, and only zone-based ticks would do global passes.
// However this is a breaking change and requires fixes in g3. Until this cleanup can be done,
// we add the `ViewTreeGlobal` flag to request a global check if any views are dirty in a
// scheduled tick (unless zoneless is enabled, in which case global checks aren't really a
// thing).
//
// TODO(alxhub): clean up and remove this workaround as a breaking change.
if (!this.zonelessEnabled && this.appRef.dirtyFlags & ApplicationRefDirtyFlags.ViewTreeAny) {
this.appRef.dirtyFlags |= ApplicationRefDirtyFlags.ViewTreeGlobal;
}

const task = this.taskService.add();
try {
this.ngZone.run(
() => {
this.runningTick = true;
this.appRef._tick(shouldRefreshViews);
this.appRef._tick();
},
undefined,
this.schedulerTickApplyArgs,
Expand Down Expand Up @@ -244,7 +268,6 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler {
}

private cleanup() {
this.shouldRefreshViews = false;
this.runningTick = false;
this.cancelScheduledCallback?.();
this.cancelScheduledCallback = null;
Expand Down
Loading