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
This commit is contained in:
Jessica Janiuk 2020-10-14 15:11:01 -07:00 committed by Andrew Kushnir
parent 96f59f64a0
commit a3812c6cbd
10 changed files with 38 additions and 33 deletions

View File

@ -11,7 +11,7 @@ import {assertIndexInRange, assertLessThan, assertNotSame} from '../util/assert'
import {getExpressionChangedErrorDetails, throwErrorIfNoChangesMode} from './errors'; import {getExpressionChangedErrorDetails, throwErrorIfNoChangesMode} from './errors';
import {LView} from './interfaces/view'; import {LView} from './interfaces/view';
import {getCheckNoChangesMode} from './state'; import {isInCheckNoChangesMode} from './state';
import {NO_CHANGE} from './tokens'; import {NO_CHANGE} from './tokens';
@ -52,7 +52,7 @@ export function bindingUpdated(lView: LView, bindingIndex: number, value: any):
if (Object.is(oldValue, value)) { if (Object.is(oldValue, value)) {
return false; return false;
} else { } else {
if (ngDevMode && getCheckNoChangesMode()) { if (ngDevMode && isInCheckNoChangesMode()) {
// View engine didn't report undefined values as changed on the first checkNoChanges pass // View engine didn't report undefined values as changed on the first checkNoChanges pass
// (before the change detection was run). // (before the change detection was run).
const oldValueToCompare = oldValue !== NO_CHANGE ? oldValue : undefined; const oldValueToCompare = oldValue !== NO_CHANGE ? oldValue : undefined;

View File

@ -13,7 +13,7 @@ import {NgOnChangesFeatureImpl} from './features/ng_onchanges_feature';
import {DirectiveDef} from './interfaces/definition'; import {DirectiveDef} from './interfaces/definition';
import {TNode} from './interfaces/node'; import {TNode} from './interfaces/node';
import {FLAGS, HookData, InitPhaseState, LView, LViewFlags, PREORDER_HOOK_FLAGS, PreOrderHookFlags, TView} from './interfaces/view'; 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 { currentNodeIndex: number|null|undefined): void {
ngDevMode && ngDevMode &&
assertEqual( assertEqual(
getCheckNoChangesMode(), false, isInCheckNoChangesMode(), false,
'Hooks should never be run in the check no changes mode.'); 'Hooks should never be run when in check no changes mode.');
const startIndex = currentNodeIndex !== undefined ? const startIndex = currentNodeIndex !== undefined ?
(currentView[PREORDER_HOOK_FLAGS] & PreOrderHookFlags.IndexOfTheNextPreOrderHookMaskMask) : (currentView[PREORDER_HOOK_FLAGS] & PreOrderHookFlags.IndexOfTheNextPreOrderHookMaskMask) :
0; 0;

View File

@ -8,7 +8,7 @@
import {assertGreaterThan, assertIndexInRange} from '../../util/assert'; import {assertGreaterThan, assertIndexInRange} from '../../util/assert';
import {executeCheckHooks, executeInitAndCheckHooks} from '../hooks'; import {executeCheckHooks, executeInitAndCheckHooks} from '../hooks';
import {FLAGS, HEADER_OFFSET, InitPhaseState, LView, LViewFlags, TView} from '../interfaces/view'; 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 { export function ɵɵadvance(delta: number): void {
ngDevMode && assertGreaterThan(delta, 0, 'Can only advance forward'); ngDevMode && assertGreaterThan(delta, 0, 'Can only advance forward');
selectIndexInternal(getTView(), getLView(), getSelectedIndex() + delta, getCheckNoChangesMode()); selectIndexInternal(getTView(), getLView(), getSelectedIndex() + delta, isInCheckNoChangesMode());
} }
export function selectIndexInternal( export function selectIndexInternal(

View File

@ -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 {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 {assertNodeNotOfTypes, assertNodeOfPossibleTypes} from '../node_assert';
import {isInlineTemplate, isNodeMatchingSelectorList} from '../node_selector_matcher'; 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 {NO_CHANGE} from '../tokens';
import {isAnimationProp, mergeHostAttrs} from '../util/attrs_utils'; import {isAnimationProp, mergeHostAttrs} from '../util/attrs_utils';
import {INTERPOLATION_DELIMITER, renderStringify, stringifyForError} from '../util/misc_utils'; import {INTERPOLATION_DELIMITER, renderStringify, stringifyForError} from '../util/misc_utils';
@ -383,7 +383,9 @@ export function refreshView<T>(
const flags = lView[FLAGS]; const flags = lView[FLAGS];
if ((flags & LViewFlags.Destroyed) === LViewFlags.Destroyed) return; if ((flags & LViewFlags.Destroyed) === LViewFlags.Destroyed) return;
enterView(lView); 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 { try {
resetPreOrderHookFlags(lView); resetPreOrderHookFlags(lView);
@ -397,7 +399,7 @@ export function refreshView<T>(
// execute pre-order hooks (OnInit, OnChanges, DoCheck) // execute pre-order hooks (OnInit, OnChanges, DoCheck)
// PERF WARNING: do NOT extract this to a separate function without running benchmarks // PERF WARNING: do NOT extract this to a separate function without running benchmarks
if (!checkNoChangesMode) { if (!isInCheckNoChangesPass) {
if (hooksInitPhaseCompleted) { if (hooksInitPhaseCompleted) {
const preOrderCheckHooks = tView.preOrderCheckHooks; const preOrderCheckHooks = tView.preOrderCheckHooks;
if (preOrderCheckHooks !== null) { if (preOrderCheckHooks !== null) {
@ -425,7 +427,7 @@ export function refreshView<T>(
// execute content hooks (AfterContentInit, AfterContentChecked) // execute content hooks (AfterContentInit, AfterContentChecked)
// PERF WARNING: do NOT extract this to a separate function without running benchmarks // PERF WARNING: do NOT extract this to a separate function without running benchmarks
if (!checkNoChangesMode) { if (!isInCheckNoChangesPass) {
if (hooksInitPhaseCompleted) { if (hooksInitPhaseCompleted) {
const contentCheckHooks = tView.contentCheckHooks; const contentCheckHooks = tView.contentCheckHooks;
if (contentCheckHooks !== null) { if (contentCheckHooks !== null) {
@ -459,7 +461,7 @@ export function refreshView<T>(
// execute view hooks (AfterViewInit, AfterViewChecked) // execute view hooks (AfterViewInit, AfterViewChecked)
// PERF WARNING: do NOT extract this to a separate function without running benchmarks // PERF WARNING: do NOT extract this to a separate function without running benchmarks
if (!checkNoChangesMode) { if (!isInCheckNoChangesPass) {
if (hooksInitPhaseCompleted) { if (hooksInitPhaseCompleted) {
const viewCheckHooks = tView.viewCheckHooks; const viewCheckHooks = tView.viewCheckHooks;
if (viewCheckHooks !== null) { if (viewCheckHooks !== null) {
@ -489,7 +491,7 @@ export function refreshView<T>(
// refresh a `NgClass` binding should work. If we would reset the dirty state in the check // 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 // 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. // be different in production mode where the component dirty state is not reset.
if (!checkNoChangesMode) { if (!isInCheckNoChangesPass) {
lView[FLAGS] &= ~(LViewFlags.Dirty | LViewFlags.FirstLViewPass); lView[FLAGS] &= ~(LViewFlags.Dirty | LViewFlags.FirstLViewPass);
} }
if (lView[FLAGS] & LViewFlags.RefreshTransplantedView) { if (lView[FLAGS] & LViewFlags.RefreshTransplantedView) {
@ -504,7 +506,7 @@ export function refreshView<T>(
export function renderComponentOrTemplate<T>( export function renderComponentOrTemplate<T>(
tView: TView, lView: LView, templateFn: ComponentTemplate<{}>|null, context: T) { tView: TView, lView: LView, templateFn: ComponentTemplate<{}>|null, context: T) {
const rendererFactory = lView[RENDERER_FACTORY]; const rendererFactory = lView[RENDERER_FACTORY];
const normalExecutionPath = !getCheckNoChangesMode(); const normalExecutionPath = !isInCheckNoChangesMode();
const creationModeIsActive = isCreationMode(lView); const creationModeIsActive = isCreationMode(lView);
try { try {
if (normalExecutionPath && !creationModeIsActive && rendererFactory.begin) { if (normalExecutionPath && !creationModeIsActive && rendererFactory.begin) {
@ -529,7 +531,7 @@ function executeTemplate<T>(
if (rf & RenderFlags.Update && lView.length > HEADER_OFFSET) { if (rf & RenderFlags.Update && lView.length > HEADER_OFFSET) {
// When we're updating, inherently select 0 so we don't // When we're updating, inherently select 0 so we don't
// have to generate that instruction for most update blocks. // have to generate that instruction for most update blocks.
selectIndexInternal(tView, lView, 0, getCheckNoChangesMode()); selectIndexInternal(tView, lView, 0, isInCheckNoChangesMode());
} }
templateFn(rf, context); templateFn(rf, context);
} finally { } finally {
@ -1918,11 +1920,11 @@ export function detectChangesInRootView(lView: LView): void {
} }
export function checkNoChangesInternal<T>(tView: TView, view: LView, context: T) { export function checkNoChangesInternal<T>(tView: TView, view: LView, context: T) {
setCheckNoChangesMode(true); setIsInCheckNoChangesMode(true);
try { try {
detectChangesInternal(tView, view, context); detectChangesInternal(tView, view, context);
} finally { } finally {
setCheckNoChangesMode(false); setIsInCheckNoChangesMode(false);
} }
} }
@ -1936,11 +1938,11 @@ export function checkNoChangesInternal<T>(tView: TView, view: LView, context: T)
* @param lView The view which the change detection should be checked on. * @param lView The view which the change detection should be checked on.
*/ */
export function checkNoChangesInRootView(lView: LView): void { export function checkNoChangesInRootView(lView: LView): void {
setCheckNoChangesMode(true); setIsInCheckNoChangesMode(true);
try { try {
detectChangesInRootView(lView); detectChangesInRootView(lView);
} finally { } finally {
setCheckNoChangesMode(false); setIsInCheckNoChangesMode(false);
} }
} }

View File

@ -159,14 +159,17 @@ interface InstructionState {
* In this mode, any changes in bindings will throw an ExpressionChangedAfterChecked error. * In this mode, any changes in bindings will throw an ExpressionChangedAfterChecked error.
* *
* Necessary to support ChangeDetectorRef.checkNoChanges(). * 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 = { export const instructionState: InstructionState = {
lFrame: createLFrame(null), lFrame: createLFrame(null),
bindingsEnabled: true, bindingsEnabled: true,
checkNoChangesMode: false, isInCheckNoChangesMode: false,
}; };
@ -287,13 +290,13 @@ export function getContextLView(): LView {
return instructionState.lFrame.contextLView; 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. // 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 { export function setIsInCheckNoChangesMode(mode: boolean): void {
instructionState.checkNoChangesMode = mode; instructionState.isInCheckNoChangesMode = mode;
} }
// top level variables should not be exported for performance reasons (PERF_NOTES.md) // top level variables should not be exported for performance reasons (PERF_NOTES.md)

View File

@ -156,7 +156,7 @@
"name": "generatePropertyAliases" "name": "generatePropertyAliases"
}, },
{ {
"name": "getCheckNoChangesMode" "name": "isInCheckNoChangesMode"
}, },
{ {
"name": "getClosureSafeProperty" "name": "getClosureSafeProperty"

View File

@ -948,7 +948,7 @@
"name": "generatePropertyAliases" "name": "generatePropertyAliases"
}, },
{ {
"name": "getCheckNoChangesMode" "name": "isInCheckNoChangesMode"
}, },
{ {
"name": "getClosureSafeProperty" "name": "getClosureSafeProperty"
@ -1485,7 +1485,7 @@
"name": "setBindingRootForHostBindings" "name": "setBindingRootForHostBindings"
}, },
{ {
"name": "setCheckNoChangesMode" "name": "setIsInCheckNoChangesMode"
}, },
{ {
"name": "setCurrentDirectiveIndex" "name": "setCurrentDirectiveIndex"

View File

@ -108,7 +108,7 @@
"name": "extractPipeDef" "name": "extractPipeDef"
}, },
{ {
"name": "getCheckNoChangesMode" "name": "isInCheckNoChangesMode"
}, },
{ {
"name": "getClosureSafeProperty" "name": "getClosureSafeProperty"

View File

@ -1260,7 +1260,7 @@
"name": "getBootstrapListener" "name": "getBootstrapListener"
}, },
{ {
"name": "getCheckNoChangesMode" "name": "isInCheckNoChangesMode"
}, },
{ {
"name": "getClosureSafeProperty" "name": "getClosureSafeProperty"
@ -1818,7 +1818,7 @@
"name": "setBindingRootForHostBindings" "name": "setBindingRootForHostBindings"
}, },
{ {
"name": "setCheckNoChangesMode" "name": "setIsInCheckNoChangesMode"
}, },
{ {
"name": "setCurrentDirectiveIndex" "name": "setCurrentDirectiveIndex"

View File

@ -330,7 +330,7 @@
"name": "generatePropertyAliases" "name": "generatePropertyAliases"
}, },
{ {
"name": "getCheckNoChangesMode" "name": "isInCheckNoChangesMode"
}, },
{ {
"name": "getClosureSafeProperty" "name": "getClosureSafeProperty"
@ -642,7 +642,7 @@
"name": "setBindingRootForHostBindings" "name": "setBindingRootForHostBindings"
}, },
{ {
"name": "setCheckNoChangesMode" "name": "setIsInCheckNoChangesMode"
}, },
{ {
"name": "setCurrentDirectiveIndex" "name": "setCurrentDirectiveIndex"