Skip to content

Commit 207654e

Browse files
Brooooooklynclaude
andauthored
fix(angular): fix 7 compiler divergences from Angular reference (#26)
fix(angular): fix 6 compiler divergences from Angular reference 1. Add missing @switch validation: report errors for non-block children, enforce exactly one @case parameter, single @default, and no @default parameters 2. Carry viewport trigger options through ingest into DeferOnOp IR 3. Change generateArrowFunctions phase to Kind::Both with host support 4. Add view.functions traversal to next_context_merging phase 5. Emit null arg for unresolved target slots in defer reify (viewport, interaction, hover triggers) 6. Add debug_assert for unexpected multi-line arrow functions Item 5 from the review (@switch @default reorder) was verified as a false positive: Angular does reorder @default last, just not in ingestSwitchBlock — the generateConditionalExpressions phase splices @default out as the ternary fallback. The existing Rust reorder is correct; updated the doc comment to explain why. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 5a69c48 commit 207654e

File tree

10 files changed

+258
-46
lines changed

10 files changed

+258
-46
lines changed

crates/oxc_angular_compiler/src/pipeline/ingest.rs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2884,9 +2884,11 @@ fn create_binary_modulo<'a>(
28842884
/// Creates one CREATE op per case (ConditionalOp for first, ConditionalBranchCreateOp for rest)
28852885
/// and one UPDATE op (ConditionalUpdateOp) containing all conditions.
28862886
///
2887-
/// IMPORTANT: Angular always processes @default LAST, regardless of where it appears in
2888-
/// the template. This affects slot allocation and function naming. To match Angular's behavior,
2889-
/// we reorder the groups to put the @default group at the end before processing.
2887+
/// Angular's `ingestSwitchBlock` in `ingest.ts` iterates groups in source order, but the
2888+
/// `generateConditionalExpressions` phase later splices `@default` out and uses it as the
2889+
/// ternary fallback. Because the Rust pipeline's conditional codegen expects `@default` last,
2890+
/// we reorder here so that slot allocation, function naming, and the conditional expression
2891+
/// all match Angular's compiled output.
28902892
///
28912893
/// Ported from Angular's `ingestSwitchBlock` in `ingest.ts`.
28922894
fn ingest_switch_block<'a>(
@@ -2904,15 +2906,17 @@ fn ingest_switch_block<'a>(
29042906
// Convert the main switch expression as the test
29052907
let test = convert_ast_to_ir(job, switch_block.expression);
29062908

2907-
// Reorder groups to put @default LAST, matching Angular's behavior.
2908-
// Angular always assigns higher slot numbers to @default regardless of template order.
2909-
// A group is considered the @default group if ALL its cases have expression: None.
2909+
// Reorder groups to put @default LAST, matching Angular's compiled output.
2910+
// While Angular's ingestSwitchBlock iterates in source order, the downstream
2911+
// generateConditionalExpressions phase (conditionals.ts) splices @default out and
2912+
// uses it as the ternary fallback base. Because slot allocation and function naming
2913+
// happen after ingest, moving @default last here ensures our xref/slot/function
2914+
// ordering matches Angular's final output.
29102915
let mut groups_vec: std::vec::Vec<_> = switch_block.groups.into_iter().collect();
29112916
let default_idx = groups_vec.iter().position(|group| {
29122917
!group.cases.is_empty() && group.cases.iter().all(|c| c.expression.is_none())
29132918
});
29142919
if let Some(idx) = default_idx {
2915-
// Move the default group to the end
29162920
let default_group = groups_vec.remove(idx);
29172921
groups_vec.push(default_group);
29182922
}
@@ -3432,6 +3436,7 @@ fn ingest_defer_triggers<'a>(
34323436

34333437
// Handle viewport trigger
34343438
if let Some(viewport_trigger) = triggers.viewport {
3439+
let options = viewport_trigger.options.map(|opts| convert_ast_to_ir(job, opts));
34353440
let op = CreateOp::DeferOn(DeferOnOp {
34363441
base: CreateOpBase {
34373442
source_span: Some(viewport_trigger.source_span),
@@ -3446,7 +3451,7 @@ fn ingest_defer_triggers<'a>(
34463451
target_slot_view_steps: None,
34473452
target_name: viewport_trigger.reference,
34483453
delay: None,
3449-
options: None,
3454+
options,
34503455
});
34513456
if let Some(view) = job.view_mut(view_xref) {
34523457
view.create.push(op);

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

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use crate::ir::ops::{CreateOp, Op};
1515
use crate::output::ast::{
1616
ArrowFunctionBody, ArrowFunctionExpr as OutputArrowFunctionExpr, FnParam,
1717
};
18-
use crate::pipeline::compilation::ComponentCompilationJob;
18+
use crate::pipeline::compilation::{ComponentCompilationJob, HostBindingCompilationJob};
1919
use oxc_allocator::{Box as AllocBox, Vec as AllocVec};
2020

2121
/// Finds arrow functions written by the user and converts them into
@@ -123,7 +123,9 @@ fn convert_output_arrow_to_ir<'a>(
123123
// The expression syntax doesn't support multi-line arrow functions,
124124
// but the output AST does. We don't need to handle them here if
125125
// the user isn't able to write one.
126-
// In TypeScript this throws an error, but we'll just skip it.
126+
// Angular throws an assertion error here; we use debug_assert to
127+
// catch any internal compiler bugs that produce this in debug builds.
128+
debug_assert!(false, "unexpected multi-line arrow function in template expression");
127129
return None;
128130
}
129131
};
@@ -143,6 +145,39 @@ fn convert_output_arrow_to_ir<'a>(
143145
})
144146
}
145147

148+
/// Generate arrow functions for host binding compilation.
149+
///
150+
/// Angular runs this phase for Kind.Both, meaning it applies to both
151+
/// template and host compilations. Host bindings can contain arrow
152+
/// function expressions (e.g., in @HostBinding values or event handlers).
153+
pub fn generate_arrow_functions_for_host(job: &mut HostBindingCompilationJob<'_>) {
154+
let allocator = job.allocator;
155+
156+
// Process create operations (skip listeners)
157+
for op in job.root.create.iter_mut() {
158+
if !is_listener_op(op) {
159+
transform_expressions_in_create_op(
160+
op,
161+
&|expr, flags| {
162+
add_arrow_function(expr, allocator, flags);
163+
},
164+
VisitorContextFlag::NONE,
165+
);
166+
}
167+
}
168+
169+
// Process update operations
170+
for op in job.root.update.iter_mut() {
171+
transform_expressions_in_update_op(
172+
op,
173+
&|expr, flags| {
174+
add_arrow_function(expr, allocator, flags);
175+
},
176+
VisitorContextFlag::NONE,
177+
);
178+
}
179+
}
180+
146181
/// Collect arrow functions from a view's operations into its functions set.
147182
fn collect_arrow_functions_from_view(
148183
view: &mut crate::pipeline::compilation::ViewCompilationUnit<'_>,

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -252,11 +252,11 @@ pub static PHASES: &[Phase] = &[
252252
run_host: None,
253253
name: "createVariadicPipes",
254254
},
255-
// Phase 21: generateArrowFunctions (Template only)
255+
// Phase 21: generateArrowFunctions (Both)
256256
Phase {
257-
kind: CompilationJobKind::Template,
257+
kind: CompilationJobKind::Both,
258258
run: generate_arrow_functions::generate_arrow_functions,
259-
run_host: None,
259+
run_host: Some(generate_arrow_functions::generate_arrow_functions_for_host),
260260
name: "generateArrowFunctions",
261261
},
262262
// Phase 22: generatePureLiteralStructures (Both)

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,13 @@ pub fn merge_next_context_expressions(job: &mut ComponentCompilationJob<'_>) {
3030

3131
for xref in view_xrefs {
3232
if let Some(view) = job.view_mut(xref) {
33+
// Merge in arrow function op lists (matches Angular's unit.functions traversal)
34+
for fn_ptr in view.functions.iter() {
35+
// SAFETY: These pointers are valid for the duration of the compilation
36+
let arrow_fn = unsafe { &mut **fn_ptr };
37+
merge_next_contexts_in_handler_ops(&mut arrow_fn.ops);
38+
}
39+
3340
// Merge in create ops for listeners
3441
for op in view.create.iter_mut() {
3542
match op {

crates/oxc_angular_compiler/src/pipeline/phases/reify/statements/defer.rs

Lines changed: 32 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -225,12 +225,19 @@ pub fn create_defer_on_stmt<'a>(
225225
if let Some(opts) = options {
226226
args.push(opts);
227227
}
228-
} else if let Some(slot) = target_slot {
229-
// Regular/Prefetch viewport with explicit target: target_slot, target_slot_view_steps?, options?
230-
args.push(OutputExpression::Literal(Box::new_in(
231-
LiteralExpr { value: LiteralValue::Number(slot as f64), source_span: None },
232-
allocator,
233-
)));
228+
} else {
229+
// Always emit the first arg: slot number or null if unresolved.
230+
// Angular: o.literal(op.trigger.targetSlot?.slot ?? null)
231+
args.push(match target_slot {
232+
Some(slot) => OutputExpression::Literal(Box::new_in(
233+
LiteralExpr { value: LiteralValue::Number(slot as f64), source_span: None },
234+
allocator,
235+
)),
236+
None => OutputExpression::Literal(Box::new_in(
237+
LiteralExpr { value: LiteralValue::Null, source_span: None },
238+
allocator,
239+
)),
240+
});
234241

235242
let view_steps = target_slot_view_steps.unwrap_or(0);
236243
if view_steps != 0 {
@@ -253,28 +260,32 @@ pub fn create_defer_on_stmt<'a>(
253260
args.push(opts);
254261
}
255262
}
256-
// No arguments when no explicit target is specified
257263
}
258264
DeferTriggerKind::Interaction | DeferTriggerKind::Hover => {
259265
// Hydrate triggers don't support targets
260266
if modifier != DeferOpModifierKind::Hydrate {
261-
// Only push arguments if there's an explicit target
262-
if let Some(slot) = target_slot {
263-
args.push(OutputExpression::Literal(Box::new_in(
267+
// Always emit the first arg: slot number or null if unresolved.
268+
// Angular: o.literal(op.trigger.targetSlot?.slot ?? null)
269+
args.push(match target_slot {
270+
Some(slot) => OutputExpression::Literal(Box::new_in(
264271
LiteralExpr { value: LiteralValue::Number(slot as f64), source_span: None },
265272
allocator,
266-
)));
273+
)),
274+
None => OutputExpression::Literal(Box::new_in(
275+
LiteralExpr { value: LiteralValue::Null, source_span: None },
276+
allocator,
277+
)),
278+
});
267279

268-
let view_steps = target_slot_view_steps.unwrap_or(0);
269-
if view_steps != 0 {
270-
args.push(OutputExpression::Literal(Box::new_in(
271-
LiteralExpr {
272-
value: LiteralValue::Number(view_steps as f64),
273-
source_span: None,
274-
},
275-
allocator,
276-
)));
277-
}
280+
let view_steps = target_slot_view_steps.unwrap_or(0);
281+
if view_steps != 0 {
282+
args.push(OutputExpression::Literal(Box::new_in(
283+
LiteralExpr {
284+
value: LiteralValue::Number(view_steps as f64),
285+
source_span: None,
286+
},
287+
allocator,
288+
)));
278289
}
279290
}
280291
}

crates/oxc_angular_compiler/src/transform/html_to_r3.rs

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2313,21 +2313,36 @@ impl<'a> HtmlToR3Transform<'a> {
23132313
let mut unknown_blocks = Vec::new_in(self.allocator);
23142314
let mut collected_cases: std::vec::Vec<R3SwitchBlockCase<'a>> = std::vec::Vec::new();
23152315
let mut first_case_start: Option<Span> = None;
2316+
let mut has_default = false;
23162317

23172318
for child in &block.children {
2318-
// Skip non-block nodes (only process block children)
2319+
// Skip comments and whitespace-only text nodes (same as Angular)
2320+
match child {
2321+
HtmlNode::Comment(_) => continue,
2322+
HtmlNode::Text(t) if t.value.trim().is_empty() => continue,
2323+
_ => {}
2324+
}
2325+
2326+
// Non-block children (elements, non-whitespace text, etc.) are invalid
23192327
let child_block = match child {
23202328
HtmlNode::Block(b) => b,
2321-
_ => continue,
2329+
_ => {
2330+
self.report_error(
2331+
"@switch block can only contain @case and @default blocks",
2332+
child.span(),
2333+
);
2334+
continue;
2335+
}
23222336
};
23232337

2324-
// Check if this is a valid case/default block
2325-
// Note: @case with no parameters is treated as unknown (same as Angular)
2326-
let is_case =
2327-
child_block.block_type == BlockType::Case && !child_block.parameters.is_empty();
2328-
let is_default = child_block.block_type == BlockType::Default;
2329-
2330-
if !is_case && !is_default {
2338+
// Validate: only @case and @default are allowed inside @switch
2339+
if child_block.block_type != BlockType::Case
2340+
&& child_block.block_type != BlockType::Default
2341+
{
2342+
self.report_error(
2343+
"@switch block can only contain @case and @default blocks",
2344+
child_block.span,
2345+
);
23312346
unknown_blocks.push(crate::ast::r3::R3UnknownBlock {
23322347
name: child_block.name.clone(),
23332348
source_span: child_block.span,
@@ -2336,6 +2351,34 @@ impl<'a> HtmlToR3Transform<'a> {
23362351
continue;
23372352
}
23382353

2354+
let is_default = child_block.block_type == BlockType::Default;
2355+
2356+
// Validate @default
2357+
if is_default {
2358+
if has_default {
2359+
self.report_error(
2360+
"@switch block can only have one @default block",
2361+
child_block.start_span,
2362+
);
2363+
} else if !child_block.parameters.is_empty() {
2364+
self.report_error(
2365+
"@default block cannot have parameters",
2366+
child_block.start_span,
2367+
);
2368+
}
2369+
has_default = true;
2370+
}
2371+
2372+
// Validate @case: must have exactly one parameter
2373+
let is_case = child_block.block_type == BlockType::Case;
2374+
if is_case && child_block.parameters.len() != 1 {
2375+
self.report_error(
2376+
"@case block must have exactly one parameter",
2377+
child_block.start_span,
2378+
);
2379+
continue;
2380+
}
2381+
23392382
// Parse expression for @case blocks
23402383
let case_expression = if is_case {
23412384
let expr_str = child_block.parameters[0].expression.as_str();

crates/oxc_angular_compiler/tests/integration_test.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,9 @@ fn test_switch_block() {
435435

436436
#[test]
437437
fn test_switch_block_default_first() {
438-
// Test @switch with @default appearing first - Angular should reorder to put @default last
438+
// Test @switch with @default appearing first - Angular reorders @default last
439+
// Angular's ingestSwitchBlock iterates in source order, but generateConditionalExpressions
440+
// splices @default out as the ternary fallback. We reorder at ingest to match the final output.
439441
let js = compile_template_to_js(
440442
r"@switch (value) { @default { <div>Other</div> } @case (1) { <div>One</div> } @case (2) { <div>Two</div> } }",
441443
"TestComponent",

0 commit comments

Comments
 (0)