Skip to content

Commit 4d691b6

Browse files
committed
fix(core): check components if an event handler inside of an embedded view fires.
BREAKING CHANGE: - ViewRef.changeDetectorRef was removed as using ChangeDetectorRefs for EmbeddedViewRefs does not make sense. Use ComponentRef.changeDetectorRef or inject ChangeDetectorRef instead. Fixes #8242
1 parent 11955f9 commit 4d691b6

File tree

5 files changed

+38
-35
lines changed

5 files changed

+38
-35
lines changed

modules/angular2/src/core/linker/component_factory.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ export class ComponentRef_ extends ComponentRef {
6262
get injector(): Injector { return this._hostElement.injector; }
6363
get instance(): any { return this._hostElement.component; };
6464
get hostView(): ViewRef { return this._hostElement.parentView.ref; };
65-
get changeDetectorRef(): ChangeDetectorRef { return this.hostView; };
65+
get changeDetectorRef(): ChangeDetectorRef { return this._hostElement.parentView.ref; };
6666
get componentType(): Type { return this._componentType; }
6767

6868
destroy(): void { this._hostElement.parentView.destroy(); }

modules/angular2/src/core/linker/view.ts

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ export abstract class AppView<T> {
6868
subscriptions: any[];
6969
contentChildren: AppView<any>[] = [];
7070
viewChildren: AppView<any>[] = [];
71-
renderParent: AppView<any>;
7271
viewContainerElement: AppElement = null;
7372

7473
// 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> {
134133
// Note: the render nodes have been attached to their host element
135134
// in the ViewFactory already.
136135
this.declarationAppElement.parentView.viewChildren.push(this);
137-
this.renderParent = this.declarationAppElement.parentView;
138136
this.dirtyParentQueriesInternal();
139137
}
140138
}
@@ -240,18 +238,6 @@ export abstract class AppView<T> {
240238
*/
241239
dirtyParentQueriesInternal(): void {}
242240

243-
addRenderContentChild(view: AppView<any>): void {
244-
this.contentChildren.push(view);
245-
view.renderParent = this;
246-
view.dirtyParentQueriesInternal();
247-
}
248-
249-
removeContentChild(view: AppView<any>): void {
250-
ListWrapper.remove(this.contentChildren, view);
251-
view.dirtyParentQueriesInternal();
252-
view.renderParent = null;
253-
}
254-
255241
detectChanges(throwOnChange: boolean): void {
256242
var s = _scope_check(this.clazz);
257243
if (this.cdMode === ChangeDetectionStrategy.Detached ||
@@ -304,12 +290,14 @@ export abstract class AppView<T> {
304290
markAsCheckOnce(): void { this.cdMode = ChangeDetectionStrategy.CheckOnce; }
305291

306292
markPathToRootAsCheckOnce(): void {
307-
var c: AppView<any> = this;
293+
let c: AppView<any> = this;
308294
while (isPresent(c) && c.cdMode !== ChangeDetectionStrategy.Detached) {
309295
if (c.cdMode === ChangeDetectionStrategy.Checked) {
310296
c.cdMode = ChangeDetectionStrategy.CheckOnce;
311297
}
312-
c = c.renderParent;
298+
let parentEl =
299+
c.type === ViewType.COMPONENT ? c.declarationAppElement : c.viewContainerElement;
300+
c = isPresent(parentEl) ? parentEl.parentView : null;
313301
}
314302
}
315303

modules/angular2/src/core/linker/view_ref.ts

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,7 @@ import {ChangeDetectorRef} from '../change_detection/change_detector_ref';
44
import {AppView} from './view';
55
import {ChangeDetectionStrategy} from 'angular2/src/core/change_detection/constants';
66

7-
export abstract class ViewRef extends ChangeDetectorRef {
8-
/**
9-
* @internal
10-
*/
11-
get changeDetectorRef(): ChangeDetectorRef { return <ChangeDetectorRef>unimplemented(); };
12-
7+
export abstract class ViewRef {
138
get destroyed(): boolean { return <boolean>unimplemented(); }
149

1510
abstract onDestroy(callback: Function);
@@ -79,16 +74,11 @@ export abstract class EmbeddedViewRef<C> extends ViewRef {
7974
abstract destroy();
8075
}
8176

82-
export class ViewRef_<C> implements EmbeddedViewRef<C> {
77+
export class ViewRef_<C> implements EmbeddedViewRef<C>, ChangeDetectorRef {
8378
constructor(private _view: AppView<C>) { this._view = _view; }
8479

8580
get internalView(): AppView<C> { return this._view; }
8681

87-
/**
88-
* Return `ChangeDetectorRef`
89-
*/
90-
get changeDetectorRef(): ChangeDetectorRef { return this; }
91-
9282
get rootNodes(): any[] { return this._view.flatRootNodes; }
9383

9484
get context() { return this._view.context; }

modules/angular2/test/core/linker/integration_spec.ts

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -800,16 +800,33 @@ function declareTests(isJit: boolean) {
800800

801801
.createAsync(MyComp)
802802
.then((fixture) => {
803-
var cmp = fixture.debugElement.children[0].references['cmp'];
804-
805-
fixture.debugElement.componentInstance.ctxProp = "one";
803+
var cmpEl = fixture.debugElement.children[0];
804+
var cmp = cmpEl.componentInstance;
805+
fixture.detectChanges();
806806
fixture.detectChanges();
807807
expect(cmp.numberOfChecks).toEqual(1);
808808

809-
fixture.debugElement.componentInstance.ctxProp = "two";
809+
cmpEl.children[0].triggerEventHandler('click', <Event>{});
810+
811+
// regular element
812+
fixture.detectChanges();
810813
fixture.detectChanges();
811814
expect(cmp.numberOfChecks).toEqual(2);
812815

816+
// element inside of an *ngIf
817+
cmpEl.children[1].triggerEventHandler('click', <Event>{});
818+
819+
fixture.detectChanges();
820+
fixture.detectChanges();
821+
expect(cmp.numberOfChecks).toEqual(3);
822+
823+
// element inside a nested component
824+
cmpEl.children[2].children[0].triggerEventHandler('click', <Event>{});
825+
826+
fixture.detectChanges();
827+
fixture.detectChanges();
828+
expect(cmp.numberOfChecks).toEqual(4);
829+
813830
async.done();
814831
})}));
815832

@@ -1987,11 +2004,18 @@ class DirectiveWithTitleAndHostProperty {
19872004
title: string;
19882005
}
19892006

2007+
@Component({selector: 'event-cmp', template: '<div (click)="noop()"></div>'})
2008+
class EventCmp {
2009+
noop() {}
2010+
}
2011+
19902012
@Component({
19912013
selector: 'push-cmp',
19922014
inputs: ['prop'],
19932015
changeDetection: ChangeDetectionStrategy.OnPush,
1994-
template: '{{field}}'
2016+
template:
2017+
'{{field}}<div (click)="noop()"></div><div *ngIf="true" (click)="noop()"></div><event-cmp></event-cmp>',
2018+
directives: [EventCmp, NgIf]
19952019
})
19962020
@Injectable()
19972021
class PushCmp {
@@ -2000,6 +2024,8 @@ class PushCmp {
20002024

20012025
constructor() { this.numberOfChecks = 0; }
20022026

2027+
noop() {}
2028+
20032029
get field() {
20042030
this.numberOfChecks++;
20052031
return "fixed";

tools/public_api_guard/public_api_spec.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,6 @@ const CORE = [
477477
'ViewQueryMetadata.isViewQuery:any',
478478
'ViewQueryMetadata.toString():string',
479479
'ViewRef',
480-
'ViewRef.changeDetectorRef:ChangeDetectorRef',
481480
'ViewRef.destroyed:boolean',
482481
'ViewRef.onDestroy(callback:Function):any',
483482
'WrappedException',

0 commit comments

Comments
 (0)