From 75b3d0f843b4a17981d58ab46a3541daa67a49bb Mon Sep 17 00:00:00 2001 From: JoostK Date: Thu, 9 Dec 2021 01:11:30 +0100 Subject: [PATCH] perf(core): allow `checkNoChanges` mode to be tree-shaken in production (#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 --- .../change_detection/change_detector_ref.ts | 2 +- .../core/src/render3/instructions/advance.ts | 3 +- .../core/src/render3/instructions/shared.ts | 14 +++++---- packages/core/src/render3/state.ts | 30 +++++++++---------- packages/core/src/render3/view_ref.ts | 8 +++-- .../animations/bundle.golden_symbols.json | 12 -------- .../cyclic_import/bundle.golden_symbols.json | 3 -- .../forms_reactive/bundle.golden_symbols.json | 12 -------- .../bundle.golden_symbols.json | 12 -------- .../hello_world/bundle.golden_symbols.json | 3 -- .../router/bundle.golden_symbols.json | 12 -------- .../bundle.golden_symbols.json | 12 -------- .../bundling/todo/bundle.golden_symbols.json | 12 -------- 13 files changed, 33 insertions(+), 102 deletions(-) diff --git a/packages/core/src/change_detection/change_detector_ref.ts b/packages/core/src/change_detection/change_detector_ref.ts index 68a358ba323..46e27beff08 100644 --- a/packages/core/src/change_detection/change_detector_ref.ts +++ b/packages/core/src/change_detection/change_detector_ref.ts @@ -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; diff --git a/packages/core/src/render3/instructions/advance.ts b/packages/core/src/render3/instructions/advance.ts index fd06bdfbb7a..8ff38fd1828 100644 --- a/packages/core/src/render3/instructions/advance.ts +++ b/packages/core/src/render3/instructions/advance.ts @@ -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( diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index f912c644742..eb3de9bfb6b 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -384,7 +384,7 @@ export function refreshView( 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); @@ -505,10 +505,14 @@ export function refreshView( export function renderComponentOrTemplate( 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) { @@ -516,7 +520,7 @@ export function renderComponentOrTemplate( } refreshView(tView, lView, templateFn, context); } finally { - if (normalExecutionPath && !creationModeIsActive && rendererFactory.end) { + if (!checkNoChangesMode && !creationModeIsActive && rendererFactory.end) { rendererFactory.end(); } } @@ -531,7 +535,7 @@ function executeTemplate( 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 = diff --git a/packages/core/src/render3/state.ts b/packages/core/src/render3/state.ts index a947ef5b83e..f2ff8ba197d 100644 --- a/packages/core/src/render3/state.ts +++ b/packages/core/src/render3/state.ts @@ -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'; @@ -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. * @@ -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) diff --git a/packages/core/src/render3/view_ref.ts b/packages/core/src/render3/view_ref.ts index aac220d2ee8..5b3adb038e0 100644 --- a/packages/core/src/render3/view_ref.ts +++ b/packages/core/src/render3/view_ref.ts @@ -281,7 +281,9 @@ export class ViewRef implements viewEngine_EmbeddedViewRef, 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() { @@ -318,7 +320,9 @@ export class RootViewRef extends ViewRef { } override checkNoChanges(): void { - checkNoChangesInRootView(this._view); + if (ngDevMode) { + checkNoChangesInRootView(this._view); + } } override get context(): T { diff --git a/packages/core/test/bundling/animations/bundle.golden_symbols.json b/packages/core/test/bundling/animations/bundle.golden_symbols.json index f8af9895d8e..3f1a44ae621 100644 --- a/packages/core/test/bundling/animations/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations/bundle.golden_symbols.json @@ -725,12 +725,6 @@ { "name": "detachMovedView" }, - { - "name": "detectChangesInRootView" - }, - { - "name": "detectChangesInternal" - }, { "name": "diPublicInInjector" }, @@ -1022,9 +1016,6 @@ { "name": "isImportedNgModuleProviders" }, - { - "name": "isInCheckNoChangesMode" - }, { "name": "isInlineTemplate" }, @@ -1313,9 +1304,6 @@ { "name": "setInputsFromAttrs" }, - { - "name": "setIsInCheckNoChangesMode" - }, { "name": "setSelectedIndex" }, diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json index bc3afb50623..f08fac8690c 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -236,9 +236,6 @@ { "name": "isCurrentTNodeParent" }, - { - "name": "isInCheckNoChangesMode" - }, { "name": "isInlineTemplate" }, diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json index 8fefd20c098..84eb4356904 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -746,12 +746,6 @@ { "name": "detachView" }, - { - "name": "detectChangesInRootView" - }, - { - "name": "detectChangesInternal" - }, { "name": "diPublicInInjector" }, @@ -1127,9 +1121,6 @@ { "name": "isImportedNgModuleProviders" }, - { - "name": "isInCheckNoChangesMode" - }, { "name": "isInlineTemplate" }, @@ -1463,9 +1454,6 @@ { "name": "setInputsFromAttrs" }, - { - "name": "setIsInCheckNoChangesMode" - }, { "name": "setSelectedIndex" }, diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json index b24fd3660e4..fca36a3f916 100644 --- a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json @@ -719,12 +719,6 @@ { "name": "detachView" }, - { - "name": "detectChangesInRootView" - }, - { - "name": "detectChangesInternal" - }, { "name": "diPublicInInjector" }, @@ -1088,9 +1082,6 @@ { "name": "isImportedNgModuleProviders" }, - { - "name": "isInCheckNoChangesMode" - }, { "name": "isInlineTemplate" }, @@ -1442,9 +1433,6 @@ { "name": "setInputsFromAttrs" }, - { - "name": "setIsInCheckNoChangesMode" - }, { "name": "setSelectedIndex" }, diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json index a7b2a9a0171..e01f3506985 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -167,9 +167,6 @@ { "name": "invertObject" }, - { - "name": "isInCheckNoChangesMode" - }, { "name": "isLView" }, diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index c88474e7ae9..c9fcbcff5fc 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -1022,12 +1022,6 @@ { "name": "detachView" }, - { - "name": "detectChangesInRootView" - }, - { - "name": "detectChangesInternal" - }, { "name": "diPublicInInjector" }, @@ -1466,9 +1460,6 @@ { "name": "isImportedNgModuleProviders" }, - { - "name": "isInCheckNoChangesMode" - }, { "name": "isInlineTemplate" }, @@ -1826,9 +1817,6 @@ { "name": "setInputsFromAttrs" }, - { - "name": "setIsInCheckNoChangesMode" - }, { "name": "setRouterState" }, diff --git a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json index c44f51d0c68..605faecd24e 100644 --- a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json +++ b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json @@ -461,12 +461,6 @@ { "name": "detachMovedView" }, - { - "name": "detectChangesInRootView" - }, - { - "name": "detectChangesInternal" - }, { "name": "domRendererFactory3" }, @@ -668,9 +662,6 @@ { "name": "isImportedNgModuleProviders" }, - { - "name": "isInCheckNoChangesMode" - }, { "name": "isLContainer" }, @@ -848,9 +839,6 @@ { "name": "setInjectImplementation" }, - { - "name": "setIsInCheckNoChangesMode" - }, { "name": "setSelectedIndex" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 0d9e6a2a3b9..c9ca239e3bf 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -299,12 +299,6 @@ { "name": "detachView" }, - { - "name": "detectChangesInRootView" - }, - { - "name": "detectChangesInternal" - }, { "name": "diPublicInInjector" }, @@ -551,9 +545,6 @@ { "name": "isDirectiveHost" }, - { - "name": "isInCheckNoChangesMode" - }, { "name": "isInlineTemplate" }, @@ -758,9 +749,6 @@ { "name": "setInputsFromAttrs" }, - { - "name": "setIsInCheckNoChangesMode" - }, { "name": "setSelectedIndex" },