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
This commit is contained in:
Andrew Scott 2020-04-14 18:52:32 -07:00 committed by Alex Rickabaugh
parent 1786586747
commit 49be32c931
2 changed files with 10 additions and 20 deletions

View File

@ -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<any>|null = null;
private _obj: Observable<any>|Promise<any>|EventEmitter<any>|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<any>|Promise<any>|EventEmitter<any>): 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;
}

View File

@ -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<any>();
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<any>(() => {});
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();