fix(core): check components if an event handler inside of an embedded view fires.

tbosch · tbosch · commit 4d691b61eeb9 · 2016-04-29T18:30:29.000-07:00
BREAKING CHANGE: - ViewRef.changeDetectorRef was removed as using ChangeDetectorRefs for EmbeddedViewRefs does not make sense. Use ComponentRef.changeDetectorRef or inject ChangeDetectorRef instead. Fixes angular#8242
diff --git a/modules/angular2/src/core/linker/component_factory.ts b/modules/angular2/src/core/linker/component_factory.ts
@@ -62,7 +62,7 @@ export class ComponentRef_ extends ComponentRef {
   get injector(): Injector { return this._hostElement.injector; }
   get instance(): any { return this._hostElement.component; };
   get hostView(): ViewRef { return this._hostElement.parentView.ref; };
-  get changeDetectorRef(): ChangeDetectorRef { return this.hostView; };
+  get changeDetectorRef(): ChangeDetectorRef { return this._hostElement.parentView.ref; };
   get componentType(): Type { return this._componentType; }
 
   destroy(): void { this._hostElement.parentView.destroy(); }
diff --git a/modules/angular2/src/core/linker/view.ts b/modules/angular2/src/core/linker/view.ts
@@ -68,7 +68,6 @@ export abstract class AppView<T> {
   subscriptions: any[];
   contentChildren: AppView<any>[] = [];
   viewChildren: AppView<any>[] = [];
-  renderParent: AppView<any>;
   viewContainerElement: AppElement = null;
 
   // The names of the below fields must be kept in sync with codegen_name_util.ts or
@@ -134,7 +133,6 @@ export abstract class AppView<T> {
       // Note: the render nodes have been attached to their host element
       // in the ViewFactory already.
       this.declarationAppElement.parentView.viewChildren.push(this);
-      this.renderParent = this.declarationAppElement.parentView;
       this.dirtyParentQueriesInternal();
     }
   }
@@ -240,18 +238,6 @@ export abstract class AppView<T> {
    */
   dirtyParentQueriesInternal(): void {}
 
-  addRenderContentChild(view: AppView<any>): void {
-    this.contentChildren.push(view);
-    view.renderParent = this;
-    view.dirtyParentQueriesInternal();
-  }
-
-  removeContentChild(view: AppView<any>): void {
-    ListWrapper.remove(this.contentChildren, view);
-    view.dirtyParentQueriesInternal();
-    view.renderParent = null;
-  }
-
   detectChanges(throwOnChange: boolean): void {
     var s = _scope_check(this.clazz);
     if (this.cdMode === ChangeDetectionStrategy.Detached ||
@@ -304,12 +290,14 @@ export abstract class AppView<T> {
   markAsCheckOnce(): void { this.cdMode = ChangeDetectionStrategy.CheckOnce; }
 
   markPathToRootAsCheckOnce(): void {
-    var c: AppView<any> = this;
+    let c: AppView<any> = this;
     while (isPresent(c) && c.cdMode !== ChangeDetectionStrategy.Detached) {
       if (c.cdMode === ChangeDetectionStrategy.Checked) {
         c.cdMode = ChangeDetectionStrategy.CheckOnce;
       }
-      c = c.renderParent;
+      let parentEl =
+          c.type === ViewType.COMPONENT ? c.declarationAppElement : c.viewContainerElement;
+      c = isPresent(parentEl) ? parentEl.parentView : null;
     }
   }
 
diff --git a/modules/angular2/src/core/linker/view_ref.ts b/modules/angular2/src/core/linker/view_ref.ts
@@ -4,12 +4,7 @@ import {ChangeDetectorRef} from '../change_detection/change_detector_ref';
 import {AppView} from './view';
 import {ChangeDetectionStrategy} from 'angular2/src/core/change_detection/constants';
 
-export abstract class ViewRef extends ChangeDetectorRef {
-  /**
-   * @internal
-   */
-  get changeDetectorRef(): ChangeDetectorRef { return <ChangeDetectorRef>unimplemented(); };
-
+export abstract class ViewRef {
   get destroyed(): boolean { return <boolean>unimplemented(); }
 
   abstract onDestroy(callback: Function);
@@ -79,16 +74,11 @@ export abstract class EmbeddedViewRef<C> extends ViewRef {
   abstract destroy();
 }
 
-export class ViewRef_<C> implements EmbeddedViewRef<C> {
+export class ViewRef_<C> implements EmbeddedViewRef<C>, ChangeDetectorRef {
   constructor(private _view: AppView<C>) { this._view = _view; }
 
   get internalView(): AppView<C> { return this._view; }
 
-  /**
-   * Return `ChangeDetectorRef`
-   */
-  get changeDetectorRef(): ChangeDetectorRef { return this; }
-
   get rootNodes(): any[] { return this._view.flatRootNodes; }
 
   get context() { return this._view.context; }
diff --git a/modules/angular2/test/core/linker/integration_spec.ts b/modules/angular2/test/core/linker/integration_spec.ts
@@ -800,16 +800,33 @@ function declareTests(isJit: boolean) {
 
                           .createAsync(MyComp)
                           .then((fixture) => {
-                            var cmp = fixture.debugElement.children[0].references['cmp'];
-
-                            fixture.debugElement.componentInstance.ctxProp = "one";
+                            var cmpEl = fixture.debugElement.children[0];
+                            var cmp = cmpEl.componentInstance;
+                            fixture.detectChanges();
                             fixture.detectChanges();
                             expect(cmp.numberOfChecks).toEqual(1);
 
-                            fixture.debugElement.componentInstance.ctxProp = "two";
+                            cmpEl.children[0].triggerEventHandler('click', <Event>{});
+
+                            // regular element
+                            fixture.detectChanges();
                             fixture.detectChanges();
                             expect(cmp.numberOfChecks).toEqual(2);
 
+                            // element inside of an *ngIf
+                            cmpEl.children[1].triggerEventHandler('click', <Event>{});
+
+                            fixture.detectChanges();
+                            fixture.detectChanges();
+                            expect(cmp.numberOfChecks).toEqual(3);
+
+                            // element inside a nested component
+                            cmpEl.children[2].children[0].triggerEventHandler('click', <Event>{});
+
+                            fixture.detectChanges();
+                            fixture.detectChanges();
+                            expect(cmp.numberOfChecks).toEqual(4);
+
                             async.done();
                           })}));
 
@@ -1987,11 +2004,18 @@ class DirectiveWithTitleAndHostProperty {
   title: string;
 }
 
+@Component({selector: 'event-cmp', template: '<div (click)="noop()"></div>'})
+class EventCmp {
+  noop() {}
+}
+
 @Component({
   selector: 'push-cmp',
   inputs: ['prop'],
   changeDetection: ChangeDetectionStrategy.OnPush,
-  template: '{{field}}'
+  template:
+      '{{field}}<div (click)="noop()"></div><div *ngIf="true" (click)="noop()"></div><event-cmp></event-cmp>',
+  directives: [EventCmp, NgIf]
 })
 @Injectable()
 class PushCmp {
@@ -2000,6 +2024,8 @@ class PushCmp {
 
   constructor() { this.numberOfChecks = 0; }
 
+  noop() {}
+
   get field() {
     this.numberOfChecks++;
     return "fixed";
diff --git a/tools/public_api_guard/public_api_spec.ts b/tools/public_api_guard/public_api_spec.ts
@@ -477,7 +477,6 @@ const CORE = [
   'ViewQueryMetadata.isViewQuery:any',
   'ViewQueryMetadata.toString():string',
   'ViewRef',
-  'ViewRef.changeDetectorRef:ChangeDetectorRef',
   'ViewRef.destroyed:boolean',
   'ViewRef.onDestroy(callback:Function):any',
   'WrappedException',
