Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FW-869: Refactor view container and view manipulations for embedded views and projections #29031

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion integration/_payload-limits.json
Expand Up @@ -12,7 +12,7 @@
"master": {
"uncompressed": {
"runtime": 1440,
"main": 13921,
"main": 13620,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

"polyfills": 38390
}
}
Expand Down
Expand Up @@ -62,8 +62,9 @@ export class AnimationEngine {
this._transitionEngine.destroy(namespaceId, context);
}

onInsert(namespaceId: string, element: any, parent: any, insertBefore: boolean): void {
this._transitionEngine.insertNode(namespaceId, element, parent, insertBefore);
onInsert(namespaceId: string, element: any, parent: any, shouldCollectEnterElement: boolean):
void {
this._transitionEngine.insertNode(namespaceId, element, parent, shouldCollectEnterElement);
}

onRemove(namespaceId: string, element: any, context: any, isHostElement?: boolean): void {
Expand Down
Expand Up @@ -657,7 +657,8 @@ export class TransitionAnimationEngine {
return false;
}

insertNode(namespaceId: string, element: any, parent: any, insertBefore: boolean): void {
insertNode(namespaceId: string, element: any, parent: any, shouldCollectEnterElement: boolean):
void {
if (!isElementNode(element)) return;

// special case for when an element is removed and reinserted (move operation)
Expand Down Expand Up @@ -688,8 +689,8 @@ export class TransitionAnimationEngine {
}
}

// only *directives and host elements are inserted before
if (insertBefore) {
// For embedded views (*directives)
if (shouldCollectEnterElement) {
this.collectEnterElement(element);
}
}
Expand Down
21 changes: 17 additions & 4 deletions packages/core/src/linker/query_list.ts
Expand Up @@ -93,20 +93,33 @@ export class QueryList<T>/* implements Iterable<T> */ {
return this._results.some(fn);
}

/**
* Returns a copy of the internal results list as an Array.
*/
toArray(): T[] { return this._results.slice(); }

[getSymbolIterator()](): Iterator<T> { return (this._results as any)[getSymbolIterator()](); }

toString(): string { return this._results.toString(); }

reset(res: Array<T|any[]>): void {
this._results = flatten(res);
/**
* Updates the stored data of the query list, and resets the `dirty` flag to `false`, so that
* on change detection, it will not notify of changes to the queries, unless a new change
* occurs.
*
* @param resultsTree The results tree to store
*/
reset(resultsTree: Array<T|any[]>): void {
this._results = depthFirstFlatten(resultsTree);
(this as{dirty: boolean}).dirty = false;
(this as{length: number}).length = this._results.length;
(this as{last: T}).last = this._results[this.length - 1];
(this as{first: T}).first = this._results[0];
}

/**
* Triggers a change event by emitting on the `changes` {@link EventEmitter}.
*/
notifyOnChanges(): void { (this.changes as EventEmitter<any>).emit(this); }

/** internal */
Expand All @@ -119,9 +132,9 @@ export class QueryList<T>/* implements Iterable<T> */ {
}
}

function flatten<T>(list: Array<T|T[]>): T[] {
function depthFirstFlatten<T>(list: Array<T|T[]>): T[] {
return list.reduce((flat: any[], item: T | T[]): T[] => {
const flatItem = Array.isArray(item) ? flatten(item) : item;
const flatItem = Array.isArray(item) ? depthFirstFlatten(item) : item;
return (<T[]>flat).concat(flatItem);
}, []);
}
18 changes: 8 additions & 10 deletions packages/core/src/render3/component.ts
Expand Up @@ -11,24 +11,24 @@
import {Type} from '../core';
import {Injector} from '../di/injector';
import {Sanitizer} from '../sanitization/security';
import {assertDefined} from '../util/assert';

import {assertComponentType} from './assert';
import {getComponentDef} from './definition';
import {diPublicInInjector, getOrCreateNodeInjectorForNode} from './di';
import {registerPostOrderHooks, registerPreOrderHooks} from './hooks';
import {CLEAN_PROMISE, addToViewTree, createLView, createNodeAtIndex, createTView, getOrCreateTView, initNodeFlags, instantiateRootComponent, invokeHostBindingsInCreationMode, locateHostElement, queueComponentIndexForCheck, refreshDescendantViews} from './instructions';
import {ComponentDef, ComponentType, RenderFlags} from './interfaces/definition';
import {CLEAN_PROMISE, createLView, createNodeAtIndex, createTView, getOrCreateTView, initNodeFlags, instantiateRootComponent, invokeHostBindingsInCreationMode, locateHostElement, queueComponentIndexForCheck, refreshDescendantViews} from './instructions';
import {ComponentDef, ComponentType} from './interfaces/definition';
import {TElementNode, TNode, TNodeFlags, TNodeType} from './interfaces/node';
import {PlayerHandler} from './interfaces/player';
import {RElement, Renderer3, RendererFactory3, domRendererFactory3} from './interfaces/renderer';
import {CONTEXT, FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, RENDERER, RootContext, RootContextFlags, TVIEW} from './interfaces/view';
import {appendChildView} from './node_manipulation';
import {applyOnCreateInstructions} from './node_util';
import {enterView, getPreviousOrParentTNode, leaveView, resetComponentState, setCurrentDirectiveDef} from './state';
import {enterView, getPreviousOrParentTNode, leaveView, resetComponentState} from './state';
import {renderInitialClasses, renderInitialStyles} from './styling/class_and_style_bindings';
import {publishDefaultGlobalUtils} from './util/global_utils';
import {defaultScheduler, renderStringify} from './util/misc_utils';
import {getRootContext, getRootView} from './util/view_traversal_utils';
import {getRootContext} from './util/view_traversal_utils';
import {readPatchedLView, resetPreOrderHookFlags} from './util/view_utils';


Expand Down Expand Up @@ -87,9 +87,7 @@ type HostFeature = (<T>(component: T, componentDef: ComponentDef<T>) => void);

// TODO: A hack to not pull in the NullInjector from @angular/core.
export const NULL_INJECTOR: Injector = {
get: (token: any, notFoundValue?: any) => {
throw new Error('NullInjector: Not found: ' + renderStringify(token));
}
get: (token: any) => { throw new Error('NullInjector: Not found: ' + renderStringify(token));}
};

/**
Expand Down Expand Up @@ -130,15 +128,15 @@ export function renderComponent<T>(
rendererFactory, renderer, undefined, opts.injector || null);

const oldView = enterView(rootView, null);
let component: T;
let component: T = null !;
benlesh marked this conversation as resolved.
Show resolved Hide resolved
try {
if (rendererFactory.begin) rendererFactory.begin();
const componentView = createRootComponentView(
hostRNode, componentDef, rootView, rendererFactory, renderer, sanitizer);
component = createRootComponent(
componentView, componentDef, rootView, rootContext, opts.hostFeatures || null);

addToViewTree(rootView, componentView);
appendChildView(rootView, componentView);

refreshDescendantViews(rootView); // creation mode pass
rootView[FLAGS] &= ~LViewFlags.CreationMode;
Expand Down
24 changes: 13 additions & 11 deletions packages/core/src/render3/component_ref.ts
Expand Up @@ -26,14 +26,15 @@ import {assertComponentType} from './assert';
import {LifecycleHooksFeature, createRootComponent, createRootComponentView, createRootContext} from './component';
import {getComponentDef} from './definition';
import {NodeInjector} from './di';
import {addToViewTree, assignTViewNodeToLView, createLView, createTView, elementCreate, locateHostElement, refreshDescendantViews} from './instructions';
import {assignTViewNodeToLView, createLView, createTView, 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, TVIEW} from './interfaces/view';
import {LViewFlags, RootContext, TVIEW, View} from './interfaces/view';
import {appendChildView} from './node_manipulation';
import {enterView, leaveView} from './state';
import {defaultScheduler} from './util/misc_utils';
import {getTNode} from './util/view_utils';
import {getTNode, lViewToView, viewToLView} from './util/view_utils';
import {createElementRef} from './view_engine_compatibility';
import {RootViewRef, ViewRef} from './view_ref';

Expand Down Expand Up @@ -169,8 +170,8 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {
// rootView is the parent when bootstrapping
const oldLView = enterView(rootLView, null);

let component: T;
let tElementNode: TElementNode;
let component: T = null !;
let tElementNode: TElementNode = null !;
benlesh marked this conversation as resolved.
Show resolved Hide resolved
try {
const componentView = createRootComponentView(
hostRNode, this.componentDef, rootLView, rendererFactory, renderer);
Expand All @@ -191,15 +192,16 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {
component = createRootComponent(
componentView, this.componentDef, rootLView, rootContext, [LifecycleHooksFeature]);

addToViewTree(rootLView, componentView);
appendChildView(rootLView, componentView);
refreshDescendantViews(rootLView);
} finally {
leaveView(oldLView);
}

const componentRef = new ComponentRef(
this.componentType, component,
createElementRef(viewEngine_ElementRef, tElementNode, rootLView), rootLView, tElementNode);
createElementRef(viewEngine_ElementRef, tElementNode, rootLView), lViewToView(rootLView),
tElementNode);

if (isInternalRootView) {
// The host element of the internal root view is attached to the component's host view node
Expand Down Expand Up @@ -239,16 +241,16 @@ export class ComponentRef<T> extends viewEngine_ComponentRef<T> {

constructor(
componentType: Type<T>, instance: T, public location: viewEngine_ElementRef,
private _rootLView: LView,
private _tNode: TElementNode|TContainerNode|TElementContainerNode) {
private _rootView: View, private _tNode: TElementNode|TContainerNode|TElementContainerNode) {
super();
this.instance = instance;
this.hostView = this.changeDetectorRef = new RootViewRef<T>(_rootLView);
this.hostView = this.changeDetectorRef = new RootViewRef<T>(_rootView);
const _rootLView = viewToLView(_rootView);
this.hostView._tViewNode = assignTViewNodeToLView(_rootLView[TVIEW], null, -1, _rootLView);
this.componentType = componentType;
}

get injector(): Injector { return new NodeInjector(this._tNode, this._rootLView); }
get injector(): Injector { return new NodeInjector(this._tNode, viewToLView(this._rootView)); }

destroy(): void {
ngDevMode && assertDefined(this.destroyCbs, 'NgModule already destroyed');
Expand Down
23 changes: 14 additions & 9 deletions packages/core/src/render3/debug.ts
Expand Up @@ -79,13 +79,16 @@ export function toDebug(obj: any): any {
function toHtml(value: any, includeChildren: boolean = false): string|null {
const node: HTMLElement|null = unwrapRNode(value) as any;
if (node) {
const isTextNode = node.nodeType === Node.TEXT_NODE;
const outerHTML = (isTextNode ? node.textContent : node.outerHTML) || '';
if (includeChildren || isTextNode) {
return outerHTML;
} else {
const isElementNode = node.nodeType === Node.ELEMENT_NODE;
const outerHTML = (isElementNode ? node.outerHTML : node.textContent) || '';
if (node.nodeType === Node.COMMENT_NODE) {
return '<!--' + outerHTML + '-->';
} else if (isElementNode && !includeChildren) {
const innerHTML = node.innerHTML;
return outerHTML.split(innerHTML)[0] || null;
const outer = outerHTML.split('>' + innerHTML + '<')[0];
return outer + '>';
} else {
return outerHTML;
}
} else {
return null;
Expand Down Expand Up @@ -168,6 +171,7 @@ export interface DebugNode {
native: Node;
nodes: DebugNode[]|null;
component: LViewDebug|null;
tNode: TNode;
}

/**
Expand All @@ -181,14 +185,15 @@ export function toDebugNodes(tNode: TNode | null, lView: LView): DebugNode[]|nul
const debugNodes: DebugNode[] = [];
let tNodeCursor: TNode|null = tNode;
while (tNodeCursor) {
const rawValue = lView[tNode.index];
const rawValue = lView[tNodeCursor.index];
const native = unwrapRNode(rawValue);
const componentLViewDebug = toDebug(readLViewValue(rawValue));
debugNodes.push({
html: toHtml(native),
native: native as any,
nodes: toDebugNodes(tNode.child, lView),
component: componentLViewDebug
nodes: toDebugNodes(tNodeCursor.child, lView),
component: componentLViewDebug,
tNode: tNodeCursor,
});
tNodeCursor = tNodeCursor.next;
}
Expand Down
28 changes: 23 additions & 5 deletions packages/core/src/render3/i18n.ts
Expand Up @@ -9,17 +9,18 @@
import {SRCSET_ATTRS, URI_ATTRS, VALID_ATTRS, VALID_ELEMENTS, getTemplateContent} from '../sanitization/html_sanitizer';
import {InertBodyHelper} from '../sanitization/inert_body';
import {_sanitizeUrl, sanitizeSrcset} from '../sanitization/url_sanitizer';
import {assertDefined, assertEqual, assertGreaterThan} from '../util/assert';
import {assertDefined, assertDomNode, assertEqual, assertGreaterThan, isDomNode} from '../util/assert';

import {attachPatchData} from './context_discovery';
import {allocExpando, createNodeAtIndex, elementAttribute, load, textBinding} from './instructions';
import {LContainer, NATIVE} from './interfaces/container';
import {COMMENT_MARKER, ELEMENT_MARKER, I18nMutateOpCode, I18nMutateOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, IcuType, TI18n, TIcu} from './interfaces/i18n';
import {TElementNode, TIcuContainerNode, TNode, TNodeType} from './interfaces/node';
import {RComment, RElement, RText} from './interfaces/renderer';
import {RComment, RElement, RNode, RText} from './interfaces/renderer';
import {SanitizerFn} from './interfaces/sanitization';
import {StylingContext} from './interfaces/styling';
import {BINDING_INDEX, HEADER_OFFSET, LView, RENDERER, TVIEW, TView, T_HOST} from './interfaces/view';
import {appendChild, createTextNode, nativeRemoveNode} from './node_manipulation';
import {appendChild, createTextNode, getRenderParent, nativeInsertBefore, nativeParentNode, nativeRemoveNode} from './node_manipulation';
import {getIsParent, getLView, getPreviousOrParentTNode, setIsParent, setPreviousOrParentTNode} from './state';
import {NO_CHANGE} from './tokens';
import {addAllToArray} from './util/array_utils';
Expand Down Expand Up @@ -376,7 +377,7 @@ function i18nStartFirstPass(
const createOpCodes: I18nMutateOpCodes = [];
// If the previous node wasn't the direct parent then we have a translation without top level
// element and we need to keep a reference of the previous element if there is one
if (index > 0 && previousOrParentTNode !== parentTNode) {
if (index > 0 && parentTNode !== null && previousOrParentTNode !== parentTNode) {
// Create an OpCode to select the previous TNode
createOpCodes.push(
previousOrParentTNode.index << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.Select);
Expand Down Expand Up @@ -497,7 +498,22 @@ function appendI18nNode(tNode: TNode, parentTNode: TNode, previousTNode: TNode |
cursor = cursor.next;
}

appendChild(getNativeByTNode(tNode, viewData), tNode, viewData);
// Insert before this node
let beforeNode: RNode|null = null;
if (tNode.parent && tNode.parent.type === TNodeType.IcuContainer) {
beforeNode = getNativeByTNode(tNode.parent, viewData);

// HACK(benlesh): In some cases, ICU comment nodes were not appropriately added to the intended
// parent. This forces them to be added, so insertion of what's important below does not error.
// This is highly suspect, IMO, but it seems to have things working.
const renderer = viewData[RENDERER];
const renderParent = getRenderParent(tNode, viewData) !;
if (nativeParentNode(renderer, beforeNode) !== renderParent) {
nativeInsertBefore(renderer, renderParent, beforeNode, null);
}
}

appendChild(getNativeByTNode(tNode, viewData), tNode, viewData, beforeNode);

const slotValue = viewData[tNode.index];
if (tNode.type !== TNodeType.Container && isLContainer(slotValue)) {
Expand Down Expand Up @@ -660,6 +676,7 @@ function createDynamicNodeAtIndex(
index: number, type: TNodeType, native: RElement | RText | null,
name: string | null): TElementNode|TIcuContainerNode {
const previousOrParentTNode = getPreviousOrParentTNode();
ngDevMode && assertDefined(previousOrParentTNode, 'parent TNode required');
const tNode = createNodeAtIndex(index, type as any, native, name, null);

// We are creating a dynamic node, the previous tNode might not be pointing at this node.
Expand Down Expand Up @@ -869,6 +886,7 @@ function removeNode(index: number, viewData: LView) {
const removedPhTNode = getTNode(index, viewData);
const removedPhRNode = getNativeByIndex(index, viewData);
if (removedPhRNode) {
ngDevMode && assertDomNode(removedPhRNode);
nativeRemoveNode(viewData[RENDERER], removedPhRNode);
}

Expand Down