From 929fe029c2cfce93241afa14f34979b1a18c69c6 Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Fri, 25 Jan 2019 15:21:51 -0800 Subject: [PATCH] refactor(ivy): LView is a proper linked list (#28382) - TView no longer stores childIndex - LView now as CHILD_HEAD and CHILD_TAIL TView used to store the head of the list, therefor all LViews had to have the same head, which is incorrect. PR Close #28382 --- packages/core/src/render3/component_ref.ts | 2 +- packages/core/src/render3/discovery_utils.ts | 4 +-- packages/core/src/render3/instructions.ts | 12 +++---- packages/core/src/render3/interfaces/view.ts | 32 +++++++++---------- .../core/src/render3/node_manipulation.ts | 6 ---- .../src/render3/view_engine_compatibility.ts | 2 +- .../cyclic_import/bundle.golden_symbols.json | 3 ++ 7 files changed, 27 insertions(+), 34 deletions(-) diff --git a/packages/core/src/render3/component_ref.ts b/packages/core/src/render3/component_ref.ts index 15d856938b..ee81969b3e 100644 --- a/packages/core/src/render3/component_ref.ts +++ b/packages/core/src/render3/component_ref.ts @@ -190,7 +190,7 @@ export class ComponentFactory extends viewEngine_ComponentFactory { component = createRootComponent( componentView, this.componentDef, rootLView, rootContext, [LifecycleHooksFeature]); - addToViewTree(rootLView, HEADER_OFFSET, componentView); + addToViewTree(rootLView, componentView); refreshDescendantViews(rootLView); } finally { leaveView(oldLView); diff --git a/packages/core/src/render3/discovery_utils.ts b/packages/core/src/render3/discovery_utils.ts index a64bb4b5fa..25d870bb0b 100644 --- a/packages/core/src/render3/discovery_utils.ts +++ b/packages/core/src/render3/discovery_utils.ts @@ -101,9 +101,7 @@ export function getViewComponent(element: Element | {}): T|null { ngDevMode && assertLView(lView); while (lView[HOST] === null && (parent = getLViewParent(lView) !)) { // As long as lView[HOST] is null we know we are part of sub-template such as `*ngIf` - if (parent) { - lView = parent; - } + lView = parent; } return lView[FLAGS] & LViewFlags.IsRoot ? null : lView[CONTEXT] as T; } diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index b2ed0c1b89..8ed23c6aaf 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -36,7 +36,7 @@ import {SanitizerFn} from './interfaces/sanitization'; import {StylingContext} from './interfaces/styling'; import {BINDING_INDEX, CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_VIEW, ExpandoInstructions, FLAGS, HEADER_OFFSET, HOST, INJECTOR, InitPhaseState, LView, LViewFlags, NEXT, OpaqueViewState, PARENT, QUERIES, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, TData, TVIEW, TView, T_HOST} from './interfaces/view'; import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert'; -import {appendChild, appendProjectedNode, createTextNode, getLViewChild, insertView, removeView} from './node_manipulation'; +import {appendChild, appendProjectedNode, createTextNode, insertView, removeView} from './node_manipulation'; import {isNodeMatchingSelectorList, matchingSelectorIndex} from './node_selector_matcher'; import {decreaseElementDepthCount, enterView, getBindingsEnabled, getCheckNoChangesMode, getContextLView, getCurrentDirectiveDef, getElementDepthCount, getIsParent, getLView, getPreviousOrParentTNode, increaseElementDepthCount, isCreationMode, leaveView, nextContextImpl, resetComponentState, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setCurrentQueryIndex, setIsParent, setPreviousOrParentTNode,} from './state'; import {getInitialClassNameValue, getInitialStyleStringValue, initializeStaticContext as initializeStaticStylingContext, patchContextWithStaticAttrs, renderInitialClasses, renderInitialStyles, renderStyling, updateClassProp as updateElementClassProp, updateContextWithBindings, updateStyleProp as updateElementStyleProp, updateStylingMap} from './styling/class_and_style_bindings'; @@ -788,7 +788,6 @@ export function createTView( viewQuery: viewQuery, node: null !, data: blueprint.slice().fill(null, bindingStartIndex), - childIndex: -1, // Children set in addToViewTree(), if any bindingStartIndex: bindingStartIndex, viewQueryStartIndex: initialViewLength, expandoStartIndex: initialViewLength, @@ -2373,7 +2372,7 @@ export function containerRefreshEnd(): void { * by executing an associated template function. */ function refreshDynamicEmbeddedViews(lView: LView) { - for (let current = getLViewChild(lView); current !== null; current = current[NEXT]) { + for (let current = lView[CHILD_HEAD]; current !== null; current = current[NEXT]) { // Note: current can be an LView or an LContainer instance, but here we are only interested // in LContainer. We can tell it's an LContainer because its length is less than the LView // header. @@ -2704,13 +2703,14 @@ export function projection(nodeIndex: number, selectorIndex: number = 0, attrs?: * * @param lView The view where LView or LContainer should be added * @param adjustedHostIndex Index of the view's host node in LView[], adjusted for header - * @param state The LView or LContainer to add to the view tree + * @param lViewOrLContainer The LView or LContainer to add to the view tree * @returns The state passed in */ export function addToViewTree(lView: LView, lViewOrLContainer: T): T { // TODO(benlesh/misko): This implementation is incorrect, because it always adds the LContainer to - // the end of the queue, which means if the developer asks for the LContainers out of order, the - // change detection will run out of order. + // the end of the queue, which means if the developer retrieves the LContainers from RNodes out of + // order, the change detection will run out of order, as the act of retrieving the the LContainer + // from the RNode is what adds it to the queue. if (lView[CHILD_HEAD]) { lView[CHILD_TAIL] ![NEXT] = lViewOrLContainer; } else { diff --git a/packages/core/src/render3/interfaces/view.ts b/packages/core/src/render3/interfaces/view.ts index d38af4e2d4..f83e61b9cb 100644 --- a/packages/core/src/render3/interfaces/view.ts +++ b/packages/core/src/render3/interfaces/view.ts @@ -46,7 +46,7 @@ export const CHILD_TAIL = 15; export const CONTENT_QUERIES = 16; export const DECLARATION_VIEW = 17; /** Size of LView's header. Necessary to adjust for it when setting slots. */ -export const HEADER_OFFSET = 18; +export const HEADER_OFFSET = 19; // This interface replaces the real LView interface if it is an arg or a @@ -79,12 +79,13 @@ export interface LView extends Array { [FLAGS]: LViewFlags; /** - * The parent view is needed when we exit the view and must restore the previous - * `LView`. Without this, the render method would have to keep a stack of + * This may store an {@link LView} or {@link LContainer}. + * + * `LView` - The parent view. This is needed when we exit the view and must restore the previous + * LView. Without this, the render method would have to keep a stack of * views as it is recursively rendering templates. * - * This is the "insertion" view for embedded views. This allows us to properly - * destroy embedded views. + * `LContainer` - The current view is part of a container, and is an embedded view. */ [PARENT]: LView|LContainer|null; @@ -162,6 +163,15 @@ export interface LView extends Array { /** An optional custom sanitizer. */ [SANITIZER]: Sanitizer|null; + /** + * Reference to the first LView or LContainer beneath this LView in + * the hierarchy. + * + * Necessary to store this so views can traverse through their nested views + * to remove listeners and call onDestroy callbacks. + */ + [CHILD_HEAD]: LView|LContainer|null; + /** * The last LView or LContainer beneath this LView in the hierarchy. * @@ -393,18 +403,6 @@ export interface TView { */ viewQueryStartIndex: number; - /** - * Index of the host node of the first LView or LContainer beneath this LView in - * the hierarchy. - * - * Necessary to store this so views can traverse through their nested views - * to remove listeners and call onDestroy callbacks. - * - * For embedded views, we store the index of an LContainer's host rather than the first - * LView to avoid managing splicing when views are added/removed. - */ - childIndex: number; - /** * A reference to the first child node located in the view. */ diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index 8da441efc2..b77cd76de0 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -369,12 +369,6 @@ export function removeView(lContainer: LContainer, removeIndex: number) { destroyLView(view); } -/** Gets the child of the given LView */ -export function getLViewChild(lView: LView): LView|LContainer|null { - const childIndex = lView[TVIEW].childIndex; - return childIndex === -1 ? null : lView[childIndex]; -} - /** * A standalone function which destroys an LView, * conducting cleanup (e.g. removing listeners, calling onDestroys). diff --git a/packages/core/src/render3/view_engine_compatibility.ts b/packages/core/src/render3/view_engine_compatibility.ts index d8fff606c3..a601d2e3f3 100644 --- a/packages/core/src/render3/view_engine_compatibility.ts +++ b/packages/core/src/render3/view_engine_compatibility.ts @@ -314,7 +314,7 @@ export function createContainerRef( hostView[hostTNode.index] = lContainer = createLContainer(slotValue, hostView, commentNode, true); - addToViewTree(hostView, hostTNode.index as number, lContainer); + addToViewTree(hostView, lContainer); } return new R3ViewContainerRef(lContainer, hostTNode, hostView); diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json index 464e2b2a82..be2e574c15 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -59,6 +59,9 @@ { "name": "INJECTOR_BLOOM_PARENT_SIZE" }, + { + "name": "LCONTAINER_LENGTH" + }, { "name": "MONKEY_PATCH_KEY_NAME" },