From 4456e7e4dec87f237cee0ed2eb6eb4431e806ea5 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Mon, 18 May 2020 11:08:23 -0700 Subject: [PATCH] refactor(core): remove `looseIdentical` in favor of built-in `Object.is` (#37191) Remove `looseIdentical` implementation and instead use the ES2015 `Object.is` in its place. They behave exactly the same way except for `+0`/`-0`. `looseIdentical(+0, -0)` => `true` `Object.is(+0, -0)` => `false` Other than the difference noted above, this is not be a breaking change because: 1. `looseIdentical` is a private API 2. ES2015 is listed as a mandatory polyfill in the [browser support guide](https://angular.io/guide/browser-support#mandatory-polyfills) 3. Also note that `Ivy` already uses `Object.is` in `bindingUpdated`. PR Close #37191 --- .../src/change_detection/change_detection_util.ts | 3 +-- .../differs/default_iterable_differ.ts | 15 +++++++-------- .../differs/default_keyvalue_differ.ts | 3 +-- packages/core/src/core_private_export.ts | 1 - packages/core/src/util/comparison.ts | 8 +------- packages/core/src/view/util.ts | 3 +-- .../test/bundling/todo/bundle.golden_symbols.json | 3 --- packages/core/test/change_detection/util.ts | 3 +-- .../directives/select_control_value_accessor.ts | 4 ++-- .../select_multiple_control_value_accessor.ts | 4 ++-- packages/forms/src/directives/shared.ts | 4 ++-- packages/upgrade/static/src/upgrade_component.ts | 4 ++-- 12 files changed, 20 insertions(+), 35 deletions(-) diff --git a/packages/core/src/change_detection/change_detection_util.ts b/packages/core/src/change_detection/change_detection_util.ts index 20b14f4235..25ab4fd4ab 100644 --- a/packages/core/src/change_detection/change_detection_util.ts +++ b/packages/core/src/change_detection/change_detection_util.ts @@ -6,7 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ -import {looseIdentical} from '../util/comparison'; import {getSymbolIterator} from '../util/symbol'; export function devModeEqual(a: any, b: any): boolean { @@ -20,7 +19,7 @@ export function devModeEqual(a: any, b: any): boolean { if (!isListLikeIterableA && isAObject && !isListLikeIterableB && isBObject) { return true; } else { - return looseIdentical(a, b); + return Object.is(a, b); } } } diff --git a/packages/core/src/change_detection/differs/default_iterable_differ.ts b/packages/core/src/change_detection/differs/default_iterable_differ.ts index 358b4256f3..736d7b1234 100644 --- a/packages/core/src/change_detection/differs/default_iterable_differ.ts +++ b/packages/core/src/change_detection/differs/default_iterable_differ.ts @@ -6,7 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ -import {looseIdentical} from '../../util/comparison'; import {stringify} from '../../util/stringify'; import {isListLikeIterable, iterateListLike} from '../change_detection_util'; @@ -180,7 +179,7 @@ export class DefaultIterableDiffer implements IterableDiffer, IterableChan for (let index = 0; index < this.length; index++) { item = collection[index]; itemTrackBy = this._trackByFn(index, item); - if (record === null || !looseIdentical(record.trackById, itemTrackBy)) { + if (record === null || !Object.is(record.trackById, itemTrackBy)) { record = this._mismatch(record, item, itemTrackBy, index); mayBeDirty = true; } else { @@ -188,7 +187,7 @@ export class DefaultIterableDiffer implements IterableDiffer, IterableChan // TODO(misko): can we limit this to duplicates only? record = this._verifyReinsertion(record, item, itemTrackBy, index); } - if (!looseIdentical(record.item, item)) this._addIdentityChange(record, item); + if (!Object.is(record.item, item)) this._addIdentityChange(record, item); } record = record._next; @@ -197,7 +196,7 @@ export class DefaultIterableDiffer implements IterableDiffer, IterableChan index = 0; iterateListLike(collection, (item: V) => { itemTrackBy = this._trackByFn(index, item); - if (record === null || !looseIdentical(record.trackById, itemTrackBy)) { + if (record === null || !Object.is(record.trackById, itemTrackBy)) { record = this._mismatch(record, item, itemTrackBy, index); mayBeDirty = true; } else { @@ -205,7 +204,7 @@ export class DefaultIterableDiffer implements IterableDiffer, IterableChan // TODO(misko): can we limit this to duplicates only? record = this._verifyReinsertion(record, item, itemTrackBy, index); } - if (!looseIdentical(record.item, item)) this._addIdentityChange(record, item); + if (!Object.is(record.item, item)) this._addIdentityChange(record, item); } record = record._next; index++; @@ -289,7 +288,7 @@ export class DefaultIterableDiffer implements IterableDiffer, IterableChan if (record !== null) { // We have seen this before, we need to move it forward in the collection. // But first we need to check if identity changed, so we can update in view if necessary - if (!looseIdentical(record.item, item)) this._addIdentityChange(record, item); + if (!Object.is(record.item, item)) this._addIdentityChange(record, item); this._moveAfter(record, previousRecord, index); } else { @@ -298,7 +297,7 @@ export class DefaultIterableDiffer implements IterableDiffer, IterableChan if (record !== null) { // It is an item which we have evicted earlier: reinsert it back into the list. // But first we need to check if identity changed, so we can update in view if necessary - if (!looseIdentical(record.item, item)) this._addIdentityChange(record, item); + if (!Object.is(record.item, item)) this._addIdentityChange(record, item); this._reinsertAfter(record, previousRecord, index); } else { @@ -628,7 +627,7 @@ class _DuplicateItemRecordList { let record: IterableChangeRecord_|null; for (record = this._head; record !== null; record = record._nextDup) { if ((atOrAfterIndex === null || atOrAfterIndex <= record.currentIndex!) && - looseIdentical(record.trackById, trackById)) { + Object.is(record.trackById, trackById)) { return record; } } diff --git a/packages/core/src/change_detection/differs/default_keyvalue_differ.ts b/packages/core/src/change_detection/differs/default_keyvalue_differ.ts index 33b82a5d4a..c99b2c7608 100644 --- a/packages/core/src/change_detection/differs/default_keyvalue_differ.ts +++ b/packages/core/src/change_detection/differs/default_keyvalue_differ.ts @@ -6,7 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ -import {looseIdentical} from '../../util/comparison'; import {stringify} from '../../util/stringify'; import {isJsObject} from '../change_detection_util'; import {KeyValueChangeRecord, KeyValueChanges, KeyValueDiffer, KeyValueDifferFactory} from './keyvalue_differs'; @@ -229,7 +228,7 @@ export class DefaultKeyValueDiffer implements KeyValueDiffer, KeyVal // Add the record or a given key to the list of changes only when the value has actually changed private _maybeAddToChanges(record: KeyValueChangeRecord_, newValue: any): void { - if (!looseIdentical(newValue, record.currentValue)) { + if (!Object.is(newValue, record.currentValue)) { record.previousValue = record.currentValue; record.currentValue = newValue; this._addToChanges(record); diff --git a/packages/core/src/core_private_export.ts b/packages/core/src/core_private_export.ts index ae5beedcdb..93b0aa2c65 100644 --- a/packages/core/src/core_private_export.ts +++ b/packages/core/src/core_private_export.ts @@ -27,7 +27,6 @@ export {GetterFn as ɵGetterFn, MethodFn as ɵMethodFn, SetterFn as ɵSetterFn} export {allowSanitizationBypassAndThrow as ɵallowSanitizationBypassAndThrow, BypassType as ɵBypassType, getSanitizationBypassType as ɵgetSanitizationBypassType, SafeHtml as ɵSafeHtml, SafeResourceUrl as ɵSafeResourceUrl, SafeScript as ɵSafeScript, SafeStyle as ɵSafeStyle, SafeUrl as ɵSafeUrl, SafeValue as ɵSafeValue, unwrapSafeValue as ɵunwrapSafeValue} from './sanitization/bypass'; export {_sanitizeHtml as ɵ_sanitizeHtml} from './sanitization/html_sanitizer'; export {_sanitizeUrl as ɵ_sanitizeUrl} from './sanitization/url_sanitizer'; -export {looseIdentical as ɵlooseIdentical,} from './util/comparison'; export {makeDecorator as ɵmakeDecorator} from './util/decorators'; export {global as ɵglobal} from './util/global'; export {isObservable as ɵisObservable, isPromise as ɵisPromise} from './util/lang'; diff --git a/packages/core/src/util/comparison.ts b/packages/core/src/util/comparison.ts index ade6d71712..dcdc7bb477 100644 --- a/packages/core/src/util/comparison.ts +++ b/packages/core/src/util/comparison.ts @@ -8,12 +8,6 @@ import {areIterablesEqual, isListLikeIterable} from './iterable'; - -// JS has NaN !== NaN -export function looseIdentical(a: any, b: any): boolean { - return a === b || typeof a === 'number' && typeof b === 'number' && isNaN(a) && isNaN(b); -} - export function devModeEqual(a: any, b: any): boolean { const isListLikeIterableA = isListLikeIterable(a); const isListLikeIterableB = isListLikeIterable(b); @@ -25,7 +19,7 @@ export function devModeEqual(a: any, b: any): boolean { if (!isListLikeIterableA && isAObject && !isListLikeIterableB && isBObject) { return true; } else { - return looseIdentical(a, b); + return Object.is(a, b); } } } diff --git a/packages/core/src/view/util.ts b/packages/core/src/view/util.ts index 4c5ae49578..9d5b52b5ba 100644 --- a/packages/core/src/view/util.ts +++ b/packages/core/src/view/util.ts @@ -10,7 +10,6 @@ import {devModeEqual, WrappedValue} from '../change_detection/change_detection'; import {SOURCE} from '../di/injector_compatibility'; import {ViewEncapsulation} from '../metadata/view'; import {RendererType2} from '../render/api'; -import {looseIdentical} from '../util/comparison'; import {stringify} from '../util/stringify'; import {expressionChangedAfterItHasBeenCheckedError} from './errors'; @@ -81,7 +80,7 @@ export function checkBinding( view: ViewData, def: NodeDef, bindingIdx: number, value: any): boolean { const oldValues = view.oldValues; if ((view.state & ViewState.FirstCheck) || - !looseIdentical(oldValues[def.bindingIndex + bindingIdx], value)) { + !Object.is(oldValues[def.bindingIndex + bindingIdx], value)) { return true; } return false; diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 3328973514..b2d427c6b6 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -941,9 +941,6 @@ { "name": "locateHostElement" }, - { - "name": "looseIdentical" - }, { "name": "makeMetadataCtor" }, diff --git a/packages/core/test/change_detection/util.ts b/packages/core/test/change_detection/util.ts index 4d37cec5fd..8e351957da 100644 --- a/packages/core/test/change_detection/util.ts +++ b/packages/core/test/change_detection/util.ts @@ -9,7 +9,6 @@ import {IterableChangeRecord, IterableChanges} from '@angular/core/src/change_detection/differs/iterable_differs'; import {KeyValueChangeRecord, KeyValueChanges} from '@angular/core/src/change_detection/differs/keyvalue_differs'; -import {looseIdentical} from '../../src/util/comparison'; import {stringify} from '../../src/util/stringify'; export function iterableDifferToString(iterableChanges: IterableChanges) { @@ -64,7 +63,7 @@ export function iterableChangesAsString({ } function kvcrAsString(kvcr: KeyValueChangeRecord) { - return looseIdentical(kvcr.previousValue, kvcr.currentValue) ? + return Object.is(kvcr.previousValue, kvcr.currentValue) ? stringify(kvcr.key) : (stringify(kvcr.key) + '[' + stringify(kvcr.previousValue) + '->' + stringify(kvcr.currentValue) + ']'); diff --git a/packages/forms/src/directives/select_control_value_accessor.ts b/packages/forms/src/directives/select_control_value_accessor.ts index a54613c957..4e2832c2a5 100644 --- a/packages/forms/src/directives/select_control_value_accessor.ts +++ b/packages/forms/src/directives/select_control_value_accessor.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Directive, ElementRef, forwardRef, Host, Input, OnDestroy, Optional, Renderer2, StaticProvider, ɵlooseIdentical as looseIdentical} from '@angular/core'; +import {Directive, ElementRef, forwardRef, Host, Input, OnDestroy, Optional, Renderer2, StaticProvider} from '@angular/core'; import {ControlValueAccessor, NG_VALUE_ACCESSOR} from './control_value_accessor'; @@ -121,7 +121,7 @@ export class SelectControlValueAccessor implements ControlValueAccessor { this._compareWith = fn; } - private _compareWith: (o1: any, o2: any) => boolean = looseIdentical; + private _compareWith: (o1: any, o2: any) => boolean = Object.is; constructor(private _renderer: Renderer2, private _elementRef: ElementRef) {} diff --git a/packages/forms/src/directives/select_multiple_control_value_accessor.ts b/packages/forms/src/directives/select_multiple_control_value_accessor.ts index cd9512168d..c7201bdece 100644 --- a/packages/forms/src/directives/select_multiple_control_value_accessor.ts +++ b/packages/forms/src/directives/select_multiple_control_value_accessor.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Directive, ElementRef, forwardRef, Host, Input, OnDestroy, Optional, Renderer2, StaticProvider, ɵlooseIdentical as looseIdentical} from '@angular/core'; +import {Directive, ElementRef, forwardRef, Host, Input, OnDestroy, Optional, Renderer2, StaticProvider} from '@angular/core'; import {ControlValueAccessor, NG_VALUE_ACCESSOR} from './control_value_accessor'; @@ -118,7 +118,7 @@ export class SelectMultipleControlValueAccessor implements ControlValueAccessor this._compareWith = fn; } - private _compareWith: (o1: any, o2: any) => boolean = looseIdentical; + private _compareWith: (o1: any, o2: any) => boolean = Object.is; constructor(private _renderer: Renderer2, private _elementRef: ElementRef) {} diff --git a/packages/forms/src/directives/shared.ts b/packages/forms/src/directives/shared.ts index 0abddd5a80..51c9182bae 100644 --- a/packages/forms/src/directives/shared.ts +++ b/packages/forms/src/directives/shared.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {isDevMode, ɵlooseIdentical as looseIdentical} from '@angular/core'; +import {isDevMode} from '@angular/core'; import {FormArray, FormControl, FormGroup} from '../model'; import {Validators} from '../validators'; @@ -156,7 +156,7 @@ export function isPropertyUpdated(changes: {[key: string]: any}, viewModel: any) const change = changes['model']; if (change.isFirstChange()) return true; - return !looseIdentical(viewModel, change.currentValue); + return !Object.is(viewModel, change.currentValue); } const BUILTIN_ACCESSORS = [ diff --git a/packages/upgrade/static/src/upgrade_component.ts b/packages/upgrade/static/src/upgrade_component.ts index d61637d438..921800dbe9 100644 --- a/packages/upgrade/static/src/upgrade_component.ts +++ b/packages/upgrade/static/src/upgrade_component.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Directive, DoCheck, ElementRef, EventEmitter, Injector, OnChanges, OnDestroy, OnInit, SimpleChanges, ɵlooseIdentical as looseIdentical} from '@angular/core'; +import {Directive, DoCheck, ElementRef, EventEmitter, Injector, OnChanges, OnDestroy, OnInit, SimpleChanges} from '@angular/core'; import {IAttributes, IAugmentedJQuery, IDirective, IInjectorService, ILinkFn, IScope, ITranscludeFunction} from '../../src/common/src/angular1'; import {$SCOPE} from '../../src/common/src/constants'; @@ -206,7 +206,7 @@ export class UpgradeComponent implements OnInit, OnChanges, DoCheck, OnDestroy { const newValue = this.bindingDestination[propName]; const oldValue = twoWayBoundLastValues[idx]; - if (!looseIdentical(newValue, oldValue)) { + if (!Object.is(newValue, oldValue)) { const outputName = propertyToOutputMap[propName]; const eventEmitter: EventEmitter = (this as any)[outputName];