Skip to content

Commit

Permalink
perf(core): allow checkNoChanges mode to be tree-shaken in producti…
Browse files Browse the repository at this point in the history
…on (#45913)

This commit guards all logic that exists for the `checkNoChanges` mode
with `ngDevMode` checks such that the logic can be tree-shaken.

PR Close #45913
  • Loading branch information
JoostK authored and AndrewKushnir committed May 9, 2022
1 parent def1f0f commit 75b3d0f
Show file tree
Hide file tree
Showing 13 changed files with 33 additions and 102 deletions.
2 changes: 1 addition & 1 deletion packages/core/src/change_detection/change_detector_ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export abstract class ChangeDetectorRef {
* Checks the change detector and its children, and throws if any changes are detected.
*
* Use in development mode to verify that running change detection doesn't introduce
* other changes.
* other changes. Calling it in production mode is a noop.
*/
abstract checkNoChanges(): void;

Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/render3/instructions/advance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ import {getLView, getSelectedIndex, getTView, isInCheckNoChangesMode, setSelecte
*/
export function ɵɵadvance(delta: number): void {
ngDevMode && assertGreaterThan(delta, 0, 'Can only advance forward');
selectIndexInternal(getTView(), getLView(), getSelectedIndex() + delta, isInCheckNoChangesMode());
selectIndexInternal(
getTView(), getLView(), getSelectedIndex() + delta, !!ngDevMode && isInCheckNoChangesMode());
}

export function selectIndexInternal(
Expand Down
14 changes: 9 additions & 5 deletions packages/core/src/render3/instructions/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ export function refreshView<T>(
enterView(lView);
// Check no changes mode is a dev only mode used to verify that bindings have not changed
// since they were assigned. We do not want to execute lifecycle hooks in that mode.
const isInCheckNoChangesPass = isInCheckNoChangesMode();
const isInCheckNoChangesPass = ngDevMode && isInCheckNoChangesMode();
try {
resetPreOrderHookFlags(lView);

Expand Down Expand Up @@ -505,18 +505,22 @@ export function refreshView<T>(
export function renderComponentOrTemplate<T>(
tView: TView, lView: LView, templateFn: ComponentTemplate<{}>|null, context: T) {
const rendererFactory = lView[RENDERER_FACTORY];
const normalExecutionPath = !isInCheckNoChangesMode();

// Check no changes mode is a dev only mode used to verify that bindings have not changed
// since they were assigned. We do not want to invoke renderer factory functions in that mode
// to avoid any possible side-effects.
const checkNoChangesMode = !!ngDevMode && isInCheckNoChangesMode();
const creationModeIsActive = isCreationMode(lView);
try {
if (normalExecutionPath && !creationModeIsActive && rendererFactory.begin) {
if (!checkNoChangesMode && !creationModeIsActive && rendererFactory.begin) {
rendererFactory.begin();
}
if (creationModeIsActive) {
renderView(tView, lView, context);
}
refreshView(tView, lView, templateFn, context);
} finally {
if (normalExecutionPath && !creationModeIsActive && rendererFactory.end) {
if (!checkNoChangesMode && !creationModeIsActive && rendererFactory.end) {
rendererFactory.end();
}
}
Expand All @@ -531,7 +535,7 @@ function executeTemplate<T>(
if (isUpdatePhase && lView.length > HEADER_OFFSET) {
// When we're updating, inherently select 0 so we don't
// have to generate that instruction for most update blocks.
selectIndexInternal(tView, lView, HEADER_OFFSET, isInCheckNoChangesMode());
selectIndexInternal(tView, lView, HEADER_OFFSET, !!ngDevMode && isInCheckNoChangesMode());
}

const preHookType =
Expand Down
30 changes: 15 additions & 15 deletions packages/core/src/render3/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import {InjectFlags} from '../di/interface/injector';
import {assertDefined, assertEqual, assertGreaterThanOrEqual, assertLessThan, assertNotEqual} from '../util/assert';
import {assertDefined, assertEqual, assertGreaterThanOrEqual, assertLessThan, assertNotEqual, throwError} from '../util/assert';

import {assertLViewOrUndefined, assertTNodeForLView, assertTNodeForTView} from './assert';
import {DirectiveDef} from './interfaces/definition';
Expand Down Expand Up @@ -172,24 +172,23 @@ interface InstructionState {
* ```
*/
bindingsEnabled: boolean;

/**
* In this mode, any changes in bindings will throw an ExpressionChangedAfterChecked error.
*
* Necessary to support ChangeDetectorRef.checkNoChanges().
*
* checkNoChanges Runs only in devmode=true and verifies that no unintended changes exist in
* the change detector or its children.
*/
isInCheckNoChangesMode: boolean;
}

const instructionState: InstructionState = {
lFrame: createLFrame(null),
bindingsEnabled: true,
isInCheckNoChangesMode: false,
};

/**
* In this mode, any changes in bindings will throw an ExpressionChangedAfterChecked error.
*
* Necessary to support ChangeDetectorRef.checkNoChanges().
*
* 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;

/**
* Returns true if the instruction state stack is empty.
*
Expand Down Expand Up @@ -353,12 +352,13 @@ export function getContextLView(): LView {
}

export function isInCheckNoChangesMode(): boolean {
// TODO(misko): remove this from the LView since it is ngDevMode=true mode only.
return instructionState.isInCheckNoChangesMode;
!ngDevMode && throwError('Must never be called in production mode');
return _isInCheckNoChangesMode;
}

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

// top level variables should not be exported for performance reasons (PERF_NOTES.md)
Expand Down
8 changes: 6 additions & 2 deletions packages/core/src/render3/view_ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,9 @@ export class ViewRef<T> implements viewEngine_EmbeddedViewRef<T>, viewEngine_Int
* introduce other changes.
*/
checkNoChanges(): void {
checkNoChangesInternal(this._lView[TVIEW], this._lView, this.context as unknown as {});
if (ngDevMode) {
checkNoChangesInternal(this._lView[TVIEW], this._lView, this.context as unknown as {});
}
}

attachToViewContainerRef() {
Expand Down Expand Up @@ -318,7 +320,9 @@ export class RootViewRef<T> extends ViewRef<T> {
}

override checkNoChanges(): void {
checkNoChangesInRootView(this._view);
if (ngDevMode) {
checkNoChangesInRootView(this._view);
}
}

override get context(): T {
Expand Down
12 changes: 0 additions & 12 deletions packages/core/test/bundling/animations/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -725,12 +725,6 @@
{
"name": "detachMovedView"
},
{
"name": "detectChangesInRootView"
},
{
"name": "detectChangesInternal"
},
{
"name": "diPublicInInjector"
},
Expand Down Expand Up @@ -1022,9 +1016,6 @@
{
"name": "isImportedNgModuleProviders"
},
{
"name": "isInCheckNoChangesMode"
},
{
"name": "isInlineTemplate"
},
Expand Down Expand Up @@ -1313,9 +1304,6 @@
{
"name": "setInputsFromAttrs"
},
{
"name": "setIsInCheckNoChangesMode"
},
{
"name": "setSelectedIndex"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,9 +236,6 @@
{
"name": "isCurrentTNodeParent"
},
{
"name": "isInCheckNoChangesMode"
},
{
"name": "isInlineTemplate"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -746,12 +746,6 @@
{
"name": "detachView"
},
{
"name": "detectChangesInRootView"
},
{
"name": "detectChangesInternal"
},
{
"name": "diPublicInInjector"
},
Expand Down Expand Up @@ -1127,9 +1121,6 @@
{
"name": "isImportedNgModuleProviders"
},
{
"name": "isInCheckNoChangesMode"
},
{
"name": "isInlineTemplate"
},
Expand Down Expand Up @@ -1463,9 +1454,6 @@
{
"name": "setInputsFromAttrs"
},
{
"name": "setIsInCheckNoChangesMode"
},
{
"name": "setSelectedIndex"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -719,12 +719,6 @@
{
"name": "detachView"
},
{
"name": "detectChangesInRootView"
},
{
"name": "detectChangesInternal"
},
{
"name": "diPublicInInjector"
},
Expand Down Expand Up @@ -1088,9 +1082,6 @@
{
"name": "isImportedNgModuleProviders"
},
{
"name": "isInCheckNoChangesMode"
},
{
"name": "isInlineTemplate"
},
Expand Down Expand Up @@ -1442,9 +1433,6 @@
{
"name": "setInputsFromAttrs"
},
{
"name": "setIsInCheckNoChangesMode"
},
{
"name": "setSelectedIndex"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,6 @@
{
"name": "invertObject"
},
{
"name": "isInCheckNoChangesMode"
},
{
"name": "isLView"
},
Expand Down
12 changes: 0 additions & 12 deletions packages/core/test/bundling/router/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -1022,12 +1022,6 @@
{
"name": "detachView"
},
{
"name": "detectChangesInRootView"
},
{
"name": "detectChangesInternal"
},
{
"name": "diPublicInInjector"
},
Expand Down Expand Up @@ -1466,9 +1460,6 @@
{
"name": "isImportedNgModuleProviders"
},
{
"name": "isInCheckNoChangesMode"
},
{
"name": "isInlineTemplate"
},
Expand Down Expand Up @@ -1826,9 +1817,6 @@
{
"name": "setInputsFromAttrs"
},
{
"name": "setIsInCheckNoChangesMode"
},
{
"name": "setRouterState"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -461,12 +461,6 @@
{
"name": "detachMovedView"
},
{
"name": "detectChangesInRootView"
},
{
"name": "detectChangesInternal"
},
{
"name": "domRendererFactory3"
},
Expand Down Expand Up @@ -668,9 +662,6 @@
{
"name": "isImportedNgModuleProviders"
},
{
"name": "isInCheckNoChangesMode"
},
{
"name": "isLContainer"
},
Expand Down Expand Up @@ -848,9 +839,6 @@
{
"name": "setInjectImplementation"
},
{
"name": "setIsInCheckNoChangesMode"
},
{
"name": "setSelectedIndex"
},
Expand Down
12 changes: 0 additions & 12 deletions packages/core/test/bundling/todo/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -299,12 +299,6 @@
{
"name": "detachView"
},
{
"name": "detectChangesInRootView"
},
{
"name": "detectChangesInternal"
},
{
"name": "diPublicInInjector"
},
Expand Down Expand Up @@ -551,9 +545,6 @@
{
"name": "isDirectiveHost"
},
{
"name": "isInCheckNoChangesMode"
},
{
"name": "isInlineTemplate"
},
Expand Down Expand Up @@ -758,9 +749,6 @@
{
"name": "setInputsFromAttrs"
},
{
"name": "setIsInCheckNoChangesMode"
},
{
"name": "setSelectedIndex"
},
Expand Down

0 comments on commit 75b3d0f

Please sign in to comment.