diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index a53438d1c7..c63ca32ad8 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -582,6 +582,15 @@ class ViewContainerRef implements viewEngine_ViewContainerRef { const lView = (viewRef as EmbeddedViewRef)._lViewNode; insertView(this._node, lView, index); + // TODO(pk): this is a temporary index adjustment so imperativelly inserted (through + // ViewContainerRef) views + // are not removed in the containerRefreshEnd instruction. + // The final fix will consist of creating a dedicated container node for views inserted through + // ViewContainerRef. + // Such container should not be trimmed as it is the case in the containerRefreshEnd + // instruction. + this._node.data.nextIndex = this._node.data.views.length; + // If the view is dynamic (has a template), it needs to be counted both at the container // level and at the node above the container. if (lView.data.template !== null) { diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 36720f7cc8..1627ad66e5 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -1204,8 +1204,9 @@ export function containerRefreshEnd(): void { container.native = undefined; ngDevMode && assertNodeType(container, LNodeFlags.Container); const nextIndex = container.data.nextIndex; + + // remove extra views at the end of the container while (nextIndex < container.data.views.length) { - // remove extra view. removeView(container, nextIndex); } } @@ -1222,6 +1223,35 @@ function refreshDynamicChildren() { } } +/** + * Looks for a view with a given view block id inside a provided LContainer. + * Removes views that need to be deleted in the process. + * + * @param containerNode where to search for views + * @param startIdx starting index in the views array to search from + * @param viewBlockId exact view block id to look for + * @returns index of a found view or -1 if not found + */ +function scanForView( + containerNode: LContainerNode, startIdx: number, viewBlockId: number): LViewNode|null { + const views = containerNode.data.views; + for (let i = startIdx; i < views.length; i++) { + const viewAtPositionId = views[i].data.id; + if (viewAtPositionId === viewBlockId) { + return views[i]; + } else if (viewAtPositionId < viewBlockId) { + // found a view that should not be at this position - remove + removeView(containerNode, i); + } else { + // found a view with id grater than the one we are searching for + // which means that required view doesn't exist and can't be found at + // later positions in the views array - stop the search here + break; + } + } + return null; +} + /** * Marks the start of an embedded view. * @@ -1233,17 +1263,13 @@ export function embeddedViewStart(viewBlockId: number): boolean { (isParent ? previousOrParentNode : previousOrParentNode.parent !) as LContainerNode; ngDevMode && assertNodeType(container, LNodeFlags.Container); const lContainer = container.data; - const views = lContainer.views; + const existingViewNode = scanForView(container, lContainer.nextIndex, viewBlockId); - const existingView: LViewNode|false = - !creationMode && lContainer.nextIndex < views.length && views[lContainer.nextIndex]; - let viewUpdateMode = existingView && viewBlockId === (existingView as LViewNode).data.id; - - if (viewUpdateMode) { - previousOrParentNode = views[lContainer.nextIndex++]; + if (existingViewNode) { + previousOrParentNode = existingViewNode; ngDevMode && assertNodeType(previousOrParentNode, LNodeFlags.View); isParent = true; - enterView((existingView as LViewNode).data, previousOrParentNode as LViewNode); + enterView((existingViewNode as LViewNode).data, existingViewNode as LViewNode); } else { // When we create a new LView, we always reset the state of the instructions. const newView = createLView( @@ -1254,10 +1280,9 @@ export function embeddedViewStart(viewBlockId: number): boolean { } enterView(newView, createLNode(null, LNodeFlags.View, null, newView)); - lContainer.nextIndex++; } - return !viewUpdateMode; + return !existingViewNode; } /** @@ -1286,19 +1311,18 @@ export function embeddedViewEnd(): void { refreshChildComponents(); isParent = false; const viewNode = previousOrParentNode = currentView.node as LViewNode; - const container = previousOrParentNode.parent as LContainerNode; - if (container) { + const containerNode = previousOrParentNode.parent as LContainerNode; + if (containerNode) { ngDevMode && assertNodeType(viewNode, LNodeFlags.View); - ngDevMode && assertNodeType(container, LNodeFlags.Container); - const containerState = container.data; - const previousView = containerState.nextIndex <= containerState.views.length ? - containerState.views[containerState.nextIndex - 1] as LViewNode : - null; - const viewIdChanged = previousView == null ? true : previousView.data.id !== viewNode.data.id; + ngDevMode && assertNodeType(containerNode, LNodeFlags.Container); + const lContainer = containerNode.data; - if (viewIdChanged) { - insertView(container, viewNode, containerState.nextIndex - 1); + if (creationMode) { + // it is a new view, insert it into collection of views for a given container + insertView(containerNode, viewNode, lContainer.nextIndex); } + + lContainer.nextIndex++; } leaveView(currentView !.parent !); ngDevMode && assertEqual(isParent, false, 'isParent'); diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index 41a12425ea..6a71ecaadf 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -270,18 +270,13 @@ export function insertView( setViewNext(views[index - 1], newView); } - if (index < views.length && views[index].data.id !== newView.data.id) { - // View ID change replace the view. + if (index < views.length) { setViewNext(newView, views[index]); views.splice(index, 0, newView); - } else if (index >= views.length) { + } else { views.push(newView); } - if (state.nextIndex <= index) { - state.nextIndex++; - } - // If the container's renderParent is null, we know that it is a root node of its own parent view // and we should wait until that parent processes its nodes (otherwise, we will insert this view's // nodes twice - once now and once when its parent inserts its views). diff --git a/packages/core/test/render3/control_flow_spec.ts b/packages/core/test/render3/control_flow_spec.ts index 4337df5d18..462722988f 100644 --- a/packages/core/test/render3/control_flow_spec.ts +++ b/packages/core/test/render3/control_flow_spec.ts @@ -125,6 +125,54 @@ describe('JS control flow', () => { expect(renderToHtml(Template, ctx)).toEqual('
Hello
'); }); + it('should work with adjacent if blocks managing views in the same container', () => { + + const ctx = {condition1: true, condition2: true, condition3: true}; + + /** + * % if(ctx.condition1) { + * 1 + * % }; if(ctx.condition2) { + * 2 + * % }; if(ctx.condition3) { + * 3 + * % } + */ + function Template(ctx: any, cm: boolean) { + if (cm) { + container(0); + } + containerRefreshStart(0); + if (ctx.condition1) { + const cm1 = embeddedViewStart(1); + if (cm1) { + text(0, '1'); + } + embeddedViewEnd(); + } // can't have ; here due linting rules + if (ctx.condition2) { + const cm2 = embeddedViewStart(2); + if (cm2) { + text(0, '2'); + } + embeddedViewEnd(); + } // can't have ; here due linting rules + if (ctx.condition3) { + const cm3 = embeddedViewStart(3); + if (cm3) { + text(0, '3'); + } + embeddedViewEnd(); + } + containerRefreshEnd(); + } + + expect(renderToHtml(Template, ctx)).toEqual('123'); + + ctx.condition2 = false; + expect(renderToHtml(Template, ctx)).toEqual('13'); + }); + it('should work with containers with views as parents', () => { function Template(ctx: any, cm: boolean) { if (cm) { @@ -490,6 +538,15 @@ describe('JS for loop', () => { const ctx: {data1: string[] | null, data2: number[] | null} = {data1: ['a', 'b', 'c'], data2: [1, 2]}; + /** + *
+ * % for (let i = 0; i < ctx.data1.length; i++) { + * {{data1[i]}} + * % } for (let j = 0; j < ctx.data2.length; j++) { + * {{data1[j]}} + * % } + *
+ */ function Template(ctx: any, cm: boolean) { if (cm) { elementStart(0, 'div');