iSharkFly-Docs
/
angular-docs-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ivy): don't create TNodes for native projectable nodes (#28275) 

Before this commit we were creating a "fake" TNode for each and every
projectable node passed during dynamic component creation. This approach
had several problems:
- the existing TView structure had to be mutated to accomodate new TNodes and
it was very easy to "corrupt" TView / TNode data structures;
- TNodes are not really needed to fully support projectable nodes so we were
creating objects and updating existing data structures for nothing.

This commit changes the approach so we don't create "fake" TNodes for projectable
nodes but instead we process projectable nodes directly in the projection instruction.
As a result we've got less code, less object allocation and - as a bonus - we fix few
bugs where TView / TNode data structures were corrupted when using projectable nodes.

PR Close #28275
This commit is contained in:
Pawel Kozlowski 2019-01-21 14:55:37 +01:00 committed by Alex Rickabaugh
parent d8f2318811
commit cf8770f3cc
11 changed files with 369 additions and 210 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
packages/core/src/render3/component_ref.ts
View File

@ -24,11 +24,11 @@ import {assertComponentType} from './assert';
import {LifecycleHooksFeature, createRootComponent, createRootComponentView, createRootContext} from './component';
import {getComponentDef} from './definition';
import {NodeInjector} from './di';
import {addToViewTree, createLView, createNodeAtIndex, createTView, createViewNode, elementCreate, locateHostElement, refreshDescendantViews} from './instructions';
import {ComponentDef, RenderFlags} from './interfaces/definition';
import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeType} from './interfaces/node';
import {RElement, RendererFactory3, domRendererFactory3, isProceduralRenderer} from './interfaces/renderer';
import {HEADER_OFFSET, LView, LViewFlags, RootContext, TVIEW} from './interfaces/view';
import {addToViewTree, createLView, createTView, createViewNode, elementCreate, locateHostElement, refreshDescendantViews} from './instructions';
import {ComponentDef} from './interfaces/definition';
import {TContainerNode, TElementContainerNode, TElementNode} from './interfaces/node';
import {RNode, RendererFactory3, domRendererFactory3, isProceduralRenderer} from './interfaces/renderer';
import {HEADER_OFFSET, LView, LViewFlags, RootContext} from './interfaces/view';
import {enterView, leaveView} from './state';
import {defaultScheduler, getTNode} from './util';
import {createElementRef} from './view_engine_compatibility';
@ -174,34 +174,12 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {
tElementNode = getTNode(0, rootLView) as TElementNode;
// Transform the arrays of native nodes into a structure that can be consumed by the
// projection instruction. This is needed to support the reprojection of these nodes.
if (projectableNodes) {
let index = 0;
const tView = rootLView[TVIEW];
const projection: TNode[] = tElementNode.projection = [];
for (let i = 0; i < projectableNodes.length; i++) {
const nodeList = projectableNodes[i];
let firstTNode: TNode|null = null;
let previousTNode: TNode|null = null;
for (let j = 0; j < nodeList.length; j++) {
if (tView.firstTemplatePass) {
// For dynamically created components such as ComponentRef, we create a new TView for
// each insert. This is not ideal since we should be sharing the TViews.
// Also the logic here should be shared with `component.ts`'s `renderComponent`
// method.
tView.expandoStartIndex++;
tView.blueprint.splice(++index + HEADER_OFFSET, 0, null);
tView.data.splice(index + HEADER_OFFSET, 0, null);
rootLView.splice(index + HEADER_OFFSET, 0, null);
}
const tNode =
createNodeAtIndex(index, TNodeType.Element, nodeList[j] as RElement, null, null);
previousTNode ? (previousTNode.next = tNode) : (firstTNode = tNode);
previousTNode = tNode;
}
projection.push(firstTNode !);
}
// projectable nodes can be passed as array of arrays or an array of iterables (ngUpgrade
// case). Here we do normalize passed data structure to be an array of arrays to avoid
// complex checks down the line.
tElementNode.projection =
projectableNodes.map((nodesforSlot: RNode[]) => { return Array.from(nodesforSlot); });
}
// TODO: should LifecycleHooksFeature and other host features be generated by the compiler and

12
packages/core/src/render3/instructions.ts
View File

@ -2471,15 +2471,21 @@ export function projection(nodeIndex: number, selectorIndex: number = 0, attrs?:
let projectedView = componentView[PARENT] !;
let projectionNodeIndex = -1;
if (Array.isArray(nodeToProject)) {
appendChild(nodeToProject, tProjectionNode, lView);
} else {
while (nodeToProject) {
if (nodeToProject.type === TNodeType.Projection) {
// This node is re-projected, so we must go up the tree to get its projected nodes.
const currentComponentView = findComponentView(projectedView);
const currentComponentHost = currentComponentView[HOST_NODE] as TElementNode;
const firstProjectedNode =
(currentComponentHost.projection as(TNode | null)[])[nodeToProject.projection as number];
const firstProjectedNode = (currentComponentHost.projection as(
TNode | null)[])[nodeToProject.projection as number];
if (firstProjectedNode) {
if (Array.isArray(firstProjectedNode)) {
appendChild(firstProjectedNode, tProjectionNode, lView);
} else {
projectionNodeStack[++projectionNodeIndex] = nodeToProject;
projectionNodeStack[++projectionNodeIndex] = projectedView;
@ -2487,6 +2493,7 @@ export function projection(nodeIndex: number, selectorIndex: number = 0, attrs?:
projectedView = currentComponentView[PARENT] !;
continue;
}
}
} else {
// This flag must be set now or we won't know that this node is projected
// if the nodes are inserted into a container later.
@ -2503,6 +2510,7 @@ export function projection(nodeIndex: number, selectorIndex: number = 0, attrs?:
nodeToProject = nodeToProject.next;
}
}
}
/**
* Adds LView or LContainer to the end of the current view tree.

12
packages/core/src/render3/interfaces/node.ts
View File

@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {RNode} from './renderer';
import {StylingContext} from './styling';
import {LView, TView} from './view';
@ -333,8 +334,11 @@ export interface TNode {
* `getHost(currentTNode).projection[currentTNode.projection]`.
* - When projecting nodes the parent node retrieved may be a `<ng-content>` node, in which case
* the process is recursive in nature (not implementation).
*
* If `projection` is of type `RNode[][]` than we have a collection of native nodes passed as
* projectable nodes during dynamic component creation.
*/
projection: (TNode|null)[]|number|null;
projection: (TNode|RNode[])[]|number|null;
}
/** Static data for an element */
@ -352,10 +356,10 @@ export interface TElementNode extends TNode {
/**
* If this is a component TNode with projection, this will be an array of projected
* TNodes (see TNode.projection for more info). If it's a regular element node or a
* component without projection, it will be null.
* TNodes or native nodes (see TNode.projection for more info). If it's a regular element node or
* a component without projection, it will be null.
*/
projection: (TNode|null)[]|null;
projection: (TNode|RNode[])[]|null;
}
/** Static data for a text node */

75
packages/core/src/render3/node_manipulation.ts
View File

@ -9,7 +9,7 @@
import {attachPatchData} from './context_discovery';
import {callHooks} from './hooks';
import {LContainer, NATIVE, VIEWS, unusedValueExportToPlacateAjd as unused1} from './interfaces/container';
import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeFlags, TNodeType, TViewNode, unusedValueExportToPlacateAjd as unused2} from './interfaces/node';
import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeFlags, TNodeType, TProjectionNode, TViewNode, unusedValueExportToPlacateAjd as unused2} from './interfaces/node';
import {unusedValueExportToPlacateAjd as unused3} from './interfaces/projection';
import {ProceduralRenderer3, RComment, RElement, RNode, RText, Renderer3, isProceduralRenderer, unusedValueExportToPlacateAjd as unused4} from './interfaces/renderer';
import {CLEANUP, CONTAINER_INDEX, FLAGS, HEADER_OFFSET, HOST_NODE, HookData, LView, LViewFlags, NEXT, PARENT, QUERIES, RENDERER, TVIEW, unusedValueExportToPlacateAjd as unused5} from './interfaces/view';
@ -109,14 +109,22 @@ function walkTNodeTree(
const head: TNode|null =
(componentHost.projection as(TNode | null)[])[tNode.projection as number];
if (Array.isArray(head)) {
for (let nativeNode of head) {
executeNodeAction(action, renderer, renderParent, nativeNode, tNode, beforeNode);
}
} else {
// Must store both the TNode and the view because this projection node could be nested
// deeply inside embedded views, and we need to get back down to this particular nested view.
// deeply inside embedded views, and we need to get back down to this particular nested
// view.
projectionNodeStack[++projectionNodeIndex] = tNode;
projectionNodeStack[++projectionNodeIndex] = currentView !;
if (head) {
currentView = componentView[PARENT] !;
nextTNode = currentView[TVIEW].data[head.index] as TNode;
}
}
} else {
// Otherwise, this is a View or an ElementContainer
nextTNode = tNode.child;
@ -548,6 +556,23 @@ export function nativeInsertBefore(
}
}
function nativeAppendChild(renderer: Renderer3, parent: RElement, child: RNode): void {
if (isProceduralRenderer(renderer)) {
renderer.appendChild(parent, child);
} else {
parent.appendChild(child);
}
}
function nativeAppendOrInsertBefore(
renderer: Renderer3, parent: RElement, child: RNode, beforeNode: RNode | null) {
if (beforeNode) {
nativeInsertBefore(renderer, parent, child, beforeNode);
} else {
nativeAppendChild(renderer, parent, child);
}
}
/**
* Removes a native child node from a given native parent node.
*/
@ -572,35 +597,47 @@ export function nativeNextSibling(renderer: Renderer3, node: RNode): RNode|null
}
/**
* Appends the `child` element to the `parent`.
* Finds a native "anchor" node for cases where we can't append a native child directly
* (`appendChild`) and need to use a reference (anchor) node for the `insertBefore` operation.
* @param parentTNode
* @param lView
*/
function getNativeAnchorNode(parentTNode: TNode, lView: LView): RNode|null {
if (parentTNode.type === TNodeType.View) {
const lContainer = getLContainer(parentTNode as TViewNode, lView) !;
const views = lContainer[VIEWS];
const index = views.indexOf(lView);
return getBeforeNodeForView(index, views, lContainer[NATIVE]);
} else if (
parentTNode.type === TNodeType.ElementContainer ||
parentTNode.type === TNodeType.IcuContainer) {
return getNativeByTNode(parentTNode, lView);
}
return null;
}
/**
* Appends the `child` native node (or a collection of nodes) to the `parent`.
*
* The element insertion might be delayed {@link canInsertNativeNode}.
*
* @param childEl The child that should be appended
* @param childEl The native child (or children) that should be appended
* @param childTNode The TNode of the child element
* @param currentView The current LView
* @returns Whether or not the child was appended
*/
export function appendChild(childEl: RNode, childTNode: TNode, currentView: LView): void {
export function appendChild(childEl: RNode | RNode[], childTNode: TNode, currentView: LView): void {
const renderParent = getRenderParent(childTNode, currentView);
if (renderParent != null) {
const renderer = currentView[RENDERER];
const parentTNode: TNode = childTNode.parent || currentView[HOST_NODE] !;
if (parentTNode.type === TNodeType.View) {
const lContainer = getLContainer(parentTNode as TViewNode, currentView) !;
const views = lContainer[VIEWS];
const index = views.indexOf(currentView);
nativeInsertBefore(
renderer, renderParent, childEl, getBeforeNodeForView(index, views, lContainer[NATIVE]));
} else if (
parentTNode.type === TNodeType.ElementContainer ||
parentTNode.type === TNodeType.IcuContainer) {
const anchorNode = getNativeByTNode(parentTNode, currentView);
nativeInsertBefore(renderer, renderParent, childEl, anchorNode);
const anchorNode = getNativeAnchorNode(parentTNode, currentView);
if (Array.isArray(childEl)) {
for (let nativeNode of childEl) {
nativeAppendOrInsertBefore(renderer, renderParent, nativeNode, anchorNode);
}
} else {
isProceduralRenderer(renderer) ? renderer.appendChild(renderParent, childEl) :
renderParent.appendChild(childEl);
nativeAppendOrInsertBefore(renderer, renderParent, childEl, anchorNode);
}
}
}

12
packages/core/test/bundling/hello_world/bundle.golden_symbols.json
View File

@ -119,6 +119,9 @@
{
"name": "__self"
},
{
"name": "__values"
},
{
"name": "__window"
},
@ -254,6 +257,9 @@
{
"name": "getLViewChild"
},
{
"name": "getNativeAnchorNode"
},
{
"name": "getNativeByTNode"
},
@ -344,6 +350,12 @@
{
"name": "namespaceHTML"
},
{
"name": "nativeAppendChild"
},
{
"name": "nativeAppendOrInsertBefore"
},
{
"name": "nativeInsertBefore"
},

12
packages/core/test/bundling/todo/bundle.golden_symbols.json
View File

@ -275,6 +275,9 @@
{
"name": "__spread"
},
{
"name": "__values"
},
{
"name": "__window"
},
@ -704,6 +707,9 @@
{
"name": "getMultiStartIndex"
},
{
"name": "getNativeAnchorNode"
},
{
"name": "getNativeByIndex"
},
@ -983,6 +989,12 @@
{
"name": "namespaceHTML"
},
{
"name": "nativeAppendChild"
},
{
"name": "nativeAppendOrInsertBefore"
},
{
"name": "nativeInsertBefore"
},

119
packages/core/test/linker/projection_integration_spec.ts
View File

@ -6,8 +6,8 @@
* found in the LICENSE file at https://angular.io/license
*/
import {Component, Directive, ElementRef, TemplateRef, ViewContainerRef, ViewEncapsulation} from '@angular/core';
import {TestBed} from '@angular/core/testing';
import {Component, ComponentFactoryResolver, ComponentRef, Directive, ElementRef, Injector, NgModule, OnInit, TemplateRef, ViewChild, ViewContainerRef, ViewEncapsulation} from '@angular/core';
import {ComponentFixture, TestBed} from '@angular/core/testing';
import {By} from '@angular/platform-browser/src/dom/debug/by';
import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter';
import {expect} from '@angular/platform-browser/testing/src/matchers';
@ -611,6 +611,121 @@ describe('projection', () => {
main.detectChanges();
expect(main.nativeElement).toHaveText('(, D)');
});
describe('projectable nodes', () => {
@Component({selector: 'test', template: ''})
class TestComponent {
constructor(public cfr: ComponentFactoryResolver) {}
}
@Component({selector: 'with-content', template: ''})
class WithContentCmpt {
@ViewChild('ref') directiveRef: any;
}
@Component({selector: 're-project', template: '<ng-content></ng-content>'})
class ReProjectCmpt {
}
@Directive({selector: '[insert]'})
class InsertTplRef implements OnInit {
constructor(private _vcRef: ViewContainerRef, private _tplRef: TemplateRef<{}>) {}
ngOnInit() { this._vcRef.createEmbeddedView(this._tplRef); }
}
@Directive({selector: '[delayedInsert]', exportAs: 'delayedInsert'})
class DelayedInsertTplRef {
constructor(public vc: ViewContainerRef, public templateRef: TemplateRef<Object>) {}
show() { this.vc.createEmbeddedView(this.templateRef); }
hide() { this.vc.clear(); }
}
@NgModule({
declarations: [WithContentCmpt, InsertTplRef, DelayedInsertTplRef, ReProjectCmpt],
entryComponents: [WithContentCmpt]
})
class TestModule {
}
let fixture: ComponentFixture<TestComponent>;
function createCmptInstance(
tpl: string, projectableNodes: any[][]): ComponentRef<WithContentCmpt> {
TestBed.configureTestingModule({declarations: [TestComponent], imports: [TestModule]});
TestBed.overrideTemplate(WithContentCmpt, tpl);
fixture = TestBed.createComponent(TestComponent);
const cfr = fixture.componentInstance.cfr;
const cf = cfr.resolveComponentFactory(WithContentCmpt);
const cmptRef = cf.create(Injector.NULL, projectableNodes);
cmptRef.changeDetectorRef.detectChanges();
return cmptRef;
}
it('should pass nodes to the default ng-content without selectors', () => {
const cmptRef = createCmptInstance(
'<div>(<ng-content></ng-content>)</div>', [[document.createTextNode('A')]]);
expect(cmptRef.location.nativeElement).toHaveText('(A)');
});
it('should pass nodes to the default ng-content at the root', () => {
const cmptRef =
createCmptInstance('<ng-content></ng-content>', [[document.createTextNode('A')]]);
expect(cmptRef.location.nativeElement).toHaveText('A');
});
it('should pass nodes to multiple ng-content tags', () => {
const cmptRef = createCmptInstance(
'A:(<ng-content></ng-content>)B:(<ng-content select="b"></ng-content>)C:(<ng-content select="c"></ng-content>)',
[
[document.createTextNode('A')], [document.createTextNode('B')],
[document.createTextNode('C')]
]);
expect(cmptRef.location.nativeElement).toHaveText('A:(A)B:(B)C:(C)');
});
it('should pass nodes to the default ng-content inside ng-container', () => {
const cmptRef = createCmptInstance(
'A<ng-container>(<ng-content></ng-content>)</ng-container>C',
[[document.createTextNode('B')]]);
expect(cmptRef.location.nativeElement).toHaveText('A(B)C');
});
it('should pass nodes to the default ng-content inside an embedded view', () => {
const cmptRef = createCmptInstance(
'A<ng-template insert>(<ng-content></ng-content>)</ng-template>C',
[[document.createTextNode('B')]]);
expect(cmptRef.location.nativeElement).toHaveText('A(B)C');
});
it('should pass nodes to the default ng-content inside a delayed embedded view', () => {
const cmptRef = createCmptInstance(
'A(<ng-template #ref="delayedInsert" delayedInsert>[<ng-content></ng-content>]</ng-template>)C',
[[document.createTextNode('B')]]);
expect(cmptRef.location.nativeElement).toHaveText('A()C');
const delayedInsert = cmptRef.instance.directiveRef as DelayedInsertTplRef;
delayedInsert.show();
cmptRef.changeDetectorRef.detectChanges();
expect(cmptRef.location.nativeElement).toHaveText('A([B])C');
delayedInsert.hide();
cmptRef.changeDetectorRef.detectChanges();
expect(cmptRef.location.nativeElement).toHaveText('A()C');
});
it('should re-project at the root', () => {
const cmptRef = createCmptInstance(
'A[<re-project>(<ng-content></ng-content>)</re-project>]C',
[[document.createTextNode('B')]]);
expect(cmptRef.location.nativeElement).toHaveText('A[(B)]C');
});
});
});
@Component({selector: 'main', template: ''})

1
packages/core/test/render3/renderer_factory_spec.ts
View File

@ -13,7 +13,6 @@ import {RendererType2, ViewEncapsulation} from '../../src/core';
import {defineComponent} from '../../src/render3/index';
import {bind, container, containerRefreshEnd, containerRefreshStart, element, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, listener, text, tick} from '../../src/render3/instructions';
import {RenderFlags} from '../../src/render3/interfaces/definition';
import {createRendererType2} from '../../src/view/index';
import {getAnimationRendererFactory2, getRendererFactory2} from './imported_renderer2';
import {TemplateFixture, containerEl, document, renderComponent, renderToHtml, toHtml} from './render_util';

7
packages/upgrade/test/dynamic/upgrade_spec.ts
View File

@ -10,7 +10,6 @@ import {ChangeDetectorRef, Component, EventEmitter, Input, NO_ERRORS_SCHEMA, NgM
import {async, fakeAsync, flushMicrotasks, tick} from '@angular/core/testing';
import {BrowserModule} from '@angular/platform-browser';
import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
import {fixmeIvy} from '@angular/private/testing';
import * as angular from '@angular/upgrade/src/common/angular1';
import {$EXCEPTION_HANDLER} from '@angular/upgrade/src/common/constants';
import {UpgradeAdapter, UpgradeAdapterRef} from '@angular/upgrade/src/dynamic/upgrade_adapter';
@ -3098,12 +3097,10 @@ withEachNg1Version(() => {
});
}));
fixmeIvy('FW-873: projected component injector hierarchy not wired up correctly')
.it('should respect hierarchical dependency injection for ng2', async(() => {
it('should respect hierarchical dependency injection for ng2', async(() => {
const ng1Module = angular.module('ng1', []);
@Component(
{selector: 'ng2-parent', template: `ng2-parent(<ng-content></ng-content>)`})
@Component({selector: 'ng2-parent', template: `ng2-parent(<ng-content></ng-content>)`})
class Ng2Parent {
}
@Component({selector: 'ng2-child', template: `ng2-child`})

3
packages/upgrade/test/static/integration/downgrade_component_spec.ts
View File

@ -705,8 +705,7 @@ withEachNg1Version(() => {
});
}));
fixmeIvy('FW-873: projected component injector hierarchy not wired up correctly')
.it('should respect hierarchical dependency injection for ng2', async(() => {
it('should respect hierarchical dependency injection for ng2', async(() => {
@Component({selector: 'parent', template: 'parent(<ng-content></ng-content>)'})
class ParentComponent {
}

6
packages/upgrade/test/static/integration/downgrade_module_spec.ts
View File

@ -330,8 +330,7 @@ withEachNg1Version(() => {
expect(multiTrim(element.children[1].textContent)).toBe('Counter:1');
}));
fixmeIvy('FW-873: projected component injector hierarchy not wired up correctly')
.it('should correctly traverse the injector tree of downgraded components', async(() => {
it('should correctly traverse the injector tree of downgraded components', async(() => {
@Component({
selector: 'ng2A',
template: 'ng2A(<ng-content></ng-content>)',
@ -392,8 +391,7 @@ withEachNg1Version(() => {
.directive(
'ng2A', downgradeComponent({component: Ng2ComponentA, propagateDigest}))
.directive(
'ng2B',
downgradeComponent({component: Ng2ComponentB, propagateDigest}));
'ng2B', downgradeComponent({component: Ng2ComponentB, propagateDigest}));
const element = html(`
<ng2-a><ng2-b ng-if="showB1"></ng2-b></ng2-a>