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

Author: crisbeto

src/lib/select/select.spec.ts

@@ -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';
@@ -26,7 +26,9 @@ describe('MdSelect', () => {
         SelectInitWithoutOptions,
         SelectWithChangeEvent,
         CustomSelectAccessor,
-        CompWithCustomSelect
+        CompWithCustomSelect,
+        SelectWithErrorSibling,
+        ThrowsErrorOnInit,
       ],
       providers: [
         {provide: OverlayContainer, useFactory: () => {
@@ -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', () => {
@@ -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

src/lib/select/select.ts

@@ -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
@@ -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 = '';
 
@@ -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) {
@@ -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. */
@@ -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;
     }
   }