Skip to content
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.

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

perf(core): allow checkNoChanges mode to be tree-shaken in production #45913

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
6 changes: 3 additions & 3 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,7 +505,7 @@ 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();
const normalExecutionPath = !ngDevMode || !isInCheckNoChangesMode();
AndrewKushnir marked this conversation as resolved.
Show resolved Hide resolved
const creationModeIsActive = isCreationMode(lView);
try {
if (normalExecutionPath && !creationModeIsActive && rendererFactory.begin) {
Expand All @@ -531,7 +531,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().
*
* checkNoChanges Runs only in devmode=true and verifies that no unintended changes exist in
AndrewKushnir marked this conversation as resolved.
Show resolved Hide resolved
* 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');
JoostK marked this conversation as resolved.
Show resolved Hide resolved
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.
AndrewKushnir marked this conversation as resolved.
Show resolved Hide resolved
*/
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