Skip to content

Commit

Permalink
refactor(core): private feature for potential zoneless-compatibility …
Browse files Browse the repository at this point in the history
…debug check

This commit adds a feature that might be useful for determining if an
application is zoneless-ready. The way this works is generally only
useful right now when zoneless is enabled. Some version of this may be useful in
the future as a general configuration option to change detection to make
`checkNoChanges` pass always exhaustive as an opt-in to address angular#45612.
  • Loading branch information
atscott committed May 3, 2024
1 parent a0ec2d8 commit 1705e06
Show file tree
Hide file tree
Showing 8 changed files with 328 additions and 19 deletions.
3 changes: 2 additions & 1 deletion packages/core/src/application/application_ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,8 @@ export class ApplicationRef {
private beforeRender = new Subject<boolean>();
/** @internal */
afterTick = new Subject<void>();
private get allViews() {
/** @internal */
get allViews() {
return [...this.externalTestViews.keys(), ...this._views];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,19 @@ import {NgZone} from '../../zone';
import {InternalNgZoneOptions} from '../../zone/ng_zone';

import {alwaysProvideZonelessScheduler} from './flags';
import {ChangeDetectionScheduler, ZONELESS_SCHEDULER_DISABLED} from './zoneless_scheduling';
import {
ChangeDetectionScheduler,
ZONELESS_ENABLED,
ZONELESS_SCHEDULER_DISABLED,
} from './zoneless_scheduling';
import {ChangeDetectionSchedulerImpl} from './zoneless_scheduling_impl';

@Injectable({providedIn: 'root'})
export class NgZoneChangeDetectionScheduler {
private readonly zone = inject(NgZone);
private readonly changeDetectionScheduler = inject(ChangeDetectionScheduler, {optional: true});
private readonly applicationRef = inject(ApplicationRef);
private readonly zonelessEnabled = inject(ZONELESS_ENABLED);

private _onMicrotaskEmptySubscription?: Subscription;

Expand All @@ -47,7 +52,7 @@ export class NgZoneChangeDetectionScheduler {
// `onMicroTaskEmpty` can happen _during_ the zoneless scheduler change detection because
// zone.run(() => {}) will result in `checkStable` at the end of the `zone.run` closure
// and emit `onMicrotaskEmpty` synchronously if run coalsecing is false.
if (this.changeDetectionScheduler?.runningTick) {
if (this.changeDetectionScheduler?.runningTick || this.zonelessEnabled) {
return;
}
this.zone.run(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ import {
ZONELESS_ENABLED,
ZONELESS_SCHEDULER_DISABLED,
} from './zoneless_scheduling';
import {EnvironmentInjector} from '../../di/r3_injector';
import {ENVIRONMENT_INITIALIZER} from '../../di';
import {CheckNoChangesMode} from '../../render3/state';
import {ErrorHandler} from '../../error_handler';

const CONSECUTIVE_MICROTASK_NOTIFICATION_LIMIT = 100;
let consecutiveMicrotaskNotifications = 0;
Expand Down Expand Up @@ -66,9 +70,9 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler {

private cancelScheduledCallback: null | (() => void) = null;
private shouldRefreshViews = false;
private pendingRenderTaskId: number | null = null;
private useMicrotaskScheduler = false;
runningTick = false;
pendingRenderTaskId: number | null = null;

constructor() {
this.subscriptions.add(
Expand Down Expand Up @@ -175,7 +179,7 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler {
}
// If we're inside the zone don't bother with scheduler. Zone will stabilize
// eventually and run change detection.
if (this.zoneIsDefined && NgZone.isInAngularZone()) {
if (!this.zonelessEnabled && this.zoneIsDefined && NgZone.isInAngularZone()) {
return false;
}

Expand Down Expand Up @@ -299,3 +303,99 @@ export function provideExperimentalZonelessChangeDetection(): EnvironmentProvide
{provide: ZONELESS_ENABLED, useValue: true},
]);
}

/**
* For internal use only. Potentially useful for determining if an application is zoneless-compatible.
*
* @param options Used to configure when the 'exhaustive' checkNoChanges will execute.
* - `interval` will periodically run exhaustive `checkNoChanges` on application views
* - `useNgZoneOnStable` will us ZoneJS to determine when change detection might have run
* in an application using ZoneJS to drive change detection. When the `NgZone.onStable` would
* have emit, all views attached to the ApplicationRef are checked for changes.
*
* With both strategies above, if the zoneless scheduler has a change detection scheduled,
* the check will be skipped for that round.
*
* "Exhaustive" means that all views attached to ApplicationRef and all the descendants of those views
* will be checked for changes (excluding those subtrees which are detached via `ChangeDetectorRef.detach()`).
* This may uncover *existing* `ExpressionChangedAfterItHasBeenCheckedError` bugs that are not related
* to zoneless. `ExpressionChangedAfterItHasBeenCheckedError` does not work for components using
* `ChangeDetectionStrategy.OnPush` because the check skips views which are `OnPush` and not marked for check.
* This check is "exhaustive" and will always check all views, regardless of their "dirty" state and `ChangeDetectionStrategy`.
*/
export function provideExhaustiveCheckNoChangesForDebug(options: {
interval?: number;
useNgZoneOnStable?: boolean;
}) {
if (typeof ngDevMode === 'undefined' || ngDevMode) {
if (options.interval === undefined && !options.useNgZoneOnStable) {
throw new Error('Must provide one of `useNgZoneOnStable` or `interval`');
}
return makeEnvironmentProviders([
options?.useNgZoneOnStable ? {provide: NgZone, useClass: DebugNgZoneForCheckNoChanges} : [],
options?.interval !== undefined ? exhaustiveCheckNoChangesInterval(options.interval) : [],
]);
} else {
return makeEnvironmentProviders([]);
}
}

@Injectable({providedIn: 'root'})
export class DebugNgZoneForCheckNoChanges extends NgZone {
applicationRef?: ApplicationRef;
scheduler?: ChangeDetectionSchedulerImpl;
errorHandler?: ErrorHandler;

constructor(injector: EnvironmentInjector) {
// Use coalsecing to ensure we aren't ever running this check synchronously
super({shouldCoalesceEventChangeDetection: true, shouldCoalesceRunChangeDetection: true});

// prevent emits to ensure code doesn't rely on these
this.onMicrotaskEmpty.emit = () => {};
this.onStable.emit = () => {
this.scheduler ||= injector.get(ChangeDetectionSchedulerImpl);
if (this.scheduler.pendingRenderTaskId || this.scheduler.runningTick) {
return;
}
this.applicationRef ||= injector.get(ApplicationRef);
for (const view of this.applicationRef.allViews) {
try {
view._checkNoChangesWithMode(CheckNoChangesMode.Exhaustive);
} catch (e) {
this.errorHandler ||= injector.get(ErrorHandler);
this.errorHandler.handleError(e);
}
}
};
this.onUnstable.emit = () => {};
}
}

function exhaustiveCheckNoChangesInterval(interval: number) {
return {
provide: ENVIRONMENT_INITIALIZER,
multi: true,
useFactory: () => {
const applicationRef = inject(ApplicationRef);
const errorHandler = inject(ErrorHandler);
const injector = inject(EnvironmentInjector);
const scheduler = inject(ChangeDetectionSchedulerImpl);

return () => {
const timer = setInterval(() => {
if (scheduler.pendingRenderTaskId || scheduler.runningTick) {
return;
}
for (const view of applicationRef.allViews) {
try {
view._checkNoChangesWithMode(CheckNoChangesMode.Exhaustive);
} catch (e) {
errorHandler.handleError(e);
}
}
}, interval);
injector.onDestroy(() => clearInterval(timer));
};
},
};
}
1 change: 1 addition & 0 deletions packages/core/src/core_private_export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export {
NotificationSource as ɵNotificationSource,
ZONELESS_ENABLED as ɵZONELESS_ENABLED,
} from './change_detection/scheduling/zoneless_scheduling';
export {provideExhaustiveCheckNoChangesForDebug as ɵprovideExhaustiveCheckNoChangesForDebug} from './change_detection/scheduling/zoneless_scheduling_impl';
export {Console as ɵConsole} from './console';
export {
DeferBlockDetails as ɵDeferBlockDetails,
Expand Down
27 changes: 18 additions & 9 deletions packages/core/src/render3/instructions/change_detection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ import {
ReactiveLViewConsumer,
} from '../reactive_lview_consumer';
import {
CheckNoChangesMode,
enterView,
isExhaustiveCheckNoChanges,
isInCheckNoChangesMode,
isRefreshingViews,
leaveView,
Expand Down Expand Up @@ -143,12 +145,16 @@ function detectChangesInViewWhileDirty(lView: LView, mode: ChangeDetectionMode)
}
}

export function checkNoChangesInternal(lView: LView, notifyErrorHandler = true) {
setIsInCheckNoChangesMode(true);
export function checkNoChangesInternal(
lView: LView,
mode: CheckNoChangesMode,
notifyErrorHandler = true,
) {
setIsInCheckNoChangesMode(mode);
try {
detectChangesInternal(lView, notifyErrorHandler);
} finally {
setIsInCheckNoChangesMode(false);
setIsInCheckNoChangesMode(CheckNoChangesMode.Off);
}
}

Expand Down Expand Up @@ -329,12 +335,13 @@ export function refreshView<T>(
lView[FLAGS] &= ~(LViewFlags.Dirty | LViewFlags.FirstLViewPass);
}
} catch (e) {
// If refreshing a view causes an error, we need to remark the ancestors as needing traversal
// because the error might have caused a situation where views below the current location are
// dirty but will be unreachable because the "has dirty children" flag in the ancestors has been
// cleared during change detection and we failed to run to completion.

markAncestorsForTraversal(lView);
if (!isInCheckNoChangesPass) {
// If refreshing a view causes an error, we need to remark the ancestors as needing traversal
// because the error might have caused a situation where views below the current location are
// dirty but will be unreachable because the "has dirty children" flag in the ancestors has been
// cleared during change detection and we failed to run to completion.
markAncestorsForTraversal(lView);
}
throw e;
} finally {
if (currentConsumer !== null) {
Expand Down Expand Up @@ -469,6 +476,8 @@ function detectChangesInView(lView: LView, mode: ChangeDetectionMode) {
// Refresh views when they have a dirty reactive consumer, regardless of mode.
shouldRefreshView ||= !!(consumer?.dirty && consumerPollProducersForChange(consumer));

shouldRefreshView ||= !!(ngDevMode && isExhaustiveCheckNoChanges());

// Mark the Flags and `ReactiveNode` as not dirty before refreshing the component, so that they
// can be re-dirtied during the refresh process.
if (consumer) {
Expand Down
19 changes: 15 additions & 4 deletions packages/core/src/render3/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,12 @@ const instructionState: InstructionState = {
skipHydrationRootTNode: null,
};

export enum CheckNoChangesMode {
Off,
Exhaustive,
OnlyDirtyViews,
}

/**
* In this mode, any changes in bindings will throw an ExpressionChangedAfterChecked error.
*
Expand All @@ -216,7 +222,7 @@ const instructionState: InstructionState = {
* The `checkNoChanges` function is invoked only in ngDevMode=true and verifies that no unintended
* changes exist in the change detector or its children.
*/
let _isInCheckNoChangesMode = false;
let _checkNoChangesMode: CheckNoChangesMode = CheckNoChangesMode.Off;

/**
* Flag used to indicate that we are in the middle running change detection on a view
Expand Down Expand Up @@ -411,12 +417,17 @@ export function getContextLView(): LView {

export function isInCheckNoChangesMode(): boolean {
!ngDevMode && throwError('Must never be called in production mode');
return _isInCheckNoChangesMode;
return _checkNoChangesMode !== CheckNoChangesMode.Off;
}

export function isExhaustiveCheckNoChanges(): boolean {
!ngDevMode && throwError('Must never be called in production mode');
return _checkNoChangesMode === CheckNoChangesMode.Exhaustive;
}

export function setIsInCheckNoChangesMode(mode: boolean): void {
export function setIsInCheckNoChangesMode(mode: CheckNoChangesMode): void {
!ngDevMode && throwError('Must never be called in production mode');
_isInCheckNoChangesMode = mode;
_checkNoChangesMode = mode;
}

export function isRefreshingViews(): boolean {
Expand Down
10 changes: 9 additions & 1 deletion packages/core/src/render3/view_ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
detachViewFromDOM,
trackMovedView,
} from './node_manipulation';
import {CheckNoChangesMode} from './state';
import {storeLViewOnDestroy, updateAncestorTraversalFlagsOnAttach} from './util/view_utils';

// Needed due to tsickle downleveling where multiple `implements` with classes creates
Expand Down Expand Up @@ -320,7 +321,14 @@ export class ViewRef<T> implements EmbeddedViewRef<T>, ChangeDetectorRefInterfac
*/
checkNoChanges(): void {
if (ngDevMode) {
checkNoChangesInternal(this._lView, this.notifyErrorHandler);
this._checkNoChangesWithMode(CheckNoChangesMode.OnlyDirtyViews);
}
}

/**@internal */
_checkNoChangesWithMode(mode: CheckNoChangesMode) {
if (ngDevMode) {
checkNoChangesInternal(this._lView, mode, this.notifyErrorHandler);
}
}

Expand Down

0 comments on commit 1705e06

Please sign in to comment.