Skip to content

Commit

Permalink
fix(core): don’t recreate TemplateRef when used as a reference.
Browse files Browse the repository at this point in the history
This was a regression introduced in v4 rc.0.

Fixes angular#14873
  • Loading branch information
tbosch committed Mar 10, 2017
1 parent 174d4c8 commit a48d3ca
Show file tree
Hide file tree
Showing 11 changed files with 118 additions and 56 deletions.
6 changes: 3 additions & 3 deletions modules/@angular/core/src/view/provider.ts
Expand Up @@ -14,7 +14,7 @@ import {ViewContainerRef} from '../linker/view_container_ref';
import {ViewEncapsulation} from '../metadata/view';
import {Renderer as RendererV1, Renderer2, RendererFactory2, RendererType2} from '../render/api';

import {createChangeDetectorRef, createInjector, createRendererV1, createTemplateRef, createViewContainerRef} from './refs';
import {createChangeDetectorRef, createInjector, createRendererV1} from './refs';
import {BindingDef, BindingType, DepDef, DepFlags, DisposableFn, NodeData, NodeDef, NodeFlags, OutputDef, OutputType, ProviderData, QueryBindingType, QueryDef, QueryValueType, RootData, Services, ViewData, ViewDefinition, ViewFlags, ViewState, asElementData, asProviderData} from './types';
import {checkBinding, dispatchEvent, filterQueryId, isComponentView, splitMatchedQueriesDsl, tokenKey, viewParentEl} from './util';

