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
angular · Oct 11, 2023 · 9d19c8e · 9d19c8e
1 parent 77284d9
commit 9d19c8e
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 10 deletions.
diff --git a/...er-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_with_pipe_template.js b/...er-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/...piler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_for_template.js b/...piler-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/...liance/test_cases/r3_view_compiler_control_flow/nested_for_template_variables_template.js b/...liance/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
@@ -1471,11 +1471,15 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, 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<void>, 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
@@ -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;