Skip to content

Commit

Permalink
fix(autocomplete): make writeValue method synchronous
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
crisbeto authored and jelbourn committed Feb 12, 2018
1 parent 591d1f2 commit 8193dac
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 6 deletions.
37 changes: 31 additions & 6 deletions src/lib/autocomplete/autocomplete-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -121,10 +122,11 @@ export function getMatAutocompleteMissingPanelError(): Error {
exportAs: 'matAutocompleteTrigger',
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 _componentDestroyed = false;
private _isDestroyed = false;

/** Old value of the native input. Used to work around issues with the `input` event on IE. */
private _previousValue: string | number | null;
Expand All @@ -138,6 +140,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<void>();

Expand All @@ -150,7 +158,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,
Expand All @@ -159,8 +168,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();
}
Expand Down Expand Up @@ -189,7 +207,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
Expand Down Expand Up @@ -261,7 +279,14 @@ export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy {

// Implemented as part of ControlValueAccessor.
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;
}
}

// Implemented as part of ControlValueAccessor.
Expand Down
36 changes: 36 additions & 0 deletions src/lib/autocomplete/autocomplete.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1805,6 +1805,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({
Expand Down Expand Up @@ -2134,3 +2145,28 @@ class AutocompleteWithNumberInputAndNgModel {
selectedValue: number;
values = [1, 2, 3];
}

@Component({
template: `
<mat-form-field>
<input matInput [matAutocomplete]="auto" [formControl]="stateCtrl">
</mat-form-field>
<mat-autocomplete #auto="matAutocomplete" [displayWith]="displayFn">
<mat-option *ngFor="let state of states" [value]="state">
<span>{{ state.name }}</span>
</mat-option>
</mat-autocomplete>
`
})
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;
}
}

0 comments on commit 8193dac

Please sign in to comment.