fix: view engine - fix some corner cases

This commit is contained in:
Tobias Bosch 2017-02-20 14:34:15 -08:00 committed by Igor Minar
parent 32012a1ffb
commit 7db93310f1
9 changed files with 93 additions and 29 deletions

View File

@ -294,9 +294,12 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver, BuiltinConverter
if (hostBindings.length) { if (hostBindings.length) {
this._addUpdateExpressions(nodeIndex, hostBindings, this.updateRendererExpressions); this._addUpdateExpressions(nodeIndex, hostBindings, this.updateRendererExpressions);
} }
inputDefs = elementBindingDefs(ast.inputs); // Note: inputDefs have to be in the same order as hostBindings:
// - first the entries from the directives, then the ones from the element.
ast.directives.forEach( ast.directives.forEach(
(dirAst, dirIndex) => inputDefs.push(...elementBindingDefs(dirAst.hostProperties))); (dirAst, dirIndex) => inputDefs.push(...elementBindingDefs(dirAst.hostProperties)));
inputDefs.push(...elementBindingDefs(ast.inputs));
outputDefs = usedEvents.map(([target, eventName]) => { outputDefs = usedEvents.map(([target, eventName]) => {
return target ? o.literalArr([o.literal(target), o.literal(eventName)]) : return target ? o.literalArr([o.literal(target), o.literal(eventName)]) :
o.literal(eventName); o.literal(eventName);
@ -847,6 +850,9 @@ function needsAdditionalRootNode(ast: TemplateAst): boolean {
} }
if (ast instanceof ElementAst) { if (ast instanceof ElementAst) {
if (ast.name === NG_CONTAINER_TAG && ast.children.length) {
return needsAdditionalRootNode(ast.children[ast.children.length - 1]);
}
return ast.hasViewContainer; return ast.hasViewContainer;
} }

View File

@ -103,10 +103,15 @@ class ViewContainerRef_ implements ViewContainerRef {
} }
} }
get(index: number): ViewRef { get(index: number): ViewRef { return this._getViewRef(this._data.embeddedViews[index]); }
const ref = new ViewRef_(this._data.embeddedViews[index]);
ref.attachToViewContainerRef(this); private _getViewRef(view: ViewData) {
return ref; if (view) {
const ref = new ViewRef_(view);
ref.attachToViewContainerRef(this);
return ref;
}
return null;
} }
get length(): number { return this._data.embeddedViews.length; }; get length(): number { return this._data.embeddedViews.length; };
@ -147,14 +152,19 @@ class ViewContainerRef_ implements ViewContainerRef {
remove(index?: number): void { remove(index?: number): void {
const viewData = Services.detachEmbeddedView(this._data, index); const viewData = Services.detachEmbeddedView(this._data, index);
Services.destroyView(viewData); if (viewData) {
Services.destroyView(viewData);
}
} }
detach(index?: number): ViewRef { detach(index?: number): ViewRef {
const view = this.get(index); const view = Services.detachEmbeddedView(this._data, index);
Services.detachEmbeddedView(this._data, index); if (view) {
(view as ViewRef_).detachFromContainer(); const viewRef = this._getViewRef(view);
return view; viewRef.detachFromContainer();
return viewRef;
}
return null;
} }
} }

View File

@ -38,7 +38,7 @@ export interface ViewDefinition {
* Especially providers are after elements / anchors. * Especially providers are after elements / anchors.
*/ */
reverseChildNodes: NodeDef[]; reverseChildNodes: NodeDef[];
lastRootNode: NodeDef; lastRenderRootNode: NodeDef;
bindingCount: number; bindingCount: number;
disposableCount: number; disposableCount: number;
/** /**

View File

@ -230,7 +230,7 @@ export function visitRootRenderNodes(
view: ViewData, action: RenderNodeAction, parentNode: any, nextSibling: any, target: any[]) { view: ViewData, action: RenderNodeAction, parentNode: any, nextSibling: any, target: any[]) {
// We need to re-compute the parent node in case the nodes have been moved around manually // We need to re-compute the parent node in case the nodes have been moved around manually
if (action === RenderNodeAction.RemoveChild) { if (action === RenderNodeAction.RemoveChild) {
parentNode = view.renderer.parentNode(renderNode(view, view.def.lastRootNode)); parentNode = view.renderer.parentNode(renderNode(view, view.def.lastRenderRootNode));
} }
visitSiblingRenderNodes( visitSiblingRenderNodes(
view, action, 0, view.def.nodes.length - 1, parentNode, nextSibling, target); view, action, 0, view.def.nodes.length - 1, parentNode, nextSibling, target);

View File

@ -37,7 +37,7 @@ export function viewDef(
let currentParent: NodeDef = null; let currentParent: NodeDef = null;
let currentElementHasPublicProviders = false; let currentElementHasPublicProviders = false;
let currentElementHasPrivateProviders = false; let currentElementHasPrivateProviders = false;
let lastRootNode: NodeDef = null; let lastRenderRootNode: NodeDef = null;
for (let i = 0; i < nodes.length; i++) { for (let i = 0; i < nodes.length; i++) {
while (currentParent && i > currentParent.index + currentParent.childCount) { while (currentParent && i > currentParent.index + currentParent.childCount) {
const newParent = currentParent.parent; const newParent = currentParent.parent;
@ -92,8 +92,8 @@ export function viewDef(
viewBindingCount += node.bindings.length; viewBindingCount += node.bindings.length;
viewDisposableCount += node.disposableCount; viewDisposableCount += node.disposableCount;
if (!currentRenderParent) { if (!currentRenderParent && (node.type === NodeType.Element || node.type === NodeType.Text)) {
lastRootNode = node; lastRenderRootNode = node;
} }
if (node.type === NodeType.Provider || node.type === NodeType.Directive) { if (node.type === NodeType.Provider || node.type === NodeType.Directive) {
if (!currentElementHasPublicProviders) { if (!currentElementHasPublicProviders) {
@ -141,7 +141,7 @@ export function viewDef(
updateRenderer: updateRenderer || NOOP, updateRenderer: updateRenderer || NOOP,
handleEvent: handleEvent || NOOP, handleEvent: handleEvent || NOOP,
bindingCount: viewBindingCount, bindingCount: viewBindingCount,
disposableCount: viewDisposableCount, lastRootNode disposableCount: viewDisposableCount, lastRenderRootNode
}; };
} }
@ -189,7 +189,8 @@ function calculateReverseChildIndex(
function validateNode(parent: NodeDef, node: NodeDef, nodeCount: number) { function validateNode(parent: NodeDef, node: NodeDef, nodeCount: number) {
const template = node.element && node.element.template; const template = node.element && node.element.template;
if (template) { if (template) {
if (template.lastRootNode && template.lastRootNode.flags & NodeFlags.HasEmbeddedViews) { if (template.lastRenderRootNode &&
template.lastRenderRootNode.flags & NodeFlags.HasEmbeddedViews) {
throw new Error( throw new Error(
`Illegal State: Last root node of a template can't have embedded views, at index ${node.index}!`); `Illegal State: Last root node of a template can't have embedded views, at index ${node.index}!`);
} }
@ -476,6 +477,9 @@ function checkNoChangesQuery(view: ViewData, nodeDef: NodeDef) {
} }
export function destroyView(view: ViewData) { export function destroyView(view: ViewData) {
if (view.state & ViewState.Destroyed) {
return;
}
execEmbeddedViewsAction(view, ViewAction.Destroy); execEmbeddedViewsAction(view, ViewAction.Destroy);
execComponentViewsAction(view, ViewAction.Destroy); execComponentViewsAction(view, ViewAction.Destroy);
callLifecycleHooksChildrenFirst(view, NodeFlags.OnDestroy); callLifecycleHooksChildrenFirst(view, NodeFlags.OnDestroy);

View File

@ -33,8 +33,11 @@ export function attachEmbeddedView(elementData: ElementData, viewIndex: number,
export function detachEmbeddedView(elementData: ElementData, viewIndex: number): ViewData { export function detachEmbeddedView(elementData: ElementData, viewIndex: number): ViewData {
const embeddedViews = elementData.embeddedViews; const embeddedViews = elementData.embeddedViews;
if (viewIndex == null) { if (viewIndex == null || viewIndex >= embeddedViews.length) {
viewIndex = embeddedViews.length; viewIndex = embeddedViews.length - 1;
}
if (viewIndex < 0) {
return null;
} }
const view = embeddedViews[viewIndex]; const view = embeddedViews[viewIndex];
removeFromArray(embeddedViews, viewIndex); removeFromArray(embeddedViews, viewIndex);
@ -76,7 +79,7 @@ export function moveEmbeddedView(
function renderAttachEmbeddedView(elementData: ElementData, prevView: ViewData, view: ViewData) { function renderAttachEmbeddedView(elementData: ElementData, prevView: ViewData, view: ViewData) {
const prevRenderNode = const prevRenderNode =
prevView ? renderNode(prevView, prevView.def.lastRootNode) : elementData.renderElement; prevView ? renderNode(prevView, prevView.def.lastRenderRootNode) : elementData.renderElement;
const parentNode = view.renderer.parentNode(prevRenderNode); const parentNode = view.renderer.parentNode(prevRenderNode);
const nextSibling = view.renderer.nextSibling(prevRenderNode); const nextSibling = view.renderer.nextSibling(prevRenderNode);
// Note: We can't check if `nextSibling` is present, as on WebWorkers it will always be! // Note: We can't check if `nextSibling` is present, as on WebWorkers it will always be!

View File

@ -19,7 +19,8 @@ import {DomSharedStylesHost} from './shared_styles_host';
export const NAMESPACE_URIS: {[ns: string]: string} = { export const NAMESPACE_URIS: {[ns: string]: string} = {
'xlink': 'http://www.w3.org/1999/xlink', 'xlink': 'http://www.w3.org/1999/xlink',
'svg': 'http://www.w3.org/2000/svg', 'svg': 'http://www.w3.org/2000/svg',
'xhtml': 'http://www.w3.org/1999/xhtml' 'xhtml': 'http://www.w3.org/1999/xhtml',
'xml': 'http://www.w3.org/XML/1998/namespace'
}; };
const TEMPLATE_COMMENT_TEXT = 'template bindings={}'; const TEMPLATE_COMMENT_TEXT = 'template bindings={}';
const TEMPLATE_BINDINGS_EXP = /^template bindings=(.*)$/; const TEMPLATE_BINDINGS_EXP = /^template bindings=(.*)$/;

View File

@ -266,11 +266,12 @@ function appendNodes(parent: any, nodes: any) {
export class ServerRendererFactoryV2 implements RendererFactoryV2 { export class ServerRendererFactoryV2 implements RendererFactoryV2 {
private rendererByCompId = new Map<string, RendererV2>(); private rendererByCompId = new Map<string, RendererV2>();
private defaultRenderer: RendererV2; private defaultRenderer: RendererV2;
private schema = new DomElementSchemaRegistry();
constructor( constructor(
private ngZone: NgZone, @Inject(DOCUMENT) private document: any, private ngZone: NgZone, @Inject(DOCUMENT) private document: any,
private sharedStylesHost: SharedStylesHost) { private sharedStylesHost: SharedStylesHost) {
this.defaultRenderer = new DefaultServerRendererV2(document, ngZone); this.defaultRenderer = new DefaultServerRendererV2(document, ngZone, this.schema);
}; };
createRenderer(element: any, type: RendererTypeV2): RendererV2 { createRenderer(element: any, type: RendererTypeV2): RendererV2 {
@ -282,7 +283,7 @@ export class ServerRendererFactoryV2 implements RendererFactoryV2 {
let renderer = this.rendererByCompId.get(type.id); let renderer = this.rendererByCompId.get(type.id);
if (!renderer) { if (!renderer) {
renderer = new EmulatedEncapsulationServerRendererV2( renderer = new EmulatedEncapsulationServerRendererV2(
this.document, this.ngZone, this.sharedStylesHost, type); this.document, this.ngZone, this.sharedStylesHost, this.schema, type);
this.rendererByCompId.set(type.id, renderer); this.rendererByCompId.set(type.id, renderer);
} }
(<EmulatedEncapsulationServerRendererV2>renderer).applyToHost(element); (<EmulatedEncapsulationServerRendererV2>renderer).applyToHost(element);
@ -303,7 +304,8 @@ export class ServerRendererFactoryV2 implements RendererFactoryV2 {
} }
class DefaultServerRendererV2 implements RendererV2 { class DefaultServerRendererV2 implements RendererV2 {
constructor(private document: any, private ngZone: NgZone) {} constructor(
private document: any, private ngZone: NgZone, private schema: DomElementSchemaRegistry) {}
destroy(): void {} destroy(): void {}
@ -382,7 +384,26 @@ class DefaultServerRendererV2 implements RendererV2 {
getDOM().removeStyle(el, style); getDOM().removeStyle(el, style);
} }
setProperty(el: any, name: string, value: any): void { getDOM().setProperty(el, name, value); } // The value was validated already as a property binding, against the property name.
// To know this value is safe to use as an attribute, the security context of the
// attribute with the given name is checked against that security context of the
// property.
private _isSafeToReflectProperty(tagName: string, propertyName: string): boolean {
return this.schema.securityContext(tagName, propertyName, true) ===
this.schema.securityContext(tagName, propertyName, false);
}
setProperty(el: any, name: string, value: any): void {
getDOM().setProperty(el, name, value);
// Mirror property values for known HTML element properties in the attributes.
const tagName = (el.tagName as string).toLowerCase();
if (isPresent(value) && (typeof value === 'number' || typeof value == 'string') &&
this.schema.hasElement(tagName, EMPTY_ARRAY) &&
this.schema.hasProperty(tagName, name, EMPTY_ARRAY) &&
this._isSafeToReflectProperty(tagName, name)) {
this.setAttribute(el, name, value.toString());
}
}
setValue(node: any, value: string): void { getDOM().setText(node, value); } setValue(node: any, value: string): void { getDOM().setText(node, value); }
@ -404,8 +425,8 @@ class EmulatedEncapsulationServerRendererV2 extends DefaultServerRendererV2 {
constructor( constructor(
document: any, ngZone: NgZone, sharedStylesHost: SharedStylesHost, document: any, ngZone: NgZone, sharedStylesHost: SharedStylesHost,
private component: RendererTypeV2) { schema: DomElementSchemaRegistry, private component: RendererTypeV2) {
super(document, ngZone); super(document, ngZone, schema);
const styles = flattenStyles(component.id, component.styles, []); const styles = flattenStyles(component.id, component.styles, []);
sharedStylesHost.addStyles(styles); sharedStylesHost.addStyles(styles);

View File

@ -7,14 +7,14 @@
*/ */
import {PlatformLocation} from '@angular/common'; import {PlatformLocation} from '@angular/common';
import {USE_VIEW_ENGINE} from '@angular/compiler/src/config';
import {ApplicationRef, CompilerFactory, Component, NgModule, NgModuleRef, NgZone, PlatformRef, destroyPlatform, getPlatform} from '@angular/core'; import {ApplicationRef, CompilerFactory, Component, NgModule, NgModuleRef, NgZone, PlatformRef, destroyPlatform, getPlatform} from '@angular/core';
import {async, inject} from '@angular/core/testing'; import {TestBed, async, inject} from '@angular/core/testing';
import {Http, HttpModule, Response, ResponseOptions, XHRBackend} from '@angular/http'; import {Http, HttpModule, Response, ResponseOptions, XHRBackend} from '@angular/http';
import {MockBackend, MockConnection} from '@angular/http/testing'; import {MockBackend, MockConnection} from '@angular/http/testing';
import {DOCUMENT} from '@angular/platform-browser'; import {DOCUMENT} from '@angular/platform-browser';
import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter'; import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter';
import {INITIAL_CONFIG, PlatformState, ServerModule, platformDynamicServer, renderModule, renderModuleFactory} from '@angular/platform-server'; import {INITIAL_CONFIG, PlatformState, ServerModule, platformDynamicServer, renderModule, renderModuleFactory} from '@angular/platform-server';
import {Subscription} from 'rxjs/Subscription'; import {Subscription} from 'rxjs/Subscription';
import {filter} from 'rxjs/operator/filter'; import {filter} from 'rxjs/operator/filter';
import {first} from 'rxjs/operator/first'; import {first} from 'rxjs/operator/first';
@ -99,6 +99,25 @@ class ImageExampleModule {
} }
export function main() { export function main() {
describe('regular', () => { declareTests({viewEngine: false}); });
describe('view engine', () => {
beforeEach(() => {
TestBed.configureCompiler({
useJit: true,
providers: [{
provide: USE_VIEW_ENGINE,
useValue: true,
}],
});
});
declareTests({viewEngine: true});
});
}
function declareTests({viewEngine}: {viewEngine: boolean}) {
if (getDOM().supportsDOMEvents()) return; // NODE only if (getDOM().supportsDOMEvents()) return; // NODE only
describe('platform-server integration', () => { describe('platform-server integration', () => {