Expand Down Expand Up @@ -356,10 +356,10 @@ export function resolveDep(
case ElementRefTokenKey:
return new ElementRef(asElementData(view, elDef.index).renderElement);
case ViewContainerRefTokenKey:
return createViewContainerRef(view, elDef);
return asElementData(view, elDef.index).viewContainer;
case TemplateRefTokenKey: {
if (elDef.element.template) {
return createTemplateRef(view, elDef);
return asElementData(view, elDef.index).template;
}
break;
}
Expand Down
11 changes: 5 additions & 6 deletions modules/@angular/core/src/view/query.ts
Expand Up @@ -11,7 +11,6 @@ import {QueryList} from '../linker/query_list';
import {TemplateRef} from '../linker/template_ref';
import {ViewContainerRef} from '../linker/view_container_ref';

import {createTemplateRef, createViewContainerRef} from './refs';
import {NodeDef, NodeFlags, QueryBindingDef, QueryBindingType, QueryDef, QueryValueType, Services, ViewData, asElementData, asProviderData, asQueryList} from './types';
import {declaredViewContainer, filterQueryId, isEmbeddedView, viewParentEl} from './util';

Expand Down Expand Up @@ -141,8 +140,8 @@ function calcQueryValues(
(nodeDef.element.template.nodeMatchedQueries & queryDef.filterId) === queryDef.filterId) {
// check embedded views that were attached at the place of their template.
const elementData = asElementData(view, i);
const embeddedViews = elementData.embeddedViews;
if (embeddedViews) {
if (nodeDef.flags & NodeFlags.EmbeddedViews) {
const embeddedViews = elementData.viewContainer._embeddedViews;
for (let k = 0; k < embeddedViews.length; k++) {
const embeddedView = embeddedViews[k];
const dvc = declaredViewContainer(embeddedView);
Expand All @@ -151,7 +150,7 @@ function calcQueryValues(
}
}
}
const projectedViews = elementData.projectedViews;
const projectedViews = elementData.template._projectedViews;
if (projectedViews) {
for (let k = 0; k < projectedViews.length; k++) {
const projectedView = projectedViews[k];
Expand Down Expand Up @@ -180,10 +179,10 @@ export function getQueryValue(
value = new ElementRef(asElementData(view, nodeDef.index).renderElement);
break;
case QueryValueType.TemplateRef:
value = createTemplateRef(view, nodeDef);
value = asElementData(view, nodeDef.index).template;
break;
case QueryValueType.ViewContainerRef:
value = createViewContainerRef(view, nodeDef);
value = asElementData(view, nodeDef.index).viewContainer;
break;
case QueryValueType.Provider:
value = asProviderData(view, nodeDef.index).instance;
Expand Down
44 changes: 24 additions & 20 deletions modules/@angular/core/src/view/refs.ts
Expand Up @@ -18,7 +18,7 @@ import {Renderer as RendererV1, Renderer2} from '../render/api';
import {Type} from '../type';
import {VERSION} from '../version';

import {ArgumentType, BindingType, DebugContext, DepFlags, ElementData, NodeCheckFn, NodeData, NodeDef, NodeFlags, RootData, Services, ViewData, ViewDefinition, ViewDefinitionFactory, ViewState, asElementData, asProviderData, asTextData} from './types';
import {ArgumentType, BindingType, DebugContext, DepFlags, ElementData, NodeCheckFn, NodeData, NodeDef, NodeFlags, RootData, Services, TemplateData, ViewContainerData, ViewData, ViewDefinition, ViewDefinitionFactory, ViewState, asElementData, asProviderData, asTextData} from './types';
import {isComponentView, markParentViewsForCheck, renderNode, resolveViewDefinition, rootRenderNodes, splitNamespace, tokenKey, viewParentEl} from './util';
import {attachEmbeddedView, detachEmbeddedView, moveEmbeddedView, renderDetachView} from './view_attach';

Expand Down Expand Up @@ -84,15 +84,17 @@ class ComponentRef_ extends ComponentRef<any> {
onDestroy(callback: Function): void { this._viewRef.onDestroy(callback); }
}

export function createViewContainerRef(view: ViewData, elDef: NodeDef): ViewContainerRef {
return new ViewContainerRef_(view, elDef);
export function createViewContainerData(
view: ViewData, elDef: NodeDef, elData: ElementData): ViewContainerData {
return new ViewContainerRef_(view, elDef, elData);
}

class ViewContainerRef_ implements ViewContainerRef {
private _data: ElementData;
constructor(private _view: ViewData, private _elDef: NodeDef) {
this._data = asElementData(_view, _elDef.index);
}
class ViewContainerRef_ implements ViewContainerData {
/**
* @internal
*/
_embeddedViews: ViewData[] = [];
constructor(private _view: ViewData, private _elDef: NodeDef, private _data: ElementData) {}

get element(): ElementRef { return new ElementRef(this._data.renderElement); }

Expand All @@ -109,15 +111,15 @@ class ViewContainerRef_ implements ViewContainerRef {
}

clear(): void {
const len = this._data.embeddedViews.length;
const len = this._embeddedViews.length;
for (let i = len - 1; i >= 0; i--) {
const view = detachEmbeddedView(this._data, i);
Services.destroyView(view);
}
}

get(index: number): ViewRef {
const view = this._data.embeddedViews[index];
const view = this._embeddedViews[index];
if (view) {
const ref = new ViewRef_(view);
ref.attachToViewContainerRef(this);
Expand All @@ -126,7 +128,7 @@ class ViewContainerRef_ implements ViewContainerRef {
return null;
}

get length(): number { return this._data.embeddedViews.length; };
get length(): number { return this._embeddedViews.length; };

createEmbeddedView<C>(templateRef: TemplateRef<C>, context?: C, index?: number):
EmbeddedViewRef<C> {
Expand All @@ -153,13 +155,13 @@ class ViewContainerRef_ implements ViewContainerRef {
}

move(viewRef: ViewRef_, currentIndex: number): ViewRef {
const previousIndex = this._data.embeddedViews.indexOf(viewRef._view);
const previousIndex = this._embeddedViews.indexOf(viewRef._view);
moveEmbeddedView(this._data, previousIndex, currentIndex);
return viewRef;
}

indexOf(viewRef: ViewRef): number {
return this._data.embeddedViews.indexOf((<ViewRef_>viewRef)._view);
return this._embeddedViews.indexOf((<ViewRef_>viewRef)._view);
}

remove(index?: number): void {
Expand Down Expand Up @@ -240,11 +242,16 @@ export class ViewRef_ implements EmbeddedViewRef<any>, InternalViewRef {
}
}

export function createTemplateRef(view: ViewData, def: NodeDef): TemplateRef<any> {
export function createTemplateData(view: ViewData, def: NodeDef): TemplateData {
return new TemplateRef_(view, def);
}

class TemplateRef_ extends TemplateRef<any> {
class TemplateRef_ extends TemplateRef<any> implements TemplateData {
/**
* @internal
*/
_projectedViews: ViewData[];

constructor(private _parentView: ViewData, private _def: NodeDef) { super(); }

createEmbeddedView(context: any): EmbeddedViewRef<any> {
Expand Down Expand Up @@ -273,11 +280,8 @@ class Injector_ implements Injector {
export function nodeValue(view: ViewData, index: number): any {
const def = view.def.nodes[index];
if (def.flags & NodeFlags.TypeElement) {
if (def.element.template) {
return createTemplateRef(view, def);
} else {
return asElementData(view, def.index).renderElement;
}
const elData = asElementData(view, def.index);
return def.element.template ? elData.template : elData.renderElement;
} else if (def.flags & NodeFlags.TypeText) {
return asTextData(view, def.index).renderText;
} else if (def.flags & (NodeFlags.CatProvider | NodeFlags.TypePipe)) {
Expand Down
10 changes: 8 additions & 2 deletions modules/@angular/core/src/view/types.ts
Expand Up @@ -355,12 +355,18 @@ export function asTextData(view: ViewData, index: number): TextData {
export interface ElementData {
renderElement: any;
componentView: ViewData;
embeddedViews: ViewData[];
viewContainer: ViewContainerData;
template: TemplateData;
}

export interface ViewContainerData extends ViewContainerRef { _embeddedViews: ViewData[]; }

export interface TemplateData extends TemplateRef<any> {
// views that have been created from the template
// of this element,
// but inserted into the embeddedViews of another element.
// By default, this is undefined.
projectedViews: ViewData[];
_projectedViews: ViewData[];
}

/**
Expand Down
8 changes: 3 additions & 5 deletions modules/@angular/core/src/view/util.ts
Expand Up @@ -293,11 +293,9 @@ function visitRenderNode(
const rn = renderNode(view, nodeDef);
execRenderNodeAction(view, rn, action, parentNode, nextSibling, target);
if (nodeDef.flags & NodeFlags.EmbeddedViews) {
const embeddedViews = asElementData(view, nodeDef.index).embeddedViews;
if (embeddedViews) {
for (let k = 0; k < embeddedViews.length; k++) {
visitRootRenderNodes(embeddedViews[k], action, parentNode, nextSibling, target);
}
const embeddedViews = asElementData(view, nodeDef.index).viewContainer._embeddedViews;
for (let k = 0; k < embeddedViews.length; k++) {
visitRootRenderNodes(embeddedViews[k], action, parentNode, nextSibling, target);
}
}
if (nodeDef.flags & NodeFlags.TypeElement && !nodeDef.element.name) {
Expand Down
16 changes: 9 additions & 7 deletions modules/@angular/core/src/view/view.ts
Expand Up @@ -15,6 +15,7 @@ import {appendNgContent} from './ng_content';
import {callLifecycleHooksChildrenFirst, checkAndUpdateDirectiveDynamic, checkAndUpdateDirectiveInline, createDirectiveInstance, createPipeInstance, createProviderInstance} from './provider';
import {checkAndUpdatePureExpressionDynamic, checkAndUpdatePureExpressionInline, createPureExpression} from './pure_expression';
import {checkAndUpdateQuery, createQuery, queryDef} from './query';
import {createTemplateData, createViewContainerData} from './refs';
import {checkAndUpdateTextDynamic, checkAndUpdateTextInline, createText} from './text';
import {ArgumentType, CheckType, ElementData, ElementDef, NodeData, NodeDef, NodeFlags, ProviderData, ProviderDef, RootData, Services, TextDef, ViewData, ViewDefinition, ViewDefinitionFactory, ViewFlags, ViewHandleEventFn, ViewState, ViewUpdateFn, asElementData, asProviderData, asPureExpressionData, asQueryList, asTextData} from './types';
import {checkBindingNoChanges, isComponentView, resolveViewDefinition, viewParentEl} from './util';
Expand Down Expand Up @@ -255,9 +256,12 @@ function createViewNodes(view: ViewData) {
nodeData = <ElementData>{
renderElement: el,
componentView,
embeddedViews: (nodeDef.flags & NodeFlags.EmbeddedViews) ? [] : undefined,
projectedViews: undefined
viewContainer: undefined,
template: nodeDef.element.template ? createTemplateData(view, nodeDef) : undefined
};
if (nodeDef.flags & NodeFlags.EmbeddedViews) {
nodeData.viewContainer = createViewContainerData(view, nodeDef, nodeData);
}
break;
case NodeFlags.TypeText:
nodeData = createText(view, renderHost, nodeDef) as any;
Expand Down Expand Up @@ -525,11 +529,9 @@ function execEmbeddedViewsAction(view: ViewData, action: ViewAction) {
const nodeDef = def.nodes[i];
if (nodeDef.flags & NodeFlags.EmbeddedViews) {
// a leaf
const embeddedViews = asElementData(view, i).embeddedViews;
if (embeddedViews) {
for (let k = 0; k < embeddedViews.length; k++) {
callViewAction(embeddedViews[k], action);
}
const embeddedViews = asElementData(view, i).viewContainer._embeddedViews;
for (let k = 0; k < embeddedViews.length; k++) {
callViewAction(embeddedViews[k], action);
}
} else if ((nodeDef.childFlags & NodeFlags.EmbeddedViews) === 0) {
// a parent with leafs
Expand Down
12 changes: 6 additions & 6 deletions modules/@angular/core/src/view/view_attach.ts
Expand Up @@ -11,17 +11,17 @@ import {RenderNodeAction, declaredViewContainer, isComponentView, renderNode, ro

export function attachEmbeddedView(
parentView: ViewData, elementData: ElementData, viewIndex: number, view: ViewData) {
let embeddedViews = elementData.embeddedViews;
let embeddedViews = elementData.viewContainer._embeddedViews;
if (viewIndex == null) {
viewIndex = embeddedViews.length;
}
view.viewContainerParent = parentView;
addToArray(embeddedViews, viewIndex, view);
const dvcElementData = declaredViewContainer(view);
if (dvcElementData && dvcElementData !== elementData) {
let projectedViews = dvcElementData.projectedViews;
let projectedViews = dvcElementData.template._projectedViews;
if (!projectedViews) {
projectedViews = dvcElementData.projectedViews = [];
projectedViews = dvcElementData.template._projectedViews = [];
}
projectedViews.push(view);
}
Expand All @@ -33,7 +33,7 @@ export function attachEmbeddedView(
}

export function detachEmbeddedView(elementData: ElementData, viewIndex: number): ViewData {
const embeddedViews = elementData.embeddedViews;
const embeddedViews = elementData.viewContainer._embeddedViews;
if (viewIndex == null || viewIndex >= embeddedViews.length) {
viewIndex = embeddedViews.length - 1;
}
Expand All @@ -46,7 +46,7 @@ export function detachEmbeddedView(elementData: ElementData, viewIndex: number):

const dvcElementData = declaredViewContainer(view);
if (dvcElementData && dvcElementData !== elementData) {
const projectedViews = dvcElementData.projectedViews;
const projectedViews = dvcElementData.template._projectedViews;
removeFromArray(projectedViews, projectedViews.indexOf(view));
}

Expand All @@ -59,7 +59,7 @@ export function detachEmbeddedView(elementData: ElementData, viewIndex: number):

export function moveEmbeddedView(
elementData: ElementData, oldViewIndex: number, newViewIndex: number): ViewData {
const embeddedViews = elementData.embeddedViews;
const embeddedViews = elementData.viewContainer._embeddedViews;
const view = embeddedViews[oldViewIndex];
removeFromArray(embeddedViews, oldViewIndex);
if (newViewIndex == null) {
Expand Down
60 changes: 56 additions & 4 deletions modules/@angular/core/test/linker/regression_integration_spec.ts
Expand Up @@ -6,8 +6,9 @@
* found in the LICENSE file at https://angular.io/license
*/

import {ANALYZE_FOR_ENTRY_COMPONENTS, Component, InjectionToken, Injector, Pipe, PipeTransform, Provider, Renderer2} from '@angular/core';
import {ANALYZE_FOR_ENTRY_COMPONENTS, Component, Directive, InjectionToken, Injector, Input, Pipe, PipeTransform, Provider, QueryList, Renderer2, TemplateRef, ViewChildren, ViewContainerRef} from '@angular/core';
import {TestBed} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
import {expect} from '@angular/platform-browser/testing/matchers';

export function main() {
Expand All @@ -16,7 +17,13 @@ export function main() {
describe('no jit', () => { declareTests({useJit: false}); });
}

function declareTests({useJit}: {useJit: boolean}) {
jasmine.pp =
function(msg) {
return msg;
}

function
declareTests({useJit}: {useJit: boolean}) {
// Place to put reproductions for regressions
describe('regressions', () => {

Expand Down Expand Up @@ -222,11 +229,56 @@ function declareTests({useJit}: {useJit: boolean}) {
const txtNode = ctx.componentInstance.renderer.createText('test');
expect(txtNode).toHaveText('test');
});

it('should not recreate TemplateRef references during dirty checking', () => {
@Component({template: '<div [someDir]="someRef"></div><ng-template #someRef></ng-template>'})
class MyComp {
}

@Directive({selector: '[someDir]'})
class MyDir {
@Input('someDir') template: TemplateRef<any>;
}

const ctx =
TestBed.configureTestingModule({declarations: [MyComp, MyDir]}).createComponent(MyComp);
const dir = <MyDir>ctx.debugElement.query(By.directive(MyDir)).injector.get(MyDir);

expect(dir.template).toBeUndefined();

ctx.detectChanges();
const template = dir.template;
expect(template).toBeDefined();

ctx.detectChanges();
expect(dir.template).toBe(template);
});

it('should not recreate ViewContainerRefs in queries', () => {
@Component({template: '<div #vc></div><div *ngIf="show" #vc></div>'})
class MyComp {
@ViewChildren('vc', {read: ViewContainerRef})
viewContainers: QueryList<ViewContainerRef>;

show = true;
}

const ctx = TestBed.configureTestingModule({declarations: [MyComp]}).createComponent(MyComp);

ctx.componentInstance.show = true;
ctx.detectChanges();
expect(ctx.componentInstance.viewContainers.length).toBe(2);
const vc = ctx.componentInstance.viewContainers.first;
expect(vc).toBeDefined();

ctx.componentInstance.show = false;
ctx.detectChanges();
expect(ctx.componentInstance.viewContainers.first).toBe(vc);
});
});
}

@Component({selector: 'my-comp', template: ''})
class MyComp1 {
@Component({selector: 'my-comp', template: ''}) class MyComp1 {
constructor(public injector: Injector) {}
}

Expand Down
2 changes: 1 addition & 1 deletion modules/@angular/core/test/view/embedded_view_spec.ts
Expand Up @@ -101,7 +101,7 @@ export function main() {

moveEmbeddedView(viewContainerData, 0, 1);

expect(viewContainerData.embeddedViews).toEqual([childView1, childView0]);
expect(viewContainerData.viewContainer._embeddedViews).toEqual([childView1, childView0]);
// 2 anchors + 2 elements
const rootChildren = getDOM().childNodes(rootNodes[0]);
expect(rootChildren.length).toBe(4);
Expand Down
2 changes: 1 addition & 1 deletion modules/@angular/core/test/view/ng_content_spec.ts
Expand Up @@ -96,7 +96,7 @@ export function main() {
const anchor = asElementData(view, 2);
expect((getDOM().childNodes(getDOM().firstChild(rootNodes[0]))[0]))
.toBe(anchor.renderElement);
const embeddedView = anchor.embeddedViews[0];
const embeddedView = anchor.viewContainer._embeddedViews[0];
expect((getDOM().childNodes(getDOM().firstChild(rootNodes[0]))[1]))
.toBe(asTextData(embeddedView, 0).renderText);
});
Expand Down

0 comments on commit a48d3ca

Please sign in to comment.