From 9b566e1e4af220c340a0074e7533f545f1b2119c Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 27 Jan 2018 17:01:40 +0100 Subject: [PATCH] fix(autocomplete): make writeValue method synchronous Refactors the `MatAutocompleteTrigger.writeValue` method to avoid having to use a `Promise.resolve` to defer rendering. This makes it easier to test and avoids potential race conditions. It seems like the reason it was added in the first place was to be able to handle components that have a preselected value through a `FormControl` as well as a custom `displayWith` function. Fixes #3250. --- src/lib/autocomplete/autocomplete-trigger.ts | 37 ++++++++++++++++---- src/lib/autocomplete/autocomplete.spec.ts | 37 ++++++++++++++++++++ 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/src/lib/autocomplete/autocomplete-trigger.ts b/src/lib/autocomplete/autocomplete-trigger.ts index 792943346d87..1723009f51bc 100644 --- a/src/lib/autocomplete/autocomplete-trigger.ts +++ b/src/lib/autocomplete/autocomplete-trigger.ts @@ -22,6 +22,7 @@ import {switchMap} from 'rxjs/operators/switchMap'; import {tap} from 'rxjs/operators/tap'; import {delay} from 'rxjs/operators/delay'; import { + AfterContentChecked, ChangeDetectorRef, Directive, ElementRef, @@ -115,11 +116,12 @@ export function getMatAutocompleteMissingPanelError(): Error { }, providers: [MAT_AUTOCOMPLETE_VALUE_ACCESSOR] }) -export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy { +export class MatAutocompleteTrigger implements ControlValueAccessor, AfterContentChecked, + OnDestroy { private _overlayRef: OverlayRef | null; private _portal: TemplatePortal; private _panelOpen: boolean = false; - private _componentDestroyed = false; + private _isDestroyed = false; /** Strategy that is used to position the panel. */ private _positionStrategy: ConnectedPositionStrategy; @@ -130,6 +132,12 @@ export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy { /** The subscription for closing actions (some are bound to document). */ private _closingActionsSubscription: Subscription; + /** Whether the component has been initializied. */ + private _isInitialized: boolean; + + /** Initial value that should be shown after the component is initialized. */ + private _initialValueToSelect: any; + /** Stream of keyboard events that can close the panel. */ private readonly _closeKeyEventStream = new Subject(); @@ -142,7 +150,8 @@ export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy { /** The autocomplete panel to be attached to this trigger. */ @Input('matAutocomplete') autocomplete: MatAutocomplete; - constructor(private _element: ElementRef, private _overlay: Overlay, + constructor(private _element: ElementRef, + private _overlay: Overlay, private _viewContainerRef: ViewContainerRef, private _zone: NgZone, private _changeDetectorRef: ChangeDetectorRef, @@ -151,8 +160,17 @@ export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy { @Optional() @Host() private _formField: MatFormField, @Optional() @Inject(DOCUMENT) private _document: any) {} + ngAfterContentChecked() { + if (!this._isInitialized && typeof this._initialValueToSelect !== 'undefined') { + this._setTriggerValue(this._initialValueToSelect); + this._initialValueToSelect = undefined; + } + + this._isInitialized = true; + } + ngOnDestroy() { - this._componentDestroyed = true; + this._isDestroyed = true; this._destroyPanel(); this._closeKeyEventStream.complete(); } @@ -182,7 +200,7 @@ export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy { // Note that in some cases this can end up being called after the component is destroyed. // Add a check to ensure that we don't try to run change detection on a destroyed view. - if (!this._componentDestroyed) { + if (!this._isDestroyed) { // We need to trigger change detection manually, because // `fromEvent` doesn't seem to do it at the proper time. // This ensures that the label is reset when the @@ -259,7 +277,14 @@ export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy { * @param value New value to be written to the model. */ writeValue(value: any): void { - Promise.resolve(null).then(() => this._setTriggerValue(value)); + if (this._isInitialized) { + this._setTriggerValue(value); + } else { + // If the component isn't initialized yet, defer until the first CD pass, otherwise we'll + // miss the initial `displayWith` value. By deferring until the first `AfterContentChecked` + // we avoid making the method async while preventing "changed after checked" errors. + this._initialValueToSelect = value; + } } /** diff --git a/src/lib/autocomplete/autocomplete.spec.ts b/src/lib/autocomplete/autocomplete.spec.ts index 02ddd3d65515..7ff17376f342 100644 --- a/src/lib/autocomplete/autocomplete.spec.ts +++ b/src/lib/autocomplete/autocomplete.spec.ts @@ -1760,6 +1760,17 @@ describe('MatAutocomplete', () => { expect(event.source).toBe(fixture.componentInstance.autocomplete); expect(event.option.value).toBe('Puerto Rico'); })); + + it('should evaluate `displayWith` before assigning the initial value', fakeAsync(() => { + const fixture = createComponent(PreselectedAutocompleteDisplayWith); + const input = fixture.nativeElement.querySelector('input'); + + fixture.detectChanges(); + flush(); + + expect(input.value).toBe('Alaska'); + })); + }); @Component({ @@ -2072,3 +2083,29 @@ class AutocompleteWithSelectEvent { class PlainAutocompleteInputWithFormControl { formControl = new FormControl(); } + + +@Component({ + template: ` + + + + + + + {{ state.name }} + + + ` +}) +class PreselectedAutocompleteDisplayWith { + stateCtrl = new FormControl({code: 'AK', name: 'Alaska'}); + states = [ + {code: 'AL', name: 'Alabama'}, + {code: 'AK', name: 'Alaska'} + ]; + + displayFn(value: any): string { + return value && typeof value === 'object' ? value.name : value; + } +}