From 49be32c9315a5c7ff155c5b88191e504006f595b Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Tue, 14 Apr 2020 18:52:32 -0700 Subject: [PATCH] refactor(common): remove WrappedValue from AsyncPipe (#36633) `AsyncPipe` only uses `WrappedValue` when the latest value from the `Promise` or `Observable` is different from the previous one. This is already enough to trigger change detection so the `WrappedValue` is not necessary. Fixes #29927 BREAKING CHANGE: This change could result in ExpressionChangedAfterItHasBeenChecked errors that were not detected before. The error could previously have gone undetected because two WrappedValues are considered "equal" in all cases for the purposes of the check, even if their respective unwrapped values are not. Additionally, `[val]=(observable | async).someProperty` will no longer trigger change detection if the value of `someProperty` is identical to the value in the previous emit. If you need to force change detection, either update the binding to use an object whose reference changes or subscribe to the observable and call markForCheck as needed. PR Close #36633 --- packages/common/src/pipes/async_pipe.ts | 12 ++---------- packages/common/test/pipes/async_pipe_spec.ts | 18 ++++++++---------- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/packages/common/src/pipes/async_pipe.ts b/packages/common/src/pipes/async_pipe.ts index 54a62d6903..2d4e44a229 100644 --- a/packages/common/src/pipes/async_pipe.ts +++ b/packages/common/src/pipes/async_pipe.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ChangeDetectorRef, EventEmitter, OnDestroy, Pipe, PipeTransform, WrappedValue, ɵisObservable, ɵisPromise, ɵlooseIdentical} from '@angular/core'; +import {ChangeDetectorRef, EventEmitter, OnDestroy, Pipe, PipeTransform, ɵisObservable, ɵisPromise} from '@angular/core'; import {Observable, SubscriptionLike} from 'rxjs'; import {invalidPipeArgumentError} from './invalid_pipe_argument_error'; @@ -81,7 +81,6 @@ const _observableStrategy = new ObservableStrategy(); @Pipe({name: 'async', pure: false}) export class AsyncPipe implements OnDestroy, PipeTransform { private _latestValue: any = null; - private _latestReturnedValue: any = null; private _subscription: SubscriptionLike|Promise|null = null; private _obj: Observable|Promise|EventEmitter|null = null; @@ -104,7 +103,6 @@ export class AsyncPipe implements OnDestroy, PipeTransform { if (obj) { this._subscribe(obj); } - this._latestReturnedValue = this._latestValue; return this._latestValue; } @@ -113,12 +111,7 @@ export class AsyncPipe implements OnDestroy, PipeTransform { return this.transform(obj as any); } - if (ɵlooseIdentical(this._latestValue, this._latestReturnedValue)) { - return this._latestReturnedValue; - } - - this._latestReturnedValue = this._latestValue; - return WrappedValue.wrap(this._latestValue); + return this._latestValue; } private _subscribe(obj: Observable|Promise|EventEmitter): void { @@ -143,7 +136,6 @@ export class AsyncPipe implements OnDestroy, PipeTransform { private _dispose(): void { this._strategy.dispose(this._subscription!); this._latestValue = null; - this._latestReturnedValue = null; this._subscription = null; this._obj = null; } diff --git a/packages/common/test/pipes/async_pipe_spec.ts b/packages/common/test/pipes/async_pipe_spec.ts index 0d1635938c..5d53c6b76e 100644 --- a/packages/common/test/pipes/async_pipe_spec.ts +++ b/packages/common/test/pipes/async_pipe_spec.ts @@ -7,7 +7,7 @@ */ import {AsyncPipe, ɵgetDOM as getDOM} from '@angular/common'; -import {EventEmitter, WrappedValue} from '@angular/core'; +import {EventEmitter} from '@angular/core'; import {AsyncTestCompleter, beforeEach, describe, expect, inject, it} from '@angular/core/testing/src/testing_internal'; import {browserDetection} from '@angular/platform-browser/testing/src/browser_util'; @@ -32,13 +32,13 @@ import {SpyChangeDetectorRef} from '../spies'; expect(pipe.transform(emitter)).toBe(null); }); - it('should return the latest available value wrapped', + it('should return the latest available value', inject([AsyncTestCompleter], (async: AsyncTestCompleter) => { pipe.transform(emitter); emitter.emit(message); setTimeout(() => { - expect(pipe.transform(emitter)).toEqual(new WrappedValue(message)); + expect(pipe.transform(emitter)).toEqual(message); async.done(); }, 0); })); @@ -82,15 +82,14 @@ import {SpyChangeDetectorRef} from '../spies'; }, 10); })); - it('should return unwrapped value for unchanged NaN', () => { + it('should return value for unchanged NaN', () => { const emitter = new EventEmitter(); emitter.emit(null); pipe.transform(emitter); emitter.next(NaN); const firstResult = pipe.transform(emitter); const secondResult = pipe.transform(emitter); - expect(firstResult instanceof WrappedValue).toBe(true); - expect((firstResult as WrappedValue).wrapped).toBeNaN(); + expect(firstResult).toBeNaN(); expect(secondResult).toBeNaN(); }); }); @@ -145,12 +144,12 @@ import {SpyChangeDetectorRef} from '../spies'; resolve(message); setTimeout(() => { - expect(pipe.transform(promise)).toEqual(new WrappedValue(message)); + expect(pipe.transform(promise)).toEqual(message); async.done(); }, timer); })); - it('should return unwrapped value when nothing has changed since the last call', + it('should return value when nothing has changed since the last call', inject([AsyncTestCompleter], (async: AsyncTestCompleter) => { pipe.transform(promise); resolve(message); @@ -169,7 +168,6 @@ import {SpyChangeDetectorRef} from '../spies'; promise = new Promise(() => {}); expect(pipe.transform(promise)).toBe(null); - // this should not affect the pipe, so it should return WrappedValue resolve(message); setTimeout(() => { @@ -203,7 +201,7 @@ import {SpyChangeDetectorRef} from '../spies'; setTimeout(() => { - expect(pipe.transform(promise)).toEqual(new WrappedValue(message)); + expect(pipe.transform(promise)).toEqual(message); pipe.ngOnDestroy(); expect(pipe.transform(promise)).toBe(null); async.done();