fix: listener handler scope should not merge parent local_definitions in resolve_names

In Angular's resolve_names.ts, listener handler ops are processed as a
completely separate lexical scope via recursive processLexicalScope(unit,
op.handlerOps, savedView) — no merging from the parent view's scope.

OXC incorrectly merged the parent view's update_scope (including
local_definitions) into the handler scope. This caused @let declarations
(which have local=true) from the update scope to take precedence over
the handler's ContextLetReferenceExpr variables. The ContextLetReferenceExpr
became unused, was removed by optimizeVariables, and then optimizeStoreLet
couldn't find external @let usage — incorrectly removing the storeLet
wrapper and causing cumulative pipe varOffset drift.

The fix makes handler ops self-contained (matching Angular), since
generateVariables already prepends all necessary variables to handler_ops.

Fixes 43 ClickUp comparison mismatches (203 → 160, 96.5% → 97.3%).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Author: Brooooooklyn

crates/oxc_angular_compiler/src/pipeline/phases/resolve_names.rs

@@ -98,6 +98,47 @@ impl<'a> ScopeMaps<'a> {
     }
 }
 
+/// Build scope maps from listener handler_ops.
+///
+/// Per Angular's resolve_names.ts (line 86), handler ops are processed as a
+/// completely separate lexical scope via recursive `processLexicalScope(unit, op.handlerOps, savedView)`.
+/// The generateVariables phase already prepends all necessary variables to handler_ops,
+/// so no merging from the parent view's scope is needed. This matches Angular's
+/// behavior where each `processLexicalScope` call starts with fresh scope/localDefinitions maps.
+fn build_scope_from_handler_ops<'a, 'b>(
+    ops: impl Iterator<Item = &'b UpdateOp<'a>>,
+) -> ScopeMaps<'a>
+where
+    'a: 'b,
+{
+    let mut maps = ScopeMaps::default();
+    for op in ops {
+        if let UpdateOp::Variable(var_op) = op {
+            match var_op.kind {
+                SemanticVariableKind::Identifier => {
+                    if var_op.local {
+                        if !maps.local_definitions.contains_key(&var_op.name) {
+                            maps.local_definitions.insert(var_op.name.clone(), var_op.xref);
+                        }
+                    } else if !maps.scope.contains_key(&var_op.name) {
+                        maps.scope.insert(var_op.name.clone(), var_op.xref);
+                    }
+                    if !maps.scope.contains_key(&var_op.name) {
+                        maps.scope.insert(var_op.name.clone(), var_op.xref);
+                    }
+                }
+                SemanticVariableKind::Alias => {
+                    if !maps.scope.contains_key(&var_op.name) {
+                        maps.scope.insert(var_op.name.clone(), var_op.xref);
+                    }
+                }
+                _ => {}
+            }
+        }
+    }
+    maps
+}
+
 /// Build scope maps from update operations (for variables like context reads).
 fn build_scope_from_update_ops<'a>(ops: &crate::ir::list::UpdateOpList<'a>) -> ScopeMaps<'a> {
     let mut maps = ScopeMaps::default();
@@ -193,58 +234,13 @@ fn process_lexical_scope_create<'a>(
     for op in ops.iter_mut() {
         match op {
             CreateOp::Listener(listener) => {
-                // Build handler-specific scope that includes Variables from handler_ops.
-                // This is critical for @for loops where listeners need access to loop variables
-                // that were prepended to handler_ops by generate_variables phase.
-                //
-                // We use "first wins" semantics: the first Variable with a given name takes
-                // precedence. This matches Angular TypeScript compiler behavior in
-                // resolve_names.ts (lines 55-58).
-                //
-                // handler_ops Variables override update_scope Variables, but among handler_ops
-                // Variables themselves, the first one wins. This is achieved by:
-                // 1. First building a scope from handler_ops only (first wins)
-                // 2. Then merging in update_scope for names not in handler_ops
-                let mut handler_scope = ScopeMaps::default();
-                // First pass: add handler_ops Variables (first wins)
-                for handler_op in listener.handler_ops.iter() {
-                    if let UpdateOp::Variable(var_op) = handler_op {
-                        match var_op.kind {
-                            SemanticVariableKind::Identifier => {
-                                // Handler-local variables take precedence
-                                if var_op.local
-                                    && !handler_scope.local_definitions.contains_key(&var_op.name)
-                                {
-                                    handler_scope
-                                        .local_definitions
-                                        .insert(var_op.name.clone(), var_op.xref);
-                                }
-                                // Only add if not already present (first wins)
-                                if !handler_scope.scope.contains_key(&var_op.name) {
-                                    handler_scope.scope.insert(var_op.name.clone(), var_op.xref);
-                                }
-                            }
-                            SemanticVariableKind::Alias => {
-                                // Only add if not already present (first wins)
-                                if !handler_scope.scope.contains_key(&var_op.name) {
-                                    handler_scope.scope.insert(var_op.name.clone(), var_op.xref);
-                                }
-                            }
-                            _ => {}
-                        }
-                    }
-                }
-                // Second pass: merge in update_scope for names not in handler_ops
-                for (name, xref) in &scope.scope {
-                    if !handler_scope.scope.contains_key(name) {
-                        handler_scope.scope.insert(name.clone(), *xref);
-                    }
-                }
-                for (name, xref) in &scope.local_definitions {
-                    if !handler_scope.local_definitions.contains_key(name) {
-                        handler_scope.local_definitions.insert(name.clone(), *xref);
-                    }
-                }
+                // Per Angular's resolve_names.ts (line 86):
+                //   processLexicalScope(unit, op.handlerOps, savedView);
+                // Handler ops are processed as a SEPARATE lexical scope — a fresh
+                // scope/localDefinitions is built from handler_ops variables only,
+                // with NO merging from the parent view's scope. The generateVariables
+                // phase already prepends all necessary variables to handler_ops.
+                let handler_scope = build_scope_from_handler_ops(listener.handler_ops.iter());
 
                 // Process listener handler_ops with the handler scope
                 for handler_op in listener.handler_ops.iter_mut() {
@@ -276,43 +272,8 @@ fn process_lexical_scope_create<'a>(
                 }
             }
             CreateOp::TwoWayListener(listener) => {
-                // Build handler-specific scope for TwoWayListener
-                // Uses same "first wins" semantics as Listener (see above)
-                let mut handler_scope = ScopeMaps::default();
-                for handler_op in listener.handler_ops.iter() {
-                    if let UpdateOp::Variable(var_op) = handler_op {
-                        match var_op.kind {
-                            SemanticVariableKind::Identifier => {
-                                if var_op.local
-                                    && !handler_scope.local_definitions.contains_key(&var_op.name)
-                                {
-                                    handler_scope
-                                        .local_definitions
-                                        .insert(var_op.name.clone(), var_op.xref);
-                                }
-                                if !handler_scope.scope.contains_key(&var_op.name) {
-                                    handler_scope.scope.insert(var_op.name.clone(), var_op.xref);
-                                }
-                            }
-                            SemanticVariableKind::Alias => {
-                                if !handler_scope.scope.contains_key(&var_op.name) {
-                                    handler_scope.scope.insert(var_op.name.clone(), var_op.xref);
-                                }
-                            }
-                            _ => {}
-                        }
-                    }
-                }
-                for (name, xref) in &scope.scope {
-                    if !handler_scope.scope.contains_key(name) {
-                        handler_scope.scope.insert(name.clone(), *xref);
-                    }
-                }
-                for (name, xref) in &scope.local_definitions {
-                    if !handler_scope.local_definitions.contains_key(name) {
-                        handler_scope.local_definitions.insert(name.clone(), *xref);
-                    }
-                }
+                // Per Angular's resolve_names.ts: handler ops get their own scope
+                let handler_scope = build_scope_from_handler_ops(listener.handler_ops.iter());
 
                 for handler_op in listener.handler_ops.iter_mut() {
                     transform_expressions_in_update_op(
@@ -330,46 +291,10 @@ fn process_lexical_scope_create<'a>(
                         VisitorContextFlag::NONE,
                     );
                 }
-                // Note: TwoWayListenerOp has no handler_expression field
             }
             CreateOp::AnimationListener(listener) => {
-                // Build handler-specific scope for AnimationListener
-                // Uses same "first wins" semantics as Listener (see above)
-                let mut handler_scope = ScopeMaps::default();
-                for handler_op in listener.handler_ops.iter() {
-                    if let UpdateOp::Variable(var_op) = handler_op {
-                        match var_op.kind {
-                            SemanticVariableKind::Identifier => {
-                                if var_op.local
-                                    && !handler_scope.local_definitions.contains_key(&var_op.name)
-                                {
-                                    handler_scope
-                                        .local_definitions
-                                        .insert(var_op.name.clone(), var_op.xref);
-                                }
-                                if !handler_scope.scope.contains_key(&var_op.name) {
-                                    handler_scope.scope.insert(var_op.name.clone(), var_op.xref);
-                                }
-                            }
-                            SemanticVariableKind::Alias => {
-                                if !handler_scope.scope.contains_key(&var_op.name) {
-                                    handler_scope.scope.insert(var_op.name.clone(), var_op.xref);
-                                }
-                            }
-                            _ => {}
-                        }
-                    }
-                }
-                for (name, xref) in &scope.scope {
-                    if !handler_scope.scope.contains_key(name) {
-                        handler_scope.scope.insert(name.clone(), *xref);
-                    }
-                }
-                for (name, xref) in &scope.local_definitions {
-                    if !handler_scope.local_definitions.contains_key(name) {
-                        handler_scope.local_definitions.insert(name.clone(), *xref);
-                    }
-                }
+                // Per Angular's resolve_names.ts: handler ops get their own scope
+                let handler_scope = build_scope_from_handler_ops(listener.handler_ops.iter());
 
                 for handler_op in listener.handler_ops.iter_mut() {
                     transform_expressions_in_update_op(
@@ -387,46 +312,10 @@ fn process_lexical_scope_create<'a>(
                         VisitorContextFlag::NONE,
                     );
                 }
-                // Note: AnimationListenerOp has no handler_expression field
             }
             CreateOp::Animation(animation) => {
-                // Build handler-specific scope for Animation
-                // Uses same "first wins" semantics as Listener (see above)
-                let mut handler_scope = ScopeMaps::default();
-                for handler_op in animation.handler_ops.iter() {
-                    if let UpdateOp::Variable(var_op) = handler_op {
-                        match var_op.kind {
-                            SemanticVariableKind::Identifier => {
-                                if var_op.local
-                                    && !handler_scope.local_definitions.contains_key(&var_op.name)
-                                {
-                                    handler_scope
-                                        .local_definitions
-                                        .insert(var_op.name.clone(), var_op.xref);
-                                }
-                                if !handler_scope.scope.contains_key(&var_op.name) {
-                                    handler_scope.scope.insert(var_op.name.clone(), var_op.xref);
-                                }
-                            }
-                            SemanticVariableKind::Alias => {
-                                if !handler_scope.scope.contains_key(&var_op.name) {
-                                    handler_scope.scope.insert(var_op.name.clone(), var_op.xref);
-                                }
-                            }
-                            _ => {}
-                        }
-                    }
-                }
-                for (name, xref) in &scope.scope {
-                    if !handler_scope.scope.contains_key(name) {
-                        handler_scope.scope.insert(name.clone(), *xref);
-                    }
-                }
-                for (name, xref) in &scope.local_definitions {
-                    if !handler_scope.local_definitions.contains_key(name) {
-                        handler_scope.local_definitions.insert(name.clone(), *xref);
-                    }
-                }
+                // Per Angular's resolve_names.ts: handler ops get their own scope
+                let handler_scope = build_scope_from_handler_ops(animation.handler_ops.iter());
 
                 for handler_op in animation.handler_ops.iter_mut() {
                     transform_expressions_in_update_op(
@@ -444,7 +333,6 @@ fn process_lexical_scope_create<'a>(
                         VisitorContextFlag::NONE,
                     );
                 }
-                // Note: AnimationOp has no handler_expression field
             }
             _ => {
                 transform_expressions_in_create_op(

crates/oxc_angular_compiler/tests/integration_test.rs

@@ -576,6 +576,88 @@ fn test_let_inside_for_if_with_component_method_call() {
     insta::assert_snapshot!("let_inside_for_if_with_component_method_call", js);
 }
 
+// ============================================================================
+// @let with pipe in child view Tests
+// ============================================================================
+
+#[test]
+fn test_let_with_pipe_used_in_child_view() {
+    // @let with pipe used in a child view (@if block) should keep BOTH declareLet and storeLet.
+    //
+    // When a @let wraps a pipe and is referenced from a child view:
+    // - declareLet is needed because pipes use DI which requires the TNode
+    // - storeLet is needed because the @let is accessed from another view via readContextLet
+    //
+    // Without storeLet, the pipe's varOffset would be wrong because storeLet contributes
+    // 1 var to the var counting, and removing it shifts all subsequent varOffsets.
+    //
+    // Expected Angular output:
+    //   i0.ɵɵstoreLet(i0.ɵɵpipeBind1(1, varOffset, ctx.name));
+    //
+    // Bug output (missing storeLet):
+    //   i0.ɵɵpipeBind1(1, varOffset, ctx.name);
+    let js = compile_template_to_js(
+        r"@let value = name | uppercase; @if (true) { {{value}} }",
+        "TestComponent",
+    );
+    // storeLet must wrap pipeBind because @let is used externally (in child @if view)
+    assert!(
+        js.contains("ɵɵstoreLet(i0.ɵɵpipeBind1("),
+        "storeLet should wrap pipeBind1 when @let with pipe is used in child view. Output:\n{js}"
+    );
+    // declareLet must be present (pipes need TNode for DI)
+    assert!(
+        js.contains("ɵɵdeclareLet("),
+        "declareLet should be present when @let contains a pipe. Output:\n{js}"
+    );
+    // readContextLet must be present in the child view
+    assert!(
+        js.contains("ɵɵreadContextLet("),
+        "readContextLet should be present in child view. Output:\n{js}"
+    );
+    insta::assert_snapshot!("let_with_pipe_used_in_child_view", js);
+}
+
+#[test]
+fn test_let_with_pipe_used_in_listener() {
+    // @let with pipe used in an event listener in the same view should keep storeLet.
+    //
+    // Event listeners are callbacks (isCallback=true), so @let declarations
+    // in the same view generate ContextLetReferenceExpr in the listener's handler ops.
+    // This means the @let is "used externally" and storeLet must be preserved.
+    let js = compile_template_to_js(
+        r#"@let value = name | uppercase; <button (click)="onClick(value)">Click</button>"#,
+        "TestComponent",
+    );
+    // storeLet must wrap pipeBind because @let is used externally (in listener callback)
+    assert!(
+        js.contains("ɵɵstoreLet(i0.ɵɵpipeBind1("),
+        "storeLet should wrap pipeBind1 when @let with pipe is used in listener. Output:\n{js}"
+    );
+    insta::assert_snapshot!("let_with_pipe_used_in_listener", js);
+}
+
+#[test]
+fn test_let_with_pipe_multiple_in_child_view_varoffset() {
+    // Multiple @let declarations with pipes used in a child view.
+    // Each storeLet contributes 1 var, so removing them would cause cumulative varOffset drift.
+    //
+    // This reproduces the ClickUp AdvancedTabComponent pattern where multiple @let
+    // declarations with pipes have their storeLet wrappers incorrectly removed,
+    // causing the second pipe's varOffset to drift by +1 for each missing storeLet.
+    let js = compile_template_to_js(
+        r"@let a = x | uppercase; @let b = y | lowercase; @if (true) { {{a}} {{b}} }",
+        "TestComponent",
+    );
+    // Both @let values should have storeLet wrappers
+    let store_let_count = js.matches("ɵɵstoreLet(").count();
+    assert!(
+        store_let_count >= 2,
+        "Expected at least 2 storeLet calls for 2 @let declarations with pipes used in child view, got {store_let_count}. Output:\n{js}"
+    );
+    insta::assert_snapshot!("let_with_pipe_multiple_in_child_view_varoffset", js);
+}
+
 // ============================================================================
 // ng-content Tests
 // ============================================================================

crates/oxc_angular_compiler/tests/snapshots/integration_test__let_with_pipe_multiple_in_child_view_varoffset.snap

@@ -0,0 +1,31 @@
+---
+source: crates/oxc_angular_compiler/tests/integration_test.rs
+expression: js
+---
+function TestComponent_Conditional_6_Template(rf,ctx) {
+  if ((rf & 1)) { i0.ɵɵtext(0); }
+  if ((rf & 2)) {
+    i0.ɵɵnextContext();
+    const a_r1 = i0.ɵɵreadContextLet(0);
+    const b_r2 = i0.ɵɵreadContextLet(3);
+    i0.ɵɵtextInterpolate2(" ",a_r1," ",b_r2," ");
+  }
+}
+function TestComponent_Template(rf,ctx) {
+  if ((rf & 1)) {
+    i0.ɵɵdeclareLet(0);
+    i0.ɵɵpipe(1,"uppercase");
+    i0.ɵɵtext(2," ");
+    i0.ɵɵdeclareLet(3);
+    i0.ɵɵpipe(4,"lowercase");
+    i0.ɵɵtext(5," ");
+    i0.ɵɵconditionalCreate(6,TestComponent_Conditional_6_Template,1,2);
+  }
+  if ((rf & 2)) {
+    i0.ɵɵstoreLet(i0.ɵɵpipeBind1(1,1,ctx.x));
+    i0.ɵɵadvance(3);
+    i0.ɵɵstoreLet(i0.ɵɵpipeBind1(4,4,ctx.y));
+    i0.ɵɵadvance(3);
+    i0.ɵɵconditional((true? 6: -1));
+  }
+}