From 9d19c8e31752d211f575246282358b83afe90969 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 11 Oct 2023 12:47:09 +0200 Subject: [PATCH] fix(compiler): don't allocate variable to for loop expression (#52158) Currently the compiler allocates a variable slot to the `@for` loop expression which ends up unused since we don't store the result on the `LView`. PR Close #52158 --- .../for_with_pipe_template.js | 2 +- .../nested_for_template.js | 2 +- .../nested_for_template_variables_template.js | 2 +- packages/compiler/src/render3/view/template.ts | 13 +++++++++---- .../core/src/render3/instructions/control_flow.ts | 4 +--- 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_with_pipe_template.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_with_pipe_template.js index ca27e748ff8af..f48267ae32b85 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_with_pipe_template.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_with_pipe_template.js @@ -9,6 +9,6 @@ function MyApp_Template(rf, ctx) { if (rf & 2) { $r3$.ɵɵadvance(1); $r3$.ɵɵtextInterpolate1(" ", ctx.message, " "); - $r3$.ɵɵrepeater(2, $r3$.ɵɵpipeBind1(4, 2, ctx.items)); + $r3$.ɵɵrepeater(2, $r3$.ɵɵpipeBind1(4, 1, ctx.items)); } } diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_template.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_template.js index f422179a096fa..596ac9770bb5a 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_template.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_template.js @@ -25,7 +25,7 @@ function MyApp_Template(rf, ctx) { if (rf & 1) { $r3$.ɵɵelementStart(0, "div"); $r3$.ɵɵtext(1); - $r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 3, 2, $r3$.ɵɵrepeaterTrackByIdentity); + $r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 3, 1, $r3$.ɵɵrepeaterTrackByIdentity); $r3$.ɵɵelementEnd(); } if (rf & 2) { diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_template_variables_template.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_template_variables_template.js index 2a1fd7630f0d4..b361148616a72 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_template_variables_template.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_template_variables_template.js @@ -25,7 +25,7 @@ function MyApp_Template(rf, ctx) { if (rf & 1) { $r3$.ɵɵelementStart(0, "div"); $r3$.ɵɵtext(1); - $r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 3, 2, $r3$.ɵɵrepeaterTrackByIdentity); + $r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 3, 1, $r3$.ɵɵrepeaterTrackByIdentity); $r3$.ɵɵelementEnd(); } if (rf & 2) { diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 3690108882705..84740788924a6 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -1471,11 +1471,15 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver const primaryData = this.prepareEmbeddedTemplateFn( block.children, '_For', [block.item, block.contextVariables.$index, block.contextVariables.$count]); - const emptyData = block.empty === null ? - null : - this.prepareEmbeddedTemplateFn(block.empty.children, '_ForEmpty'); const {expression: trackByExpression, usesComponentInstance: trackByUsesComponentInstance} = this.createTrackByFunction(block); + let emptyData: TemplateData|null = null; + + if (block.empty !== null) { + emptyData = this.prepareEmbeddedTemplateFn(block.empty.children, '_ForEmpty'); + // Allocate an extra slot for the empty block tracking. + this.allocateBindingSlots(null); + } this.registerComputedLoopVariables(block, primaryData.scope); @@ -1503,8 +1507,9 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver // Note: the expression needs to be processed *after* the template, // otherwise pipes injecting some symbols won't work (see #52102). + // Note: we don't allocate binding slots for this expression, + // because its value isn't stored in the LView. const value = block.expression.visit(this._valueConverter); - this.allocateBindingSlots(value); // `repeater(0, iterable)` this.updateInstruction( diff --git a/packages/core/src/render3/instructions/control_flow.ts b/packages/core/src/render3/instructions/control_flow.ts index c8765946713ba..26690c0567ebf 100644 --- a/packages/core/src/render3/instructions/control_flow.ts +++ b/packages/core/src/render3/instructions/control_flow.ts @@ -227,10 +227,8 @@ export function ɵɵrepeater( } // handle empty blocks - // PERF: maybe I could skip allocation of memory for the empty block? Isn't it the "fix" on the - // compiler side that we've been discussing? Talk to K & D! - const bindingIndex = nextBindingIndex(); if (metadata.hasEmptyBlock) { + const bindingIndex = nextBindingIndex(); const isCollectionEmpty = lContainer.length - CONTAINER_HEADER_OFFSET === 0; if (bindingUpdated(hostLView, bindingIndex, isCollectionEmpty)) { const emptyTemplateIndex = metadataSlotIdx + 2;