fix(compiler): prevent mutation of children array in RecursiveVisitor

atscott · atscott · commit 1df1697c6e3a · 2026-03-04T22:41:36.000Z
RecursiveVisitor.visitIfBlockBranch was permanently mutating the children array by pushing the expressionAlias into it. This change clones the array before pushing to avoid this side effect. (cherry picked from commit 72a17af)
diff --git a/packages/compiler/src/render3/r3_ast.ts b/packages/compiler/src/render3/r3_ast.ts
@@ -803,9 +803,8 @@ export class RecursiveVisitor implements Visitor<void> {
     visitAll(this, block.branches);
   }
   visitIfBlockBranch(block: IfBlockBranch): void {
-    const blockItems = block.children;
-    block.expressionAlias && blockItems.push(block.expressionAlias);
-    visitAll(this, blockItems);
+    visitAll(this, block.children);
+    block.expressionAlias?.visit(this);
   }
   visitContent(content: Content): void {
     visitAll(this, content.children);
diff --git a/packages/compiler/test/render3/r3_ast_visitor_spec.ts b/packages/compiler/test/render3/r3_ast_visitor_spec.ts
@@ -0,0 +1,52 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.dev/license
+ */
+
+import * as t from '../../src/render3/r3_ast';
+import {parseR3 as parse} from './view/util';
+
+describe('RecursiveVisitor', () => {
+  it('should not mutate IfBlockBranch children when visiting', () => {
+    // Template with an @if block that has an alias.
+    // The alias is key because `visitIfBlockBranch` previously pushed the alias
+    // to the children array, causing mutation if the array wasn't cloned.
+    const template = '@if (val; as alias) { <div></div> }';
+    const result = parse(template);
+
+    // Structure:
+    // result.nodes[0] is the IfBlock
+    // IfBlock.branches[0] is the IfBlockBranch
+    const ifBlock = result.nodes[0] as t.IfBlock;
+    expect(ifBlock instanceof t.IfBlock).toBe(true);
+
+    const branch = ifBlock.branches[0];
+    expect(branch instanceof t.IfBlockBranch).toBe(true);
+
+    // Verify initial state
+    // Children should contain only the Element (<div></div>)
+    expect(branch.children.length).toBe(1);
+    expect(branch.children[0] instanceof t.Element).toBe(true);
+    const element = branch.children[0] as t.Element;
+    expect(element.name).toBe('div');
+
+    // Alias should exist
+    expect(branch.expressionAlias).not.toBeNull();
+    const alias = branch.expressionAlias!;
+
+    const visitor = new t.RecursiveVisitor();
+
+    // Visit the branch
+    visitor.visitIfBlockBranch(branch);
+
+    // Verify post-visit state
+    // The children array should STILL only contain the Element.
+    // It should NOT contain the alias variable.
+    expect(branch.children.length).toBe(1);
+    expect(branch.children[0]).toBe(element);
+    expect(branch.children).not.toContain(alias);
+  });
+});

Original file line number	Diff line number	Diff line change
`@@ -803,9 +803,8 @@ export class RecursiveVisitor implements Visitor<void> {`
`803`	`803`	`visitAll(this, block.branches);`
`804`	`804`	`}`
`805`	`805`	`visitIfBlockBranch(block: IfBlockBranch): void {`
`806`		`- const blockItems = block.children;`
`807`		`- block.expressionAlias && blockItems.push(block.expressionAlias);`
`808`		`- visitAll(this, blockItems);`
	`806`	`+ visitAll(this, block.children);`
	`807`	`+ block.expressionAlias?.visit(this);`
`809`	`808`	`}`
`810`	`809`	`visitContent(content: Content): void {`
`811`	`810`	`visitAll(this, content.children);`