diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index af89965760..1562ed407f 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -151,6 +151,13 @@ let bindingIndex: number; */ let cleanup: any[]|null; +/** + * In this mode, any changes in bindings will throw an ExpressionChangedAfterChecked error. + * + * Necessary to support ChangeDetectorRef.checkNoChanges(). + */ +let checkNoChangesMode = false; + const enum BindingDirection { Input, Output, @@ -194,9 +201,11 @@ export function enterView(newView: LView, host: LElementNode | LViewNode | null) * the direction of traversal (up or down the view tree) a bit clearer. */ export function leaveView(newView: LView): void { - executeHooks( - currentView.data, currentView.tView.viewHooks, currentView.tView.viewCheckHooks, - creationMode); + if (!checkNoChangesMode) { + executeHooks( + currentView.data, currentView.tView.viewHooks, currentView.tView.viewCheckHooks, + creationMode); + } // Views should be clean and in update mode after being checked, so these bits are cleared currentView.flags &= ~(LViewFlags.CreationMode | LViewFlags.Dirty); currentView.lifecycleStage = LifecycleStage.INIT; @@ -1135,9 +1144,11 @@ export function containerRefreshStart(index: number): void { (previousOrParentNode as LContainerNode).native, undefined, `the container's native element should not have been set yet.`); - // We need to execute init hooks here so ngOnInit hooks are called in top level views - // before they are called in embedded views (for backwards compatibility). - executeInitHooks(currentView, currentView.tView, creationMode); + if (!checkNoChangesMode) { + // We need to execute init hooks here so ngOnInit hooks are called in top level views + // before they are called in embedded views (for backwards compatibility). + executeInitHooks(currentView, currentView.tView, creationMode); + } } /** @@ -1270,8 +1281,10 @@ export function embeddedViewEnd(): void { * @param elementIndex */ export function directiveRefresh(directiveIndex: number, elementIndex: number): void { - executeInitHooks(currentView, currentView.tView, creationMode); - executeContentHooks(currentView, currentView.tView, creationMode); + if (!checkNoChangesMode) { + executeInitHooks(currentView, currentView.tView, creationMode); + executeContentHooks(currentView, currentView.tView, creationMode); + } const template = (tData[directiveIndex] as ComponentDef).template; if (template != null) { ngDevMode && assertDataInRange(elementIndex); @@ -1594,6 +1607,37 @@ export function detectChanges(component: T): void { } +/** + * Checks the change detector and its children, and throws if any changes are detected. + * + * This is used in development mode to verify that running change detection doesn't + * introduce other changes. + */ +export function checkNoChanges(component: T): void { + checkNoChangesMode = true; + try { + detectChanges(component); + } finally { + checkNoChangesMode = false; + } +} + +/** Throws an ExpressionChangedAfterChecked error if checkNoChanges mode is on. */ +function throwErrorIfNoChangesMode(oldValue: any, currValue: any): never|void { + if (checkNoChangesMode) { + let msg = + `ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value: '${oldValue}'. Current value: '${currValue}'.`; + if (creationMode) { + msg += + ` It seems like the view has been created after its parent and its children have been dirty checked.` + + ` Has it been created in a change detection hook ?`; + } + // TODO: include debug context + throw new Error(msg); + } +} + + /** Checks the view of the component provided. Does not gate on dirty checks or execute doCheck. */ function detectChangesInternal(hostView: LView, hostNode: LElementNode, component: T) { const componentIndex = hostNode.flags >> LNodeFlags.INDX_SHIFT; @@ -1672,6 +1716,7 @@ export function bind(value: T | NO_CHANGE): T|NO_CHANGE { const changed: boolean = value !== NO_CHANGE && isDifferent(data[bindingIndex], value); if (changed) { + throwErrorIfNoChangesMode(data[bindingIndex], value); data[bindingIndex] = value; } bindingIndex++; @@ -1841,14 +1886,17 @@ export function consumeBinding(): any { export function bindingUpdated(value: any): boolean { ngDevMode && assertNotEqual(value, NO_CHANGE, 'Incoming value should never be NO_CHANGE.'); - if (creationMode || isDifferent(data[bindingIndex], value)) { - creationMode && initBindings(); - data[bindingIndex++] = value; - return true; + if (creationMode) { + initBindings(); + } else if (isDifferent(data[bindingIndex], value)) { + throwErrorIfNoChangesMode(data[bindingIndex], value); } else { bindingIndex++; return false; } + + data[bindingIndex++] = value; + return true; } /** Updates binding if changed, then returns the latest value. */ diff --git a/packages/core/src/render3/view_ref.ts b/packages/core/src/render3/view_ref.ts index 3b52c44554..b98a688fa4 100644 --- a/packages/core/src/render3/view_ref.ts +++ b/packages/core/src/render3/view_ref.ts @@ -8,7 +8,7 @@ import {EmbeddedViewRef as viewEngine_EmbeddedViewRef} from '../linker/view_ref'; -import {detectChanges, markViewDirty} from './instructions'; +import {checkNoChanges, detectChanges, markViewDirty} from './instructions'; import {ComponentTemplate} from './interfaces/definition'; import {LViewNode} from './interfaces/node'; import {LView, LViewFlags} from './interfaces/view'; @@ -195,7 +195,13 @@ export class ViewRef implements viewEngine_EmbeddedViewRef { */ detectChanges(): void { detectChanges(this.context); } - checkNoChanges(): void { notImplemented(); } + /** + * Checks the change detector and its children, and throws if any changes are detected. + * + * This is used in development mode to verify that running change detection doesn't + * introduce other changes. + */ + checkNoChanges(): void { checkNoChanges(this.context); } } 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 1ddc1cde4f..6152cabb9f 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -35,6 +35,9 @@ { "name": "canInsertNativeNode" }, + { + "name": "checkNoChangesMode" + }, { "name": "createLNode" }, diff --git a/packages/core/test/render3/change_detection_spec.ts b/packages/core/test/render3/change_detection_spec.ts index 281b604663..b9d6e217db 100644 --- a/packages/core/test/render3/change_detection_spec.ts +++ b/packages/core/test/render3/change_detection_spec.ts @@ -911,6 +911,162 @@ describe('change detection', () => { // TODO(kara): add test for dynamic views once bug fix is in }); + describe('checkNoChanges', () => { + let comp: NoChangesComp; + + class NoChangesComp { + value = 1; + doCheckCount = 0; + contentCheckCount = 0; + viewCheckCount = 0; + + ngDoCheck() { this.doCheckCount++; } + + ngAfterContentChecked() { this.contentCheckCount++; } + + ngAfterViewChecked() { this.viewCheckCount++; } + + constructor(public cdr: ChangeDetectorRef) {} + + static ngComponentDef = defineComponent({ + type: NoChangesComp, + tag: 'no-changes-comp', + factory: () => comp = new NoChangesComp(injectChangeDetectorRef()), + template: (ctx: NoChangesComp, cm: boolean) => { + if (cm) { + text(0); + } + textBinding(0, bind(ctx.value)); + } + }); + } + + class AppComp { + value = 1; + + constructor(public cdr: ChangeDetectorRef) {} + + static ngComponentDef = defineComponent({ + type: AppComp, + tag: 'app-comp', + factory: () => new AppComp(injectChangeDetectorRef()), + /** + * {{ value }} - + * + */ + template: (ctx: AppComp, cm: boolean) => { + if (cm) { + text(0); + elementStart(1, NoChangesComp); + elementEnd(); + } + textBinding(0, interpolation1('', ctx.value, ' - ')); + NoChangesComp.ngComponentDef.h(2, 1); + directiveRefresh(2, 1); + } + }); + } + + it('should throw if bindings in current view have changed', () => { + const comp = renderComponent(NoChangesComp); + + expect(() => comp.cdr.checkNoChanges()).not.toThrow(); + + comp.value = 2; + expect(() => comp.cdr.checkNoChanges()) + .toThrowError( + /ExpressionChangedAfterItHasBeenCheckedError: .+ Previous value: '1'. Current value: '2'/gi); + }); + + it('should throw if interpolations in current view have changed', () => { + const app = renderComponent(AppComp); + + expect(() => app.cdr.checkNoChanges()).not.toThrow(); + + app.value = 2; + expect(() => app.cdr.checkNoChanges()) + .toThrowError( + /ExpressionChangedAfterItHasBeenCheckedError: .+ Previous value: '1'. Current value: '2'/gi); + }); + + it('should throw if bindings in children of current view have changed', () => { + const app = renderComponent(AppComp); + + expect(() => app.cdr.checkNoChanges()).not.toThrow(); + + comp.value = 2; + expect(() => app.cdr.checkNoChanges()) + .toThrowError( + /ExpressionChangedAfterItHasBeenCheckedError: .+ Previous value: '1'. Current value: '2'/gi); + }); + + it('should throw if bindings in embedded view have changed', () => { + class EmbeddedViewApp { + value = 1; + showing = true; + + constructor(public cdr: ChangeDetectorRef) {} + + static ngComponentDef = defineComponent({ + type: EmbeddedViewApp, + tag: 'embedded-view-app', + factory: () => new EmbeddedViewApp(injectChangeDetectorRef()), + /** + * % if (showing) { + * {{ value }} + * %} + */ + template: (ctx: EmbeddedViewApp, cm: boolean) => { + if (cm) { + container(0); + } + containerRefreshStart(0); + { + if (ctx.showing) { + if (embeddedViewStart(0)) { + text(0); + } + textBinding(0, bind(ctx.value)); + embeddedViewEnd(); + } + } + containerRefreshEnd(); + } + }); + } + + const app = renderComponent(EmbeddedViewApp); + + expect(() => app.cdr.checkNoChanges()).not.toThrow(); + + app.value = 2; + expect(() => app.cdr.checkNoChanges()) + .toThrowError( + /ExpressionChangedAfterItHasBeenCheckedError: .+ Previous value: '1'. Current value: '2'/gi); + }); + + it('should NOT call lifecycle hooks', () => { + const app = renderComponent(AppComp); + expect(comp.doCheckCount).toEqual(1); + expect(comp.contentCheckCount).toEqual(1); + expect(comp.viewCheckCount).toEqual(1); + + comp.value = 2; + expect(() => app.cdr.checkNoChanges()).toThrow(); + expect(comp.doCheckCount).toEqual(1); + expect(comp.contentCheckCount).toEqual(1); + expect(comp.viewCheckCount).toEqual(1); + }); + + it('should NOT throw if bindings in ancestors of current view have changed', () => { + const app = renderComponent(AppComp); + + app.value = 2; + expect(() => comp.cdr.checkNoChanges()).not.toThrow(); + }); + + }); + }); });