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 5a84965
Show file tree
Hide file tree
Showing 10 changed files with 393 additions and 20 deletions.
3 changes: 2 additions & 1 deletion packages/core/src/application/application_ref.ts
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
@@ -0,0 +1,136 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {ApplicationRef} from '../../application/application_ref';
import {ChangeDetectionSchedulerImpl} from './zoneless_scheduling_impl';
import {Injectable} from '../../di/injectable';
import {inject} from '../../di/injector_compatibility';
import {makeEnvironmentProviders} from '../../di/provider_collection';
import {NgZone} from '../../zone/ng_zone';

import {EnvironmentInjector} from '../../di/r3_injector';
import {ENVIRONMENT_INITIALIZER} from '../../di/initializer_token';
import {InjectionToken} from '../../di/injection_token';
import {CheckNoChangesMode} from '../../render3/state';
import {ErrorHandler} from '../../error_handler';
import {checkNoChangesInternal} from '../../render3/instructions/change_detection';

const EXHAUSTIVE_CHECK_NO_CHANGES_FOR_DEBUG = new InjectionToken<boolean>(
'exhaustive check no changes for debug',
);

/**
* For internal use only. For use with zoneless change detection to check bindings update without notification.
*
* @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. By default, this check is exhaustive
* and will always check all views, regardless of their "dirty" state and `ChangeDetectionStrategy`.
*/
export function provideCheckNoChangesForDebug(options: {
interval?: number;
useNgZoneOnStable?: boolean;
exhaustive?: boolean;
}) {
if (typeof ngDevMode === 'undefined' || ngDevMode) {
if (options.interval === undefined && !options.useNgZoneOnStable) {
throw new Error('Must provide one of `useNgZoneOnStable` or `interval`');
}
return makeEnvironmentProviders([
{
provide: EXHAUSTIVE_CHECK_NO_CHANGES_FOR_DEBUG,
useValue: options?.exhaustive === false ? false : true,
},
options?.useNgZoneOnStable ? {provide: NgZone, useClass: DebugNgZoneForCheckNoChanges} : [],
options?.interval !== undefined ? exhaustiveCheckNoChangesInterval(options.interval) : [],
]);
} else {
return makeEnvironmentProviders([]);
}
}

@Injectable({providedIn: 'root'})
export class DebugNgZoneForCheckNoChanges extends NgZone {
private applicationRef?: ApplicationRef;
private scheduler?: ChangeDetectionSchedulerImpl;
private errorHandler?: ErrorHandler;
private readonly checkNoChangesMode = inject(EXHAUSTIVE_CHECK_NO_CHANGES_FOR_DEBUG)
? CheckNoChangesMode.Exhaustive
: CheckNoChangesMode.OnlyDirtyViews;

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 {
checkNoChangesInternal(view._lView, this.checkNoChangesMode, view.notifyErrorHandler);
} 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);
const ngZone = inject(NgZone);
const checkNoChangesMode = inject(EXHAUSTIVE_CHECK_NO_CHANGES_FOR_DEBUG)
? CheckNoChangesMode.Exhaustive
: CheckNoChangesMode.OnlyDirtyViews;

return () => {
ngZone.runOutsideAngular(() => {
const timer = setInterval(() => {
if (scheduler.pendingRenderTaskId || scheduler.runningTick) {
return;
}
for (const view of applicationRef.allViews) {
try {
checkNoChangesInternal(view._lView, checkNoChangesMode, view.notifyErrorHandler);
} catch (e) {
errorHandler.handleError(e);
}
}
}, interval);

injector.onDestroy(() => clearInterval(timer));
});
};
},
};
}
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 Down
Expand Up @@ -66,9 +66,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 @@ -173,9 +173,11 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler {
if (this.pendingRenderTaskId !== null || this.runningTick || this.appRef._runningTick) {
return false;
}
// 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) {
return true;
} else if (this.zoneIsDefined && NgZone.isInAngularZone()) {
// If we're inside the zone don't bother with scheduler. Zone will stabilize
// eventually and run change detection.
return false;
}

Expand Down
1 change: 1 addition & 0 deletions packages/core/src/core_private_export.ts
Expand Up @@ -26,6 +26,7 @@ export {
NotificationSource as ɵNotificationSource,
ZONELESS_ENABLED as ɵZONELESS_ENABLED,
} from './change_detection/scheduling/zoneless_scheduling';
export {provideCheckNoChangesForDebug as ɵprovideCheckNoChangesForDebug} from './change_detection/scheduling/exhaustive_check_no_changes';
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
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
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 = 0; /* 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
7 changes: 6 additions & 1 deletion packages/core/src/render3/view_ref.ts
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,11 @@ export class ViewRef<T> implements EmbeddedViewRef<T>, ChangeDetectorRefInterfac
*/
checkNoChanges(): void {
if (ngDevMode) {
checkNoChangesInternal(this._lView, this.notifyErrorHandler);
checkNoChangesInternal(
this._lView,
CheckNoChangesMode.OnlyDirtyViews,
this.notifyErrorHandler,
);
}
}

Expand Down

0 comments on commit 5a84965

Please sign in to comment.