fix(ivy): unify checkNoChanges logic with the view engine (#28366)

This commit unifies handling of the "check no changes" mode between
ngIvy and the view engine. More specifically:
- check no changes can be invoked before change detection in ivy;
- `undefined` values are considered equal `NO_CHANGES` for the "check no changes"
mode purposes.

Chanes in this commit enables several tests that were previously running only in ivy
or only in the view engine.

PR Close #28366

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 {LView} from './interfaces/view';
-import {getCheckNoChangesMode, isCreationMode} from './state';
+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) {
+  const oldValue = lView[bindingIndex];
-    // initial pass
+  if (isDifferent(oldValue, value)) {
-    lView[bindingIndex] = value;
-  } else if (isDifferent(lView[bindingIndex], value)) {
     if (ngDevMode && getCheckNoChangesMode()) {
-      if (!devModeEqual(lView[bindingIndex], value)) {
+      // View engine didn't report undefined values as changed on the first checkNoChanges pass
-        throwErrorIfNoChangesMode(isCreationMode(lView), lView[bindingIndex], value);
+      // (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 true;
-    return false;
   }
-  return true;
+
+  return false;
 }
 
 /** Updates 2 bindings if changed, then returns whether either was updated. */

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';

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(() => {
-          .it('should throw when a record gets changed after it has been checked', fakeAsync(() => {
+           @Directive({selector: '[changed]'})
-                @Directive({selector: '[changed]'})
+           class ChangingDirective {
-                class ChangingDirective {
+             @Input() changed: any;
-                  @Input() changed: any;
+           }
-                }
 
-                TestBed.configureTestingModule({declarations: [ChangingDirective]});
+           TestBed.configureTestingModule({declarations: [ChangingDirective]});
 
-                const ctx = createCompFixture('<div [someProp]="a" [changed]="b"></div>', TestData);
+           const ctx = createCompFixture('<div [someProp]="a" [changed]="b"></div>', TestData);
 
-                ctx.componentInstance.b = 1;
+           ctx.componentInstance.b = 1;
-
+           const errMsgRegExp = ivyEnabled ?
-                expect(() => ctx.checkNoChanges())
+               /Previous value: 'undefined'\. Current value: '1'/g :
-                    .toThrowError(
+               /Previous value: 'changed: undefined'\. Current value: 'changed: 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',
-          .it('should throw when a record gets changed after the first change detection pass',
+         fakeAsync(() => {
-              fakeAsync(() => {
+           @Directive({selector: '[changed]'})
-                @Directive({selector: '[changed]'})
+           class ChangingDirective {
-                class ChangingDirective {
+             @Input() changed: any;
-                  @Input() changed: any;
+           }
-                }
 
-                TestBed.configureTestingModule({declarations: [ChangingDirective]});
+           TestBed.configureTestingModule({declarations: [ChangingDirective]});
 
-                const ctx = createCompFixture('<div [someProp]="a" [changed]="b"></div>', TestData);
+           const ctx = createCompFixture('<div [someProp]="a" [changed]="b"></div>', TestData);
 
-                ctx.componentInstance.b = 1;
+           ctx.componentInstance.b = 1;
-                // should not throw here as change detection didn't run yet
+           ctx.detectChanges();
-                ctx.checkNoChanges();
 
-                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;
+      it('should warn when the view has been created in a cd hook', fakeAsync(() => {
-                expect(() => ctx.checkNoChanges())
+           const ctx = createCompFixture('<div *gh9882>{{ a }}</div>', TestData);
-                    .toThrowError(/Previous value: '1'\. Current value: '2'/g);
+           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')
+           // subsequent change detection should run without issues
-          .it('should warn when the view has been created in a cd hook', fakeAsync(() => {
+           ctx.detectChanges();
-                const ctx = createCompFixture('<div *gh9882>{{ a }}</div>', 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/);
-              }));
 
       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(() => {
-          .it('should not break the next run', fakeAsync(() => {
+           const ctx = _bindSimpleValue('a', TestData);
-                const ctx = _bindSimpleValue('a', TestData);
+           ctx.componentInstance.a = 'value';
-                ctx.componentInstance.a = 'value';
+           expect(() => ctx.checkNoChanges()).toThrow();
-                expect(() => ctx.checkNoChanges()).toThrow();
 
-                ctx.detectChanges();
+           ctx.detectChanges();
-                expect(renderLog.loggedValues).toEqual(['value']);
+           expect(renderLog.loggedValues).toEqual(['value']);
-              }));
+         }));
 
       it('should not break the next run (view engine and ivy)', fakeAsync(() => {
            const ctx = _bindSimpleValue('a', TestData);