diff --git a/packages/core/src/render3/bindings.ts b/packages/core/src/render3/bindings.ts index 8ddbca9da7..f46d21ab2a 100644 --- a/packages/core/src/render3/bindings.ts +++ b/packages/core/src/render3/bindings.ts @@ -7,11 +7,10 @@ */ import {devModeEqual} from '../change_detection/change_detection_util'; - import {assertDataInRange, assertLessThan, assertNotEqual} from '../util/assert'; import {throwErrorIfNoChangesMode} from './errors'; -import {BINDING_INDEX, LView} from './interfaces/view'; -import {getCheckNoChangesMode, isCreationMode} from './state'; +import {LView} from './interfaces/view'; +import {getCheckNoChangesMode} from './state'; import {NO_CHANGE} from './tokens'; import {isDifferent} from './util'; @@ -38,20 +37,21 @@ export function bindingUpdated(lView: LView, bindingIndex: number, value: any): ngDevMode && assertLessThan(bindingIndex, lView.length, `Slot should have been initialized to NO_CHANGE`); - if (lView[bindingIndex] === NO_CHANGE) { - // initial pass - lView[bindingIndex] = value; - } else if (isDifferent(lView[bindingIndex], value)) { + const oldValue = lView[bindingIndex]; + if (isDifferent(oldValue, value)) { if (ngDevMode && getCheckNoChangesMode()) { - if (!devModeEqual(lView[bindingIndex], value)) { - throwErrorIfNoChangesMode(isCreationMode(lView), lView[bindingIndex], value); + // 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; + if (!devModeEqual(oldValueToCompare, value)) { + throwErrorIfNoChangesMode(oldValue === NO_CHANGE, oldValueToCompare, value); } } lView[bindingIndex] = value; - } else { - return false; + return true; } - return true; + + return false; } /** Updates 2 bindings if changed, then returns whether either was updated. */ diff --git a/packages/core/src/render3/util.ts b/packages/core/src/render3/util.ts index f05f3764ac..a90a6c3001 100644 --- a/packages/core/src/render3/util.ts +++ b/packages/core/src/render3/util.ts @@ -9,7 +9,7 @@ import {assertDataInRange, assertDefined, assertGreaterThan, assertLessThan} from '../util/assert'; import {global} from '../util/global'; -import {ACTIVE_INDEX, LCONTAINER_LENGTH, LContainer} from './interfaces/container'; +import {LCONTAINER_LENGTH, LContainer} from './interfaces/container'; import {LContext, MONKEY_PATCH_KEY_NAME} from './interfaces/context'; import {ComponentDef, DirectiveDef} from './interfaces/definition'; import {NO_PARENT_INJECTOR, RelativeInjectorLocation, RelativeInjectorLocationFlags} from './interfaces/injector'; diff --git a/packages/core/test/linker/change_detection_integration_spec.ts b/packages/core/test/linker/change_detection_integration_spec.ts index 6b036a4683..e8aa35ddad 100644 --- a/packages/core/test/linker/change_detection_integration_spec.ts +++ b/packages/core/test/linker/change_detection_integration_spec.ts @@ -13,7 +13,7 @@ import {ComponentFixture, TestBed, fakeAsync} from '@angular/core/testing'; import {By} from '@angular/platform-browser/src/dom/debug/by'; import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter'; import {expect} from '@angular/platform-browser/testing/src/matchers'; -import {fixmeIvy, ivyEnabled, modifiedInIvy, onlyInIvy} from '@angular/private/testing'; +import {fixmeIvy, ivyEnabled, modifiedInIvy} from '@angular/private/testing'; export function createUrlResolverWithoutPackagePrefix(): UrlResolver { return new UrlResolver(); @@ -1183,56 +1183,55 @@ const TEST_COMPILER_PROVIDERS: Provider[] = [ }); describe('enforce no new changes', () => { - modifiedInIvy('checkNoChanges has no effect before first change detection run') - .it('should throw when a record gets changed after it has been checked', fakeAsync(() => { - @Directive({selector: '[changed]'}) - class ChangingDirective { - @Input() changed: any; - } + it('should throw when a record gets changed after it has been checked', fakeAsync(() => { + @Directive({selector: '[changed]'}) + class ChangingDirective { + @Input() changed: any; + } - TestBed.configureTestingModule({declarations: [ChangingDirective]}); + TestBed.configureTestingModule({declarations: [ChangingDirective]}); - const ctx = createCompFixture('
', TestData); + const ctx = createCompFixture('
', TestData); - ctx.componentInstance.b = 1; - - expect(() => ctx.checkNoChanges()) - .toThrowError( - /Previous value: 'changed: undefined'\. Current value: 'changed: 1'/g); - })); + ctx.componentInstance.b = 1; + const errMsgRegExp = ivyEnabled ? + /Previous value: 'undefined'\. Current value: '1'/g : + /Previous value: 'changed: undefined'\. Current value: 'changed: 1'/g; + expect(() => ctx.checkNoChanges()).toThrowError(errMsgRegExp); + })); - onlyInIvy('checkNoChanges has no effect before first change detection run') - .it('should throw when a record gets changed after the first change detection pass', - fakeAsync(() => { - @Directive({selector: '[changed]'}) - class ChangingDirective { - @Input() changed: any; - } + it('should throw when a record gets changed after the first change detection pass', + fakeAsync(() => { + @Directive({selector: '[changed]'}) + class ChangingDirective { + @Input() changed: any; + } - TestBed.configureTestingModule({declarations: [ChangingDirective]}); + TestBed.configureTestingModule({declarations: [ChangingDirective]}); - const ctx = createCompFixture('
', TestData); + const ctx = createCompFixture('
', TestData); - ctx.componentInstance.b = 1; - // should not throw here as change detection didn't run yet - ctx.checkNoChanges(); + ctx.componentInstance.b = 1; + ctx.detectChanges(); - ctx.detectChanges(); + ctx.componentInstance.b = 2; + const errMsgRegExp = ivyEnabled ? + /Previous value: '1'\. Current value: '2'/g : + /Previous value: 'changed: 1'\. Current value: 'changed: 2'/g; + expect(() => ctx.checkNoChanges()).toThrowError(errMsgRegExp); + })); - ctx.componentInstance.b = 2; - expect(() => ctx.checkNoChanges()) - .toThrowError(/Previous value: '1'\. Current value: '2'/g); - })); + it('should warn when the view has been created in a cd hook', fakeAsync(() => { + const ctx = createCompFixture('
{{ a }}
', TestData); + ctx.componentInstance.a = 1; + expect(() => ctx.detectChanges()) + .toThrowError( + /It seems like the view has been created after its parent and its children have been dirty checked/); - fixmeIvy('FW-831: Views created in a cd hooks throw in view engine') - .it('should warn when the view has been created in a cd hook', fakeAsync(() => { - const ctx = createCompFixture('
{{ a }}
', TestData); - ctx.componentInstance.a = 1; - expect(() => ctx.detectChanges()) - .toThrowError( - /It seems like the view has been created after its parent and its children have been dirty checked/); - })); + // subsequent change detection should run without issues + ctx.detectChanges(); + })); it('should not throw when two arrays are structurally the same', fakeAsync(() => { const ctx = _bindSimpleValue('a', TestData); @@ -1242,15 +1241,14 @@ const TEST_COMPILER_PROVIDERS: Provider[] = [ expect(() => ctx.checkNoChanges()).not.toThrow(); })); - modifiedInIvy('checkNoChanges has no effect before first change detection run') - .it('should not break the next run', fakeAsync(() => { - const ctx = _bindSimpleValue('a', TestData); - ctx.componentInstance.a = 'value'; - expect(() => ctx.checkNoChanges()).toThrow(); + it('should not break the next run', fakeAsync(() => { + const ctx = _bindSimpleValue('a', TestData); + ctx.componentInstance.a = 'value'; + expect(() => ctx.checkNoChanges()).toThrow(); - ctx.detectChanges(); - expect(renderLog.loggedValues).toEqual(['value']); - })); + ctx.detectChanges(); + expect(renderLog.loggedValues).toEqual(['value']); + })); it('should not break the next run (view engine and ivy)', fakeAsync(() => { const ctx = _bindSimpleValue('a', TestData);