From b38931b48476bae751eae8905df139a7d170ca21 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Wed, 1 Aug 2018 10:57:16 -0700 Subject: [PATCH] fix(ivy): use devModeEqual in no change mode (#25252) To avoid the unfamous error `Expression has changed after it was checked.` PR Close #25252 --- packages/core/src/render3/instructions.ts | 2 +- packages/core/src/render3/util.ts | 13 ++-- .../bundling/todo/bundle.golden_symbols.json | 2 +- packages/core/test/render3/util_spec.ts | 63 ++++++++++++++----- 4 files changed, 59 insertions(+), 21 deletions(-) diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index deb7709f29..f0b7be0af2 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -2739,7 +2739,7 @@ export function bindingUpdated(value: any): boolean { if (bindingIndex >= viewData.length) { viewData[viewData[BINDING_INDEX]++] = value; - } else if (isDifferent(viewData[bindingIndex], value)) { + } else if (isDifferent(viewData[bindingIndex], value, checkNoChangesMode)) { throwErrorIfNoChangesMode(creationMode, checkNoChangesMode, viewData[bindingIndex], value); viewData[viewData[BINDING_INDEX]++] = value; } else { diff --git a/packages/core/src/render3/util.ts b/packages/core/src/render3/util.ts index 22e73caeb2..784cc69dc4 100644 --- a/packages/core/src/render3/util.ts +++ b/packages/core/src/render3/util.ts @@ -5,15 +5,20 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import {devModeEqual} from '../change_detection/change_detection'; import {assertLessThan} from './assert'; import {LElementNode} from './interfaces/node'; import {HEADER_OFFSET, LViewData} from './interfaces/view'; - /** -* Must use this method for CD (instead of === ) since NaN !== NaN -*/ -export function isDifferent(a: any, b: any): boolean { + * Returns wether the values are different from a change detection stand point. + * + * Constraints are relaxed in checkNoChanges mode. See `devModeEqual` for details. + */ +export function isDifferent(a: any, b: any, checkNoChangesMode: boolean): boolean { + if (ngDevMode && checkNoChangesMode) { + return !devModeEqual(a, b); + } // NaN is the only value that is not equal to itself so the first // test checks if both a and b are not NaN return !(a !== a && b !== b) && a !== b; diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 757637d08b..70f606c52a 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -9,7 +9,7 @@ "name": "BLOOM_MASK" }, { - "name": "CIRCULAR$1" + "name": "CIRCULAR$2" }, { "name": "CLEANUP" diff --git a/packages/core/test/render3/util_spec.ts b/packages/core/test/render3/util_spec.ts index d591ff1267..02416e653d 100644 --- a/packages/core/test/render3/util_spec.ts +++ b/packages/core/test/render3/util_spec.ts @@ -12,25 +12,58 @@ describe('util', () => { describe('isDifferent', () => { - it('should mark non-equal arguments as different', () => { - expect(isDifferent({}, {})).toBeTruthy(); - expect(isDifferent('foo', 'bar')).toBeTruthy(); - expect(isDifferent(0, 1)).toBeTruthy(); + describe('checkNoChangeMode = false', () => { + it('should mark non-equal arguments as different', () => { + expect(isDifferent({}, {}, false)).toBeTruthy(); + expect(isDifferent('foo', 'bar', false)).toBeTruthy(); + expect(isDifferent(0, 1, false)).toBeTruthy(); + }); + + it('should not mark equal arguments as different', () => { + const obj = {}; + expect(isDifferent(obj, obj, false)).toBeFalsy(); + expect(isDifferent('foo', 'foo', false)).toBeFalsy(); + expect(isDifferent(1, 1, false)).toBeFalsy(); + }); + + it('should not mark NaN as different', + () => { expect(isDifferent(NaN, NaN, false)).toBeFalsy(); }); + + it('should mark NaN with other values as different', () => { + expect(isDifferent(NaN, 'foo', false)).toBeTruthy(); + expect(isDifferent(5, NaN, false)).toBeTruthy(); + }); }); - it('should not mark equal arguments as different', () => { - const obj = {}; - expect(isDifferent(obj, obj)).toBeFalsy(); - expect(isDifferent('foo', 'foo')).toBeFalsy(); - expect(isDifferent(1, 1)).toBeFalsy(); + describe('checkNoChangeMode = true', () => { + // Assert relaxed constraint in checkNoChangeMode + it('should not mark non-equal arrays, object and function as different', () => { + expect(isDifferent([], [], true)).toBeFalsy(); + expect(isDifferent(() => 0, () => 0, true)).toBeFalsy(); + expect(isDifferent({}, {}, true)).toBeFalsy(); + }); + + it('should mark non-equal arguments as different', () => { + expect(isDifferent('foo', 'bar', true)).toBeTruthy(); + expect(isDifferent(0, 1, true)).toBeTruthy(); + }); + + it('should not mark equal arguments as different', () => { + const obj = {}; + expect(isDifferent(obj, obj, false)).toBeFalsy(); + expect(isDifferent('foo', 'foo', false)).toBeFalsy(); + expect(isDifferent(1, 1, false)).toBeFalsy(); + }); + + it('should not mark NaN as different', + () => { expect(isDifferent(NaN, NaN, false)).toBeFalsy(); }); + + it('should mark NaN with other values as different', () => { + expect(isDifferent(NaN, 'foo', false)).toBeTruthy(); + expect(isDifferent(5, NaN, false)).toBeTruthy(); + }); }); - it('should not mark NaN as different', () => { expect(isDifferent(NaN, NaN)).toBeFalsy(); }); - - it('should mark NaN with other values as different', () => { - expect(isDifferent(NaN, 'foo')).toBeTruthy(); - expect(isDifferent(5, NaN)).toBeTruthy(); - }); }); describe('flatten', () => {