Skip to content

Commit 2759cd4

Browse files
edbznclaudeBrooooooklyn
authored
fix(compiler): resolve template locals inside $safeNavigationMigration in listeners and arrow functions (#587)
* fix(compiler): resolve template locals inside `$safeNavigationMigration` in listeners A listener's `handler_expression` is resolved by calling `resolve_expression` directly rather than through the generic expression visitor, and its explicit match had no arm for the `SafeNavigationMigration` wrapper. Template locals in the wrapped subtree (`@for` items, `@if ... as` aliases) were left unresolved and later read from the listener context (`ctx.option`), so `(click)="pick($safeNavigationMigration(option?.value))"` always passed `null`. Fixes #585 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test(compiler): cover arrow functions and more template locals for `$safeNavigationMigration` - resolve_names: the wrapper arm comment now names both direct roots (a listener's handler_expression and hoisted arrow-function bodies). - Listener test: add ng-template `let-`, root-view `#ref` and `@let` cases; assert the exact declaration and read for each local. - New test for arrow functions: the hoisted factory restores the local from its own view and returns through `ɵɵresetView`. - Move the compare fixture into safe-navigation-migration.fixture.ts and add more listener locals plus an arrow-function entry; both match Angular 22.2.2. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(compiler): assert whole listener bodies in the `$safeNavigationMigration` test Assert every statement of each listener body as one block, as the arrow-function test does. The old checks still passed when the root-view `#ref`/`@let` listeners had no `ɵɵrestoreView`/`ɵɵresetView`, which is the pre-fix shape. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: LongYinan <lynweklm@gmail.com>
1 parent 3a4f8c3 commit 2759cd4

3 files changed

Lines changed: 210 additions & 0 deletions

File tree

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -877,6 +877,20 @@ fn resolve_expression<'a>(
877877
);
878878
}
879879

880+
// `$safeNavigationMigration(...)` wrapper - resolve the wrapped expression. A listener's
881+
// `handler_expression` and hoisted arrow-function bodies reach this function directly,
882+
// not through the generic visitor, so the wrapper must be descended here.
883+
IrExpression::SafeNavigationMigration(migration) => {
884+
resolve_expression(
885+
migration.expr.as_mut(),
886+
scope,
887+
root_xref,
888+
saved_view,
889+
allocator,
890+
expressions,
891+
);
892+
}
893+
880894
// ResolvedTemplateLiteral (created by ingest for template literals with inner expressions)
881895
// - resolve each inner expression so LexicalRead refs to @let vars and pipe args are resolved
882896
IrExpression::ResolvedTemplateLiteral(tl) => {

‎crates/oxc_angular_compiler/tests/integration_test.rs‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2339,6 +2339,123 @@ fn test_safe_navigation_migration_ignores_qualified_call() {
23392339
);
23402340
}
23412341

2342+
#[test]
2343+
fn test_safe_navigation_migration_in_listener_reads_template_locals() {
2344+
// Inside a listener, the wrapped expression must resolve template locals
2345+
// (`@for` items, `@if ... as` aliases, `let-` variables, `#ref`s, `@let`s) to
2346+
// the restored-view variables, like any other listener expression, instead of
2347+
// falling back to `ctx.<name>`.
2348+
// (template, statements of the listener body, old context read)
2349+
let cases = [
2350+
(
2351+
r#"@for (option of options; track $index) {
2352+
<button (click)="pick($safeNavigationMigration(option?.value))"></button>
2353+
}"#,
2354+
[
2355+
"const option_r2 = i0.ɵɵrestoreView(_r1).$implicit;",
2356+
"const ctx_r2 = i0.ɵɵnextContext();",
2357+
"return i0.ɵɵresetView(ctx_r2.pick(((option_r2 == null)? null: option_r2.value)));",
2358+
],
2359+
"ctx.option.value",
2360+
),
2361+
(
2362+
r#"@if (user; as u) {
2363+
<button (click)="save($safeNavigationMigration(u?.id))"></button>
2364+
}"#,
2365+
[
2366+
"const u_r2 = i0.ɵɵrestoreView(_r1);",
2367+
"const ctx_r2 = i0.ɵɵnextContext();",
2368+
"return i0.ɵɵresetView(ctx_r2.save(((u_r2 == null)? null: u_r2.id)));",
2369+
],
2370+
"ctx.u.id",
2371+
),
2372+
(
2373+
r#"<ng-template let-item>
2374+
<button (click)="pick($safeNavigationMigration(item?.value))"></button>
2375+
</ng-template>"#,
2376+
[
2377+
"const item_r2 = i0.ɵɵrestoreView(_r1).$implicit;",
2378+
"const ctx_r2 = i0.ɵɵnextContext();",
2379+
"return i0.ɵɵresetView(ctx_r2.pick(((item_r2 == null)? null: item_r2.value)));",
2380+
],
2381+
"ctx.item.value",
2382+
),
2383+
(
2384+
r#"<input #box />
2385+
<button (click)="pick($safeNavigationMigration(box?.value))"></button>"#,
2386+
[
2387+
"i0.ɵɵrestoreView(_r1);",
2388+
"const box_r2 = i0.ɵɵreference(1);",
2389+
"return i0.ɵɵresetView(ctx.pick(((box_r2 == null)? null: box_r2.value)));",
2390+
],
2391+
"ctx.box.value",
2392+
),
2393+
(
2394+
r#"@let v = user?.profile;
2395+
<button (click)="pick($safeNavigationMigration(v?.name))"></button>"#,
2396+
[
2397+
"i0.ɵɵrestoreView(_r1);",
2398+
"const v_r2 = i0.ɵɵreadContextLet(0);",
2399+
"return i0.ɵɵresetView(ctx.pick(((v_r2 == null)? null: v_r2.name)));",
2400+
],
2401+
"ctx.v.name",
2402+
),
2403+
];
2404+
for (template, statements, old_read) in cases {
2405+
let js = compile_template_to_js_with_version(
2406+
template,
2407+
"TestComponent",
2408+
Some(AngularVersion::new(22, 0, 0)),
2409+
);
2410+
let body = format!("_listener() {{\n {}\n }});", statements.join("\n "));
2411+
assert!(js.contains(&body), "expected the listener body\n{body}\ngot:\n{js}");
2412+
assert!(!js.contains(old_read), "must not read `{old_read}`, got:\n{js}");
2413+
assert!(
2414+
!js.contains("$safeNavigationMigration"),
2415+
"the wrapper must be stripped, got:\n{js}"
2416+
);
2417+
}
2418+
}
2419+
2420+
#[test]
2421+
fn test_safe_navigation_migration_in_arrow_function_reads_template_locals() {
2422+
// A template arrow function is hoisted into a module-level factory that gets
2423+
// the view as a parameter. The wrapped expression in its body must restore
2424+
// the local from that view (and return through `ɵɵresetView`) instead of
2425+
// reading a free variable that only exists inside the template function.
2426+
// (template, declaration of the local, return of the factory)
2427+
let cases = [
2428+
(
2429+
r#"@for (option of options; track $index) {
2430+
<div [title]="fmt(() => $safeNavigationMigration(option?.value))"></div>
2431+
}"#,
2432+
"const option_r1 = i0.ɵɵrestoreView(view).$implicit;",
2433+
"return i0.ɵɵresetView(((option_r1 == null)? null: option_r1.value));",
2434+
),
2435+
(
2436+
r#"@if (user; as u) {
2437+
<div [title]="fmt(() => $safeNavigationMigration(u?.id))"></div>
2438+
}"#,
2439+
"const u_r1 = i0.ɵɵrestoreView(view);",
2440+
"return i0.ɵɵresetView(((u_r1 == null)? null: u_r1.id));",
2441+
),
2442+
];
2443+
for (template, declaration, ret) in cases {
2444+
let js = compile_template_to_js_with_version(
2445+
template,
2446+
"TestComponent",
2447+
Some(AngularVersion::new(22, 0, 0)),
2448+
);
2449+
let factory =
2450+
format!("const arrowFn0 = (ctx,view) =>() =>{{\n {declaration}\n {ret}\n}};");
2451+
assert!(js.contains(&factory), "expected the arrow factory\n{factory}\ngot:\n{js}");
2452+
assert!(
2453+
!js.contains("$safeNavigationMigration"),
2454+
"the wrapper must be stripped, got:\n{js}"
2455+
);
2456+
}
2457+
}
2458+
23422459
// ============================================================================
23432460
// Event Modifier Tests
23442461
// ============================================================================

‎napi/angular-compiler/e2e/compare/fixtures/regressions/safe-navigation-migration.fixture.ts‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
/**
2+
* Regression: Template locals inside $safeNavigationMigration.
3+
*
4+
* Issue: Template locals read inside $safeNavigationMigration(...) in
5+
* listeners and arrow functions must resolve to the restored-view
6+
* variables instead of the component context (#585).
7+
*/
8+
import type { Fixture } from '../types.js'
9+
10+
export const fixtures: Fixture[] = [
11+
{
12+
name: 'safe-nav-migration-in-listener',
13+
category: 'regressions',
14+
description: '$safeNavigationMigration in listeners reading template locals',
15+
className: 'SafeNavMigrationInListenerComponent',
16+
type: 'full-transform',
17+
sourceCode: `
18+
import { Component } from '@angular/core';
19+
20+
@Component({
21+
selector: 'app-safe-nav-migration-in-listener',
22+
standalone: true,
23+
template: \`
24+
@for (option of options; track $index) {
25+
<button (click)="pick($safeNavigationMigration(option?.value))">{{ option?.label }}</button>
26+
<button (click)="pick($safeNavigationMigration(selected?.value))">Selected</button>
27+
}
28+
@if (user; as u) {
29+
<button (click)="save($event, $safeNavigationMigration(u?.flags?.verified))">Save</button>
30+
}
31+
<ng-template let-item>
32+
<button (click)="pick($safeNavigationMigration(item?.value))">Item</button>
33+
</ng-template>
34+
<input #box />
35+
<button (click)="pick($safeNavigationMigration(box?.value))">Box</button>
36+
@let firstOption = options[0];
37+
<button (click)="pick($safeNavigationMigration(firstOption?.value))">First</button>
38+
\`,
39+
})
40+
export class SafeNavMigrationInListenerComponent {
41+
options: Array<{ value: string; label: string } | null> = [];
42+
selected: { value: string } | null = null;
43+
user: { flags?: { verified: boolean } } | null = null;
44+
pick(value: string | null) {}
45+
save(event: Event, verified: boolean | null) {}
46+
}
47+
`.trim(),
48+
expectedFeatures: ['ɵɵlistener', 'ɵɵreference', 'ɵɵreadContextLet'],
49+
},
50+
{
51+
name: 'safe-nav-migration-in-arrow-function',
52+
category: 'regressions',
53+
description: '$safeNavigationMigration in arrow functions reading template locals',
54+
className: 'SafeNavMigrationInArrowFunctionComponent',
55+
type: 'full-transform',
56+
sourceCode: `
57+
import { Component } from '@angular/core';
58+
59+
@Component({
60+
selector: 'app-safe-nav-migration-in-arrow-function',
61+
standalone: true,
62+
template: \`
63+
@for (option of options; track $index) {
64+
<div [title]="fmt(() => $safeNavigationMigration(option?.value))">
65+
{{ fmt(() => $safeNavigationMigration(option?.label)) }}
66+
</div>
67+
}
68+
\`,
69+
})
70+
export class SafeNavMigrationInArrowFunctionComponent {
71+
options: Array<{ value: string; label: string } | null> = [];
72+
fmt(fn: () => string | null) {
73+
return fn() ?? '';
74+
}
75+
}
76+
`.trim(),
77+
expectedFeatures: ['ɵɵarrowFunction', 'ɵɵrestoreView', 'ɵɵresetView'],
78+
},
79+
]

0 commit comments

Comments
 (0)