Skip to content

Commit

Permalink
fix: more elegant approach to initial value and add unit test
Browse files Browse the repository at this point in the history
  • Loading branch information
crisbeto committed Feb 7, 2017
1 parent a8cbd0a commit bd6f080
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 21 deletions.
36 changes: 34 additions & 2 deletions src/lib/select/select.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {TestBed, async, ComponentFixture, fakeAsync, tick} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
import {Component, DebugElement, QueryList, ViewChild, ViewChildren} from '@angular/core';
import {Component, DebugElement, QueryList, ViewChild, ViewChildren, OnInit} from '@angular/core';
import {MdSelectModule} from './index';
import {OverlayContainer} from '../core/overlay/overlay-container';
import {MdSelect} from './select';
Expand All @@ -26,7 +26,9 @@ describe('MdSelect', () => {
SelectInitWithoutOptions,
SelectWithChangeEvent,
CustomSelectAccessor,
CompWithCustomSelect
CompWithCustomSelect,
SelectWithErrorSibling,
ThrowsErrorOnInit,
],
providers: [
{provide: OverlayContainer, useFactory: () => {
Expand Down Expand Up @@ -1221,6 +1223,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', () => {
Expand Down Expand Up @@ -1433,6 +1443,28 @@ 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!');
}
}


/**
* TODO: Move this to core testing utility until Angular has event faking
Expand Down
30 changes: 11 additions & 19 deletions src/lib/select/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,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
Expand Down Expand Up @@ -123,9 +125,6 @@ export class MdSelect implements AfterContentInit, ControlValueAccessor, OnDestr
/** The placeholder displayed in the trigger of the select. */
private _placeholder: string;

/** Holds a value that was attempted to be assigned before the component was initialized. */
private _tempValue: any;

/** The animation state of the placeholder. */
_placeholderState = '';

Expand Down Expand Up @@ -246,15 +245,7 @@ export class MdSelect implements AfterContentInit, ControlValueAccessor, OnDestr
this._initKeyManager();
this._resetOptions();

// Assign any values that were deferred until the component is initialized.
if (this._tempValue) {
Promise.resolve(null).then(() => {
this.writeValue(this._tempValue);
this._tempValue = null;
});
}

this._changeSubscription = this.options.changes.subscribe(() => {
this._changeSubscription = this.options.changes.startWith(null).subscribe(() => {
this._resetOptions();

if (this._control) {
Expand All @@ -267,8 +258,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. */
Expand Down Expand Up @@ -304,11 +301,6 @@ export class MdSelect implements AfterContentInit, ControlValueAccessor, OnDestr
writeValue(value: any): void {
if (this.options) {
this._setSelectionByValue(value);
} else {
// In reactive forms, writeValue() will be called synchronously before
// the select's child options have been created. We save the value and
// assign it after everything is set up, in order to avoid lost data.
this._tempValue = value;
}
}

Expand Down

0 comments on commit bd6f080

Please sign in to comment.