Skip to content

Commit

Permalink
refactor(core): renames checkNoChangesMode to be clearer (#39277)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
thePunderWoman authored and AndrewKushnir committed Oct 16, 2020
1 parent 96f59f6 commit a3812c6
Show file tree
Hide file tree
Showing 10 changed files with 38 additions and 33 deletions.
4 changes: 2 additions & 2 deletions packages/core/src/render3/bindings.ts
Expand Up @@ -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';


Expand Down Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/render3/hooks.ts
Expand Up @@ -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';



Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/render3/instructions/advance.ts
Expand Up @@ -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';


/**
Expand Down Expand Up @@ -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(
Expand Down
26 changes: 14 additions & 12 deletions packages/core/src/render3/instructions/shared.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -383,7 +383,9 @@ export function refreshView<T>(
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);

Expand All @@ -397,7 +399,7 @@ export function refreshView<T>(

// 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) {
Expand Down Expand Up @@ -425,7 +427,7 @@ export function refreshView<T>(

// 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) {
Expand Down Expand Up @@ -459,7 +461,7 @@ export function refreshView<T>(

// 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) {
Expand Down Expand Up @@ -489,7 +491,7 @@ export function refreshView<T>(
// 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) {
Expand All @@ -504,7 +506,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 = !getCheckNoChangesMode();
const normalExecutionPath = !isInCheckNoChangesMode();
const creationModeIsActive = isCreationMode(lView);
try {
if (normalExecutionPath && !creationModeIsActive && rendererFactory.begin) {
Expand All @@ -529,7 +531,7 @@ function executeTemplate<T>(
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 {
Expand Down Expand Up @@ -1918,11 +1920,11 @@ export function detectChangesInRootView(lView: LView): void {
}

export function checkNoChangesInternal<T>(tView: TView, view: LView, context: T) {
setCheckNoChangesMode(true);
setIsInCheckNoChangesMode(true);
try {
detectChangesInternal(tView, view, context);
} finally {
setCheckNoChangesMode(false);
setIsInCheckNoChangesMode(false);
}
}

Expand All @@ -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.
*/
export function checkNoChangesInRootView(lView: LView): void {
setCheckNoChangesMode(true);
setIsInCheckNoChangesMode(true);
try {
detectChangesInRootView(lView);
} finally {
setCheckNoChangesMode(false);
setIsInCheckNoChangesMode(false);
}
}

Expand Down
15 changes: 9 additions & 6 deletions packages/core/src/render3/state.ts
Expand Up @@ -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,
};


Expand Down Expand Up @@ -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)
Expand Down
Expand Up @@ -156,7 +156,7 @@
"name": "generatePropertyAliases"
},
{
"name": "getCheckNoChangesMode"
"name": "isInCheckNoChangesMode"
},
{
"name": "getClosureSafeProperty"
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/bundling/forms/bundle.golden_symbols.json
Expand Up @@ -948,7 +948,7 @@
"name": "generatePropertyAliases"
},
{
"name": "getCheckNoChangesMode"
"name": "isInCheckNoChangesMode"
},
{
"name": "getClosureSafeProperty"
Expand Down Expand Up @@ -1485,7 +1485,7 @@
"name": "setBindingRootForHostBindings"
},
{
"name": "setCheckNoChangesMode"
"name": "setIsInCheckNoChangesMode"
},
{
"name": "setCurrentDirectiveIndex"
Expand Down
Expand Up @@ -108,7 +108,7 @@
"name": "extractPipeDef"
},
{
"name": "getCheckNoChangesMode"
"name": "isInCheckNoChangesMode"
},
{
"name": "getClosureSafeProperty"
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/bundling/router/bundle.golden_symbols.json
Expand Up @@ -1260,7 +1260,7 @@
"name": "getBootstrapListener"
},
{
"name": "getCheckNoChangesMode"
"name": "isInCheckNoChangesMode"
},
{
"name": "getClosureSafeProperty"
Expand Down Expand Up @@ -1818,7 +1818,7 @@
"name": "setBindingRootForHostBindings"
},
{
"name": "setCheckNoChangesMode"
"name": "setIsInCheckNoChangesMode"
},
{
"name": "setCurrentDirectiveIndex"
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/bundling/todo/bundle.golden_symbols.json
Expand Up @@ -330,7 +330,7 @@
"name": "generatePropertyAliases"
},
{
"name": "getCheckNoChangesMode"
"name": "isInCheckNoChangesMode"
},
{
"name": "getClosureSafeProperty"
Expand Down Expand Up @@ -642,7 +642,7 @@
"name": "setBindingRootForHostBindings"
},
{
"name": "setCheckNoChangesMode"
"name": "setIsInCheckNoChangesMode"
},
{
"name": "setCurrentDirectiveIndex"
Expand Down

0 comments on commit a3812c6

Please sign in to comment.