Skip to content

fix(angular): fix 5 compiler divergences from Angular reference #34

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Brooooooklyn merged 1 commit into from
Feb 24, 2026
Merged

fix(angular): fix 5 compiler divergences from Angular reference #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions crates/oxc_angular_compiler/src/pipeline/phases/reify/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -829,16 +829,15 @@ fn reify_update_op<'a>(
Some(create_repeater_stmt(allocator, expr))
}
UpdateOp::Conditional(cond) => {
// Use processed expression (built by conditionals phase) or fall back to test
// Use processed expression (built by conditionals phase).
// Angular asserts that processed is always set by this point
// (throws "Conditional test was not set." in reify.ts:698).
let expr = if let Some(ref processed) = cond.processed {
convert_ir_expression(allocator, processed, expressions, root_xref)
} else if let Some(ref test) = cond.test {
convert_ir_expression(allocator, test, expressions, root_xref)
} else {
OutputExpression::Literal(Box::new_in(
LiteralExpr { value: LiteralValue::Null, source_span: None },
allocator,
))
diagnostics
.push(OxcDiagnostic::error("AssertionError: Conditional test was not set."));
return None;
};
let context_value = cond
.context_value
Expand Down
32 changes: 24 additions & 8 deletions crates/oxc_angular_compiler/src/pipeline/phases/remove_illegal_let_references.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,22 +73,32 @@ pub fn remove_illegal_let_references(job: &mut ComponentCompilationJob<'_>) {
None => continue,
};

// We process by iterating and tracking which let names have been "declared"
// A let name is "declared" when we encounter its Variable op
// We process by iterating and tracking which let names have been "declared".
// A let name is "declared" AFTER we finish transforming its Variable op.
// This matches Angular which walks backward from the declaration op itself,
// replacing self-references (e.g. `@let x = x + 1`) with `undefined`.
let mut declared_names: Vec<Atom<'_>> = Vec::new();

for op in view.update.iter_mut() {
// Check if this is a Variable op that declares a let
if let UpdateOp::Variable(var) = op {
// Check if this op declares a let variable — extract the name before
// transforming so we can mark it as declared AFTER the transform.
let newly_declared = if let UpdateOp::Variable(var) = &*op {
if var.kind == SemanticVariableKind::Identifier {
if let IrExpression::StoreLet(_) = var.initializer.as_ref() {
// Mark this name as declared (after this point, refs are legal)
declared_names.push(var.name.clone());
Some(var.name.clone())
} else {
None
}
} else {
None
}
}
} else {
None
};

// Replace any LexicalRead with undeclared let names with undefined
// Replace any LexicalRead with undeclared let names with undefined.
// This runs BEFORE marking the current op's name as declared, so
// self-references in the declaration op are also replaced.
let let_names_ref = &let_names;
let declared_ref = &declared_names;

Expand All @@ -113,6 +123,12 @@ pub fn remove_illegal_let_references(job: &mut ComponentCompilationJob<'_>) {
},
VisitorContextFlag::NONE,
);

// Mark this name as declared AFTER transforming, so subsequent ops
// can legally reference it, but the declaration op itself cannot.
if let Some(name) = newly_declared {
declared_names.push(name);
}
}
}
}
158 changes: 89 additions & 69 deletions crates/oxc_angular_compiler/src/transform/html_to_r3.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use oxc_span::{Atom, Span};
use rustc_hash::{FxHashMap, FxHashSet};

use crate::ast::expression::{
ASTWithSource, AbsoluteSourceSpan, AngularExpression, BindingType, ParseSpan, ParsedEventType,
AbsoluteSourceSpan, AngularExpression, BindingType, ParseSpan, ParsedEventType,
};
use crate::ast::html::{
BlockType, HtmlAttribute, HtmlBlock, HtmlComponent, HtmlDirective, HtmlElement, HtmlExpansion,
Expand Down Expand Up @@ -2025,30 +2025,38 @@ impl<'a> HtmlToR3Transform<'a> {
});
}

let children = self.visit_children(&block.children);
// Match Angular's createIfBlock: only push the main branch when
// parseConditionalBlockParameters succeeds (returns non-null).
// When parameters are empty, Angular returns null and skips the branch.
if main_params.expression.is_some() {
let children = self.visit_children(&block.children);

// Create i18n placeholder if inside an i18n context
let i18n =
self.create_block_placeholder("if", &[], block.span, block.start_span, block.end_span);
// Create i18n placeholder if inside an i18n context
let i18n = self.create_block_placeholder(
"if",
&[],
block.span,
block.start_span,
block.end_span,
);

let main_branch = R3IfBlockBranch {
expression: main_params.expression.map(|e| e.ast),
children,
expression_alias: main_params.expression_alias,
source_span: block.span,
start_source_span: block.start_span,
end_source_span: block.end_span,
name_span: block.name_span,
i18n,
};
branches.push(main_branch);
let main_branch = R3IfBlockBranch {
expression: main_params.expression.map(|e| e.ast),
children,
expression_alias: main_params.expression_alias,
source_span: block.span,
start_source_span: block.start_span,
end_source_span: block.end_span,
name_span: block.name_span,
i18n,
};
branches.push(main_branch);
}

// Validate connected blocks and process @else if and @else blocks
let mut has_else = false;

for (i, connected) in connected_blocks.iter().enumerate() {
let children = self.visit_children(&connected.children);

match connected.block_type {
BlockType::ElseIf => {
// Parse @else if parameters (condition and optional "as" alias)
Expand Down Expand Up @@ -2088,26 +2096,32 @@ impl<'a> HtmlToR3Transform<'a> {
});
}

// Create i18n placeholder if inside an i18n context
let i18n = self.create_block_placeholder(
"else if",
&[],
connected.span,
connected.start_span,
connected.end_span,
);
// Match Angular: only push the branch when params are non-null
// (i.e., when an expression was successfully parsed).
if params.expression.is_some() {
let children = self.visit_children(&connected.children);

let branch = R3IfBlockBranch {
expression: params.expression.map(|e| e.ast),
children,
expression_alias: params.expression_alias,
source_span: connected.span,
start_source_span: connected.start_span,
end_source_span: connected.end_span,
name_span: connected.name_span,
i18n,
};
branches.push(branch);
// Create i18n placeholder if inside an i18n context
let i18n = self.create_block_placeholder(
"else if",
&[],
connected.span,
connected.start_span,
connected.end_span,
);

let branch = R3IfBlockBranch {
expression: params.expression.map(|e| e.ast),
children,
expression_alias: params.expression_alias,
source_span: connected.span,
start_source_span: connected.start_span,
end_source_span: connected.end_span,
name_span: connected.name_span,
i18n,
};
branches.push(branch);
}
}
BlockType::Else => {
// Validation: check for duplicate @else
Expand All @@ -2133,16 +2147,19 @@ impl<'a> HtmlToR3Transform<'a> {
}
has_else = true;

// @else has no condition (null expression) and no alias
let children = self.visit_children(&connected.children);

// Create i18n placeholder if inside an i18n context
// (must be after visit_children to match @if/@else if ordering
// and preserve i18n placeholder numbering)
let i18n = self.create_block_placeholder(
"else",
&[],
connected.span,
connected.start_span,
connected.end_span,
);

// @else has no condition (null expression) and no alias
let branch = R3IfBlockBranch {
expression: None,
children,
Expand Down Expand Up @@ -2233,32 +2250,8 @@ impl<'a> HtmlToR3Transform<'a> {
});
}

let item = params.item;
let expression = params.expression;
let context_variables = params.context_variables;

// Get track expression or create empty one for error recovery.
// Only report missing-track error if the expression itself parsed successfully
// (matching Angular which returns null params and skips track validation on parse failure).
let (track_by, track_keyword_span) = if let Some(track_info) = params.track_by {
(track_info.expression, track_info.keyword_span)
} else {
if !expression_parse_failed {
// Track is required but missing - report error and create empty for error recovery
self.report_error("@for loop must have a \"track\" expression", block.start_span);
}
let empty_ast = ASTWithSource {
ast: self.create_empty_expression(block.span),
source: None,
location: Atom::from(""),
absolute_offset: block.span.start,
};
(empty_ast, block.name_span)
};

let children = self.visit_children(&block.children);

// Process and validate connected @empty block
// Process and validate connected @empty block.
// Angular processes connected blocks before checking params, so we do too.
let mut empty: Option<R3ForLoopBlockEmpty<'a>> = None;

for connected in connected_blocks {
Expand Down Expand Up @@ -2304,6 +2297,28 @@ impl<'a> HtmlToR3Transform<'a> {
}
}

// Match Angular's createForLoop: if expression parsing failed entirely
// (params === null in Angular), return None — no ForLoopBlock is emitted.
if expression_parse_failed {
return None;
}

let item = params.item;
let expression = params.expression;
let context_variables = params.context_variables;

// Match Angular's createForLoop: if track is missing and expression parsed OK,
// report the error and return None (Angular only creates the node when
// params !== null AND params.trackBy !== null).
let (track_by, track_keyword_span) = if let Some(track_info) = params.track_by {
(track_info.expression, track_info.keyword_span)
} else {
self.report_error("@for loop must have a \"track\" expression", block.start_span);
return None;
};

let children = self.visit_children(&block.children);
Comment thread
Brooooooklyn marked this conversation as resolved.

// Calculate the outer span to encompass @empty block if present
let end_source_span =
if let Some(last) = connected_blocks.last() { last.end_span } else { block.end_span };
Expand Down Expand Up @@ -2349,14 +2364,19 @@ impl<'a> HtmlToR3Transform<'a> {
use crate::ast::html::{BlockType, HtmlNode};
use crate::ast::r3::{R3SwitchBlockCase, R3SwitchBlockCaseGroup};

// Validation: @switch must have exactly one parameter
let expression = if block.parameters.len() == 1 {
// Validation: @switch must have exactly one parameter.
// Match Angular's createSwitchBlock: always parse the first parameter when present
// (even if there are extra parameters), only use empty when there are no parameters.
// Validation errors are reported by validateSwitchBlock in Angular; we report inline.
if block.parameters.len() != 1 {
self.report_error("@switch block must have exactly one parameter", block.start_span);
}
let expression = if !block.parameters.is_empty() {
let expr_str = block.parameters[0].expression.as_str();
let parsed = self.binding_parser.parse_binding(expr_str, block.parameters[0].span);
parsed.ast
} else {
self.report_error("@switch block must have exactly one parameter", block.start_span);
self.create_empty_expression(block.span)
self.binding_parser.parse_binding("", block.span).ast
};

let mut groups = Vec::new_in(self.allocator);
Expand Down
Loading
Loading