fix(select): avoid going into infinite loop under certain conditions (#2955)

* fix(select): avoid going into infinite loop under certain conditions

Currently `md-select` calls `writeValue` recursively until the `QueryList` with all of the options is initialized. This usually means 1-2 iterations max, however in certain conditions (e.g. a sibling component throws an error on init) this may not happen and the browser would get thrown into an infinite loop.

This change switches to saving the attempted value assignments in a property and applying it after initialization.

Fixes #2950.

* fix: more elegant approach to initial value and add unit test

* fix: button not being removed after test

* chore: remove unnecessary expression
This commit is contained in:
Kristiyan Kostadinov
2017-02-09 20:46:47 +01:00
committed by tinayuangao
parent 93937e606e
commit 998a583c0a
3 changed files with 50 additions and 15 deletions
+2
View File
@@ -381,6 +381,8 @@ describe('MdDialog', () => {
expect(document.activeElement.id)
.toBe('dialog-trigger', 'Expected that the trigger was refocused after dialog close');
document.body.removeChild(button);
}));
});
+33 -1
View File
@@ -7,6 +7,7 @@ import {
ViewChild,
ViewChildren,
ChangeDetectionStrategy,
OnInit,
} from '@angular/core';
import {MdSelectModule} from './index';
import {OverlayContainer} from '../core/overlay/overlay-container';
@@ -34,6 +35,8 @@ describe('MdSelect', () => {
SelectWithChangeEvent,
CustomSelectAccessor,
CompWithCustomSelect,
SelectWithErrorSibling,
ThrowsErrorOnInit,
BasicSelectOnPush
],
providers: [
@@ -1239,6 +1242,14 @@ describe('MdSelect', () => {
});
}));
it('should not crash the browser when a sibling throws an error on init', async(() => {
// Note that this test can be considered successful if the error being thrown didn't
// end up crashing the testing setup altogether.
expect(() => {
TestBed.createComponent(SelectWithErrorSibling).detectChanges();
}).toThrowError(new RegExp('Oh no!', 'g'));
}));
});
describe('change event', () => {
@@ -1281,7 +1292,7 @@ describe('MdSelect', () => {
beforeEach(() => {
fixture = TestBed.createComponent(BasicSelectOnPush);
fixture.detectChanges();
trigger = fixture.debugElement.query(By.css('.md-select-trigger')).nativeElement;
trigger = fixture.debugElement.query(By.css('.mat-select-trigger')).nativeElement;
});
it('should update the trigger based on the value', () => {
@@ -1474,6 +1485,27 @@ class CompWithCustomSelect {
@ViewChild(CustomSelectAccessor) customAccessor: CustomSelectAccessor;
}
@Component({
selector: 'select-infinite-loop',
template: `
<md-select [(ngModel)]="value"></md-select>
<throws-error-on-init></throws-error-on-init>
`
})
class SelectWithErrorSibling {
value: string;
}
@Component({
selector: 'throws-error-on-init',
template: ''
})
export class ThrowsErrorOnInit implements OnInit {
ngOnInit() {
throw new Error('Oh no!');
}
}
@Component({
selector: 'basic-select-on-push',
changeDetection: ChangeDetectionStrategy.OnPush,
+15 -14
View File
@@ -25,6 +25,8 @@ import {ControlValueAccessor, NgControl} from '@angular/forms';
import {coerceBooleanProperty} from '../core/coercion/boolean-property';
import {ConnectedOverlayDirective} from '../core/overlay/overlay-directives';
import {ViewportRuler} from '../core/overlay/position/viewport-ruler';
import 'rxjs/add/operator/startWith';
/**
* The following style constants are necessary to save here in order
@@ -243,8 +245,8 @@ export class MdSelect implements AfterContentInit, ControlValueAccessor, OnDestr
ngAfterContentInit() {
this._initKeyManager();
this._resetOptions();
this._changeSubscription = this.options.changes.subscribe(() => {
this._changeSubscription = this.options.changes.startWith(null).subscribe(() => {
this._resetOptions();
if (this._control) {
@@ -257,8 +259,14 @@ export class MdSelect implements AfterContentInit, ControlValueAccessor, OnDestr
ngOnDestroy() {
this._dropSubscriptions();
this._changeSubscription.unsubscribe();
this._tabSubscription.unsubscribe();
if (this._changeSubscription) {
this._changeSubscription.unsubscribe();
}
if (this._tabSubscription) {
this._tabSubscription.unsubscribe();
}
}
/** Toggles the overlay panel open or closed. */
@@ -292,17 +300,10 @@ export class MdSelect implements AfterContentInit, ControlValueAccessor, OnDestr
* @param value New value to be written to the model.
*/
writeValue(value: any): void {
if (!this.options) {
// In reactive forms, writeValue() will be called synchronously before
// the select's child options have been created. It's necessary to call
// writeValue() again after the options have been created to ensure any
// initial view value is set.
Promise.resolve(null).then(() => this.writeValue(value));
return;
if (this.options) {
this._setSelectionByValue(value);
this._changeDetectorRef.markForCheck();
}
this._setSelectionByValue(value);
this._changeDetectorRef.markForCheck();
}
/**