Skip to content

Commit

Permalink
fix(material/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 angular#3250.
  • Loading branch information
crisbeto committed Mar 2, 2022
1 parent 175937e commit b9c748c
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 9 deletions.
34 changes: 34 additions & 0 deletions src/material-experimental/mdc-autocomplete/autocomplete.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3349,6 +3349,16 @@ describe('MDC-based MatAutocomplete', () => {

expect(fixture.componentInstance.trigger.panelOpen).toBe(true);
});

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');
}));
});

const SIMPLE_AUTOCOMPLETE_TEMPLATE = `
Expand Down Expand Up @@ -3770,3 +3780,27 @@ class AutocompleteWithActivatedEvent {
@ViewChild(MatAutocomplete) autocomplete: MatAutocomplete;
@ViewChildren(MatOption) options: QueryList<MatOption>;
}

@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;
}
}
35 changes: 27 additions & 8 deletions src/material/autocomplete/autocomplete-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {ViewportRuler} from '@angular/cdk/scrolling';
import {DOCUMENT} from '@angular/common';
import {
AfterViewInit,
AfterContentChecked,
ChangeDetectorRef,
Directive,
ElementRef,
Expand Down Expand Up @@ -98,7 +99,7 @@ export function getMatAutocompleteMissingPanelError(): Error {
/** Base class with all of the `MatAutocompleteTrigger` functionality. */
@Directive()
export abstract class _MatAutocompleteTriggerBase
implements ControlValueAccessor, AfterViewInit, OnChanges, OnDestroy
implements ControlValueAccessor, AfterViewInit, AfterContentChecked, OnChanges, OnDestroy
{
private _overlayRef: OverlayRef | null;
private _portal: TemplatePortal;
Expand Down Expand Up @@ -137,6 +138,12 @@ export abstract class _MatAutocompleteTriggerBase
*/
private _pendingAutoselectedOption: _MatOptionBase | null;

/** 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 Down Expand Up @@ -233,6 +240,15 @@ export abstract class _MatAutocompleteTriggerBase
}
}

ngAfterContentChecked() {
if (!this._isInitialized && typeof this._initialValueToSelect !== 'undefined') {
this._assignOptionValue(this._initialValueToSelect);
this._initialValueToSelect = undefined;
}

this._isInitialized = true;
}

ngOnDestroy() {
const window = this._getWindow();

Expand Down Expand Up @@ -380,7 +396,14 @@ export abstract class _MatAutocompleteTriggerBase

// Implemented as part of ControlValueAccessor.
writeValue(value: any): void {
Promise.resolve(null).then(() => this._assignOptionValue(value));
if (this._isInitialized) {
this._assignOptionValue(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 Expand Up @@ -576,12 +599,8 @@ export abstract class _MatAutocompleteTriggerBase
private _updateNativeInputValue(value: string): void {
// If it's used within a `MatFormField`, we should set it through the property so it can go
// through change detection.
if (this._formField) {
this._formField._control.value = value;
} else {
this._element.nativeElement.value = value;
}

const target = this._formField?._control || this._element.nativeElement;
target.value = value;
this._previousValue = value;
}

Expand Down
34 changes: 34 additions & 0 deletions src/material/autocomplete/autocomplete.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3355,6 +3355,16 @@ describe('MatAutocomplete', () => {

expect(fixture.componentInstance.trigger.panelOpen).toBe(true);
});

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');
}));
});

const SIMPLE_AUTOCOMPLETE_TEMPLATE = `
Expand Down Expand Up @@ -3790,3 +3800,27 @@ class AutocompleteWithActivatedEvent {
@ViewChild(MatAutocomplete) autocomplete: MatAutocomplete;
@ViewChildren(MatOption) options: QueryList<MatOption>;
}

@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;
}
}
5 changes: 4 additions & 1 deletion tools/public_api_guard/material/autocomplete.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import { _AbstractConstructor } from '@angular/material/core';
import { ActiveDescendantKeyManager } from '@angular/cdk/a11y';
import { AfterContentChecked } from '@angular/core';
import { AfterContentInit } from '@angular/core';
import { AfterViewInit } from '@angular/core';
import { BooleanInput } from '@angular/cdk/coercion';
Expand Down Expand Up @@ -190,7 +191,7 @@ export class MatAutocompleteTrigger extends _MatAutocompleteTriggerBase {
}

// @public
export abstract class _MatAutocompleteTriggerBase implements ControlValueAccessor, AfterViewInit, OnChanges, OnDestroy {
export abstract class _MatAutocompleteTriggerBase implements ControlValueAccessor, AfterViewInit, AfterContentChecked, OnChanges, OnDestroy {
constructor(_element: ElementRef<HTMLInputElement>, _overlay: Overlay, _viewContainerRef: ViewContainerRef, _zone: NgZone, _changeDetectorRef: ChangeDetectorRef, scrollStrategy: any, _dir: Directionality, _formField: MatFormField, _document: any, _viewportRuler: ViewportRuler, _defaults?: MatAutocompleteDefaultOptions | undefined);
protected abstract _aboveClass: string;
get activeOption(): _MatOptionBase | null;
Expand All @@ -209,6 +210,8 @@ export abstract class _MatAutocompleteTriggerBase implements ControlValueAccesso
// (undocumented)
_handleKeydown(event: KeyboardEvent): void;
// (undocumented)
ngAfterContentChecked(): void;
// (undocumented)
ngAfterViewInit(): void;
// (undocumented)
ngOnChanges(changes: SimpleChanges): void;
Expand Down

0 comments on commit b9c748c

Please sign in to comment.