From 20b801e707b9f3c9c3263973fb89b4ec5c775c64 Mon Sep 17 00:00:00 2001 From: Jessica Janiuk Date: Wed, 14 Oct 2020 15:11:01 -0700 Subject: [PATCH] refactor(core): renames checkNoChangesMode to be clearer (#39277) getCheckNoChangesMode was discovered to be unclear as to the purpose of it. This refactor is a simple renaming to make it much clearer what that method and property does. PR Close #39277 --- packages/core/src/render3/bindings.ts | 4 +-- packages/core/src/render3/hooks.ts | 6 ++--- .../core/src/render3/instructions/advance.ts | 4 +-- .../core/src/render3/instructions/shared.ts | 26 ++++++++++--------- packages/core/src/render3/state.ts | 15 ++++++----- .../cyclic_import/bundle.golden_symbols.json | 2 +- .../bundling/forms/bundle.golden_symbols.json | 4 +-- .../hello_world/bundle.golden_symbols.json | 2 +- .../router/bundle.golden_symbols.json | 4 +-- .../bundling/todo/bundle.golden_symbols.json | 4 +-- 10 files changed, 38 insertions(+), 33 deletions(-) diff --git a/packages/core/src/render3/bindings.ts b/packages/core/src/render3/bindings.ts index 0968e054ad..b81e2daf23 100644 --- a/packages/core/src/render3/bindings.ts +++ b/packages/core/src/render3/bindings.ts @@ -11,7 +11,7 @@ import {assertIndexInRange, assertLessThan, assertNotSame} from '../util/assert' import {getExpressionChangedErrorDetails, throwErrorIfNoChangesMode} from './errors'; import {LView} from './interfaces/view'; -import {getCheckNoChangesMode} from './state'; +import {isInCheckNoChangesMode} from './state'; import {NO_CHANGE} from './tokens'; @@ -52,7 +52,7 @@ export function bindingUpdated(lView: LView, bindingIndex: number, value: any): if (Object.is(oldValue, value)) { return false; } else { - if (ngDevMode && getCheckNoChangesMode()) { + if (ngDevMode && isInCheckNoChangesMode()) { // View engine didn't report undefined values as changed on the first checkNoChanges pass // (before the change detection was run). const oldValueToCompare = oldValue !== NO_CHANGE ? oldValue : undefined; diff --git a/packages/core/src/render3/hooks.ts b/packages/core/src/render3/hooks.ts index 5bcf1bacc6..bd08fdcaab 100644 --- a/packages/core/src/render3/hooks.ts +++ b/packages/core/src/render3/hooks.ts @@ -13,7 +13,7 @@ import {NgOnChangesFeatureImpl} from './features/ng_onchanges_feature'; import {DirectiveDef} from './interfaces/definition'; import {TNode} from './interfaces/node'; import {FLAGS, HookData, InitPhaseState, LView, LViewFlags, PREORDER_HOOK_FLAGS, PreOrderHookFlags, TView} from './interfaces/view'; -import {getCheckNoChangesMode} from './state'; +import {isInCheckNoChangesMode} from './state'; @@ -205,8 +205,8 @@ function callHooks( currentNodeIndex: number|null|undefined): void { ngDevMode && assertEqual( - getCheckNoChangesMode(), false, - 'Hooks should never be run in the check no changes mode.'); + isInCheckNoChangesMode(), false, + 'Hooks should never be run when in check no changes mode.'); const startIndex = currentNodeIndex !== undefined ? (currentView[PREORDER_HOOK_FLAGS] & PreOrderHookFlags.IndexOfTheNextPreOrderHookMaskMask) : 0; diff --git a/packages/core/src/render3/instructions/advance.ts b/packages/core/src/render3/instructions/advance.ts index 7ddbfc7ae2..0a037b18af 100644 --- a/packages/core/src/render3/instructions/advance.ts +++ b/packages/core/src/render3/instructions/advance.ts @@ -8,7 +8,7 @@ import {assertGreaterThan, assertIndexInRange} from '../../util/assert'; import {executeCheckHooks, executeInitAndCheckHooks} from '../hooks'; import {FLAGS, HEADER_OFFSET, InitPhaseState, LView, LViewFlags, TView} from '../interfaces/view'; -import {getCheckNoChangesMode, getLView, getSelectedIndex, getTView, setSelectedIndex} from '../state'; +import {getLView, getSelectedIndex, getTView, isInCheckNoChangesMode, setSelectedIndex} from '../state'; /** @@ -36,7 +36,7 @@ import {getCheckNoChangesMode, getLView, getSelectedIndex, getTView, setSelected */ export function ɵɵadvance(delta: number): void { ngDevMode && assertGreaterThan(delta, 0, 'Can only advance forward'); - selectIndexInternal(getTView(), getLView(), getSelectedIndex() + delta, getCheckNoChangesMode()); + selectIndexInternal(getTView(), getLView(), getSelectedIndex() + delta, isInCheckNoChangesMode()); } export function selectIndexInternal( diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index 39242c4a38..a234772125 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -33,7 +33,7 @@ import {isComponentDef, isComponentHost, isContentQueryHost, isLContainer, isRoo import {CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_COMPONENT_VIEW, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, InitPhaseState, INJECTOR, LView, LViewFlags, NEXT, PARENT, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, T_HOST, TData, TRANSPLANTED_VIEWS_TO_REFRESH, TVIEW, TView, TViewType} from '../interfaces/view'; import {assertNodeNotOfTypes, assertNodeOfPossibleTypes} from '../node_assert'; import {isInlineTemplate, isNodeMatchingSelectorList} from '../node_selector_matcher'; -import {enterView, getBindingsEnabled, getCheckNoChangesMode, getCurrentDirectiveIndex, getCurrentTNode, getSelectedIndex, isCurrentTNodeParent, leaveView, setBindingIndex, setBindingRootForHostBindings, setCheckNoChangesMode, setCurrentDirectiveIndex, setCurrentQueryIndex, setCurrentTNode, setSelectedIndex} from '../state'; +import {enterView, getBindingsEnabled, getCurrentDirectiveIndex, getCurrentTNode, getSelectedIndex, isCurrentTNodeParent, isInCheckNoChangesMode, leaveView, setBindingIndex, setBindingRootForHostBindings, setCurrentDirectiveIndex, setCurrentQueryIndex, setCurrentTNode, setIsInCheckNoChangesMode, setSelectedIndex} from '../state'; import {NO_CHANGE} from '../tokens'; import {isAnimationProp, mergeHostAttrs} from '../util/attrs_utils'; import {INTERPOLATION_DELIMITER, renderStringify, stringifyForError} from '../util/misc_utils'; @@ -383,7 +383,9 @@ export function refreshView( const flags = lView[FLAGS]; if ((flags & LViewFlags.Destroyed) === LViewFlags.Destroyed) return; enterView(lView); - const checkNoChangesMode = getCheckNoChangesMode(); + // 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(); try { resetPreOrderHookFlags(lView); @@ -397,7 +399,7 @@ export function refreshView( // execute pre-order hooks (OnInit, OnChanges, DoCheck) // PERF WARNING: do NOT extract this to a separate function without running benchmarks - if (!checkNoChangesMode) { + if (!isInCheckNoChangesPass) { if (hooksInitPhaseCompleted) { const preOrderCheckHooks = tView.preOrderCheckHooks; if (preOrderCheckHooks !== null) { @@ -425,7 +427,7 @@ export function refreshView( // execute content hooks (AfterContentInit, AfterContentChecked) // PERF WARNING: do NOT extract this to a separate function without running benchmarks - if (!checkNoChangesMode) { + if (!isInCheckNoChangesPass) { if (hooksInitPhaseCompleted) { const contentCheckHooks = tView.contentCheckHooks; if (contentCheckHooks !== null) { @@ -459,7 +461,7 @@ export function refreshView( // execute view hooks (AfterViewInit, AfterViewChecked) // PERF WARNING: do NOT extract this to a separate function without running benchmarks - if (!checkNoChangesMode) { + if (!isInCheckNoChangesPass) { if (hooksInitPhaseCompleted) { const viewCheckHooks = tView.viewCheckHooks; if (viewCheckHooks !== null) { @@ -489,7 +491,7 @@ export function refreshView( // refresh a `NgClass` binding should work. If we would reset the dirty state in the check // no changes cycle, the component would be not be dirty for the next update pass. This would // be different in production mode where the component dirty state is not reset. - if (!checkNoChangesMode) { + if (!isInCheckNoChangesPass) { lView[FLAGS] &= ~(LViewFlags.Dirty | LViewFlags.FirstLViewPass); } if (lView[FLAGS] & LViewFlags.RefreshTransplantedView) { @@ -504,7 +506,7 @@ export function refreshView( export function renderComponentOrTemplate( tView: TView, lView: LView, templateFn: ComponentTemplate<{}>|null, context: T) { const rendererFactory = lView[RENDERER_FACTORY]; - const normalExecutionPath = !getCheckNoChangesMode(); + const normalExecutionPath = !isInCheckNoChangesMode(); const creationModeIsActive = isCreationMode(lView); try { if (normalExecutionPath && !creationModeIsActive && rendererFactory.begin) { @@ -529,7 +531,7 @@ function executeTemplate( if (rf & RenderFlags.Update && 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, 0, getCheckNoChangesMode()); + selectIndexInternal(tView, lView, 0, isInCheckNoChangesMode()); } templateFn(rf, context); } finally { @@ -1918,11 +1920,11 @@ export function detectChangesInRootView(lView: LView): void { } export function checkNoChangesInternal(tView: TView, view: LView, context: T) { - setCheckNoChangesMode(true); + setIsInCheckNoChangesMode(true); try { detectChangesInternal(tView, view, context); } finally { - setCheckNoChangesMode(false); + setIsInCheckNoChangesMode(false); } } @@ -1936,11 +1938,11 @@ export function checkNoChangesInternal(tView: TView, view: LView, context: T) * @param lView The view which the change detection should be checked on. */ export function checkNoChangesInRootView(lView: LView): void { - setCheckNoChangesMode(true); + setIsInCheckNoChangesMode(true); try { detectChangesInRootView(lView); } finally { - setCheckNoChangesMode(false); + setIsInCheckNoChangesMode(false); } } diff --git a/packages/core/src/render3/state.ts b/packages/core/src/render3/state.ts index 1362dca13e..453a4679ca 100644 --- a/packages/core/src/render3/state.ts +++ b/packages/core/src/render3/state.ts @@ -159,14 +159,17 @@ interface InstructionState { * 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. */ - checkNoChangesMode: boolean; + isInCheckNoChangesMode: boolean; } export const instructionState: InstructionState = { lFrame: createLFrame(null), bindingsEnabled: true, - checkNoChangesMode: false, + isInCheckNoChangesMode: false, }; @@ -287,13 +290,13 @@ export function getContextLView(): LView { return instructionState.lFrame.contextLView; } -export function getCheckNoChangesMode(): boolean { +export function isInCheckNoChangesMode(): boolean { // TODO(misko): remove this from the LView since it is ngDevMode=true mode only. - return instructionState.checkNoChangesMode; + return instructionState.isInCheckNoChangesMode; } -export function setCheckNoChangesMode(mode: boolean): void { - instructionState.checkNoChangesMode = mode; +export function setIsInCheckNoChangesMode(mode: boolean): void { + instructionState.isInCheckNoChangesMode = mode; } // top level variables should not be exported for performance reasons (PERF_NOTES.md) 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 edc3754c80..d728771a54 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -156,7 +156,7 @@ "name": "generatePropertyAliases" }, { - "name": "getCheckNoChangesMode" + "name": "isInCheckNoChangesMode" }, { "name": "getClosureSafeProperty" diff --git a/packages/core/test/bundling/forms/bundle.golden_symbols.json b/packages/core/test/bundling/forms/bundle.golden_symbols.json index 0bf195a2ab..e8ecdcc6bb 100644 --- a/packages/core/test/bundling/forms/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms/bundle.golden_symbols.json @@ -948,7 +948,7 @@ "name": "generatePropertyAliases" }, { - "name": "getCheckNoChangesMode" + "name": "isInCheckNoChangesMode" }, { "name": "getClosureSafeProperty" @@ -1485,7 +1485,7 @@ "name": "setBindingRootForHostBindings" }, { - "name": "setCheckNoChangesMode" + "name": "setIsInCheckNoChangesMode" }, { "name": "setCurrentDirectiveIndex" 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 60853be1fe..f9022bcdb9 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -108,7 +108,7 @@ "name": "extractPipeDef" }, { - "name": "getCheckNoChangesMode" + "name": "isInCheckNoChangesMode" }, { "name": "getClosureSafeProperty" diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index a05336e440..8f93e8055f 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -1260,7 +1260,7 @@ "name": "getBootstrapListener" }, { - "name": "getCheckNoChangesMode" + "name": "isInCheckNoChangesMode" }, { "name": "getClosureSafeProperty" @@ -1818,7 +1818,7 @@ "name": "setBindingRootForHostBindings" }, { - "name": "setCheckNoChangesMode" + "name": "setIsInCheckNoChangesMode" }, { "name": "setCurrentDirectiveIndex" diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index dfaf93b213..e6a729ac4d 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -330,7 +330,7 @@ "name": "generatePropertyAliases" }, { - "name": "getCheckNoChangesMode" + "name": "isInCheckNoChangesMode" }, { "name": "getClosureSafeProperty" @@ -642,7 +642,7 @@ "name": "setBindingRootForHostBindings" }, { - "name": "setCheckNoChangesMode" + "name": "setIsInCheckNoChangesMode" }, { "name": "setCurrentDirectiveIndex"