fix(dynamic_component_loader): leave the view tree in a consistent state when hydration fails

vsavkin · jelbourn · commit 0df8bc4e5281 · 2015-12-08T16:09:38.000-08:00
Closes #5718
diff --git a/modules/angular2/src/core/application_ref.ts b/modules/angular2/src/core/application_ref.ts
@@ -1,5 +1,12 @@
 import {NgZone} from 'angular2/src/core/zone/ng_zone';
-import {Type, isBlank, isPresent, assertionsEnabled, print, IS_DART} from 'angular2/src/facade/lang';
+import {
+  Type,
+  isBlank,
+  isPresent,
+  assertionsEnabled,
+  print,
+  IS_DART
+} from 'angular2/src/facade/lang';
 import {provide, Provider, Injector, OpaqueToken} from 'angular2/src/core/di';
 import {
   APP_COMPONENT_REF_PROMISE,
diff --git a/modules/angular2/src/core/change_detection/abstract_change_detector.ts b/modules/angular2/src/core/change_detection/abstract_change_detector.ts
@@ -169,7 +169,7 @@ export class AbstractChangeDetector<T> implements ChangeDetector {
   // any work done in `hydrateDirectives`.
   dehydrateDirectives(destroyPipes: boolean): void {}
 
-  hydrated(): boolean { return this.context !== null; }
+  hydrated(): boolean { return isPresent(this.context); }
 
   afterContentLifecycleCallbacks(): void {
     this.dispatcher.notifyAfterContentChecked();
diff --git a/modules/angular2/src/core/linker/view_manager.ts b/modules/angular2/src/core/linker/view_manager.ts
@@ -319,9 +319,15 @@ export class AppViewManager_ extends AppViewManager {
     }
     this._utils.attachViewInContainer(parentView, boundElementIndex, contextView,
                                       contextBoundElementIndex, index, view);
-    this._utils.hydrateViewInContainer(parentView, boundElementIndex, contextView,
-                                       contextBoundElementIndex, index,
-                                       imperativelyCreatedInjector);
+
+    try {
+      this._utils.hydrateViewInContainer(parentView, boundElementIndex, contextView,
+                                         contextBoundElementIndex, index,
+                                         imperativelyCreatedInjector);
+    } catch (e) {
+      this._utils.detachViewInContainer(parentView, boundElementIndex, index);
+      throw e;
+    }
     return view.ref;
   }
 
diff --git a/modules/angular2/test/core/linker/dynamic_component_loader_spec.ts b/modules/angular2/test/core/linker/dynamic_component_loader_spec.ts
@@ -26,6 +26,8 @@ import {ElementRef} from 'angular2/src/core/linker/element_ref';
 import {DOCUMENT} from 'angular2/src/platform/dom/dom_tokens';
 import {DOM} from 'angular2/src/platform/dom/dom_adapter';
 import {ComponentFixture_} from "angular2/src/testing/test_component_builder";
+import {BaseException} from 'angular2/src/facade/exceptions';
+import {PromiseWrapper} from 'angular2/src/facade/promise';
 
 export function main() {
   describe('DynamicComponentLoader', function() {
@@ -124,6 +126,28 @@ export function main() {
                       });
                 }));
 
+      it('should leave the view tree in a consistent state if hydration fails',
+         inject([DynamicComponentLoader, TestComponentBuilder, AsyncTestCompleter],
+                (loader, tcb: TestComponentBuilder, async) => {
+                  tcb.overrideView(MyComp, new ViewMetadata({
+                                     template: '<div><location #loc></location></div>',
+                                     directives: [Location]
+                                   }))
+                      .createAsync(MyComp)
+                      .then((tc: ComponentFixture) => {
+                        tc.debugElement
+
+                            PromiseWrapper.catchError(
+                                loader.loadIntoLocation(DynamicallyLoadedThrows,
+                                                        tc.debugElement.elementRef, 'loc'),
+                                error => {
+                                  expect(error.message).toContain("ThrownInConstructor");
+                                  expect(() => tc.detectChanges()).not.toThrow();
+                                  async.done();
+                                });
+                      });
+                }));
+
       it('should throw if the variable does not exist',
          inject([DynamicComponentLoader, TestComponentBuilder, AsyncTestCompleter],
                 (loader, tcb: TestComponentBuilder, async) => {
@@ -223,6 +247,7 @@ export function main() {
                             });
                       });
                 }));
+
     });
 
     describe('loadAsRoot', () => {
@@ -291,6 +316,12 @@ class DynamicallyCreatedCmp implements OnDestroy {
 class DynamicallyLoaded {
 }
 
+@Component({selector: 'dummy'})
+@View({template: "DynamicallyLoaded;"})
+class DynamicallyLoadedThrows {
+  constructor() { throw new BaseException("ThrownInConstructor"); }
+}
+
 @Component({selector: 'dummy'})
 @View({template: "DynamicallyLoaded2;"})
 class DynamicallyLoaded2 {
