feat(html): recognize Angular component templates#9658
feat(html): recognize Angular component templates#9658raashish1601 wants to merge 2 commits intobiomejs:mainfrom
Conversation
|
Merging this PR will not alter performance
Comparing Footnotes
|
WalkthroughThis PR adds support for Angular framework to Biome's HTML parser and formatter. Changes include introducing Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_html_parser/src/parser.rs`:
- Around line 180-182: HtmlVariant::Angular currently enables double-text
expression but doesn't mark the options as HTML, so parser.options().is_html()
remains false; update the Angular branch (where options =
options.with_double_text_expression() is called) to also set the HTML flag on
the parser options (e.g., call the options method that marks HTML, such as
options.with_is_html(true) or the equivalent setter) so Angular templates are
treated as HTML by parser.options().is_html().
In `@crates/biome_html_parser/tests/spec_tests.rs`:
- Around line 121-148: The test currently hardcodes HtmlFileSource::angular()
and doesn't exercise path-based detection or invalid fixtures; add a real spec
pair (one ok and one error) named with the .component.html suffix under the
parser spec runner (tests/specs/... for the Angular component group) where the
error case contains malformed Angular template to trigger diagnostics, then
update the gen_tests! glob/selector used to generate parser tests so it picks up
*.component.html files (or adjust the path-based detection logic used by
parse_html() to infer HtmlFileSource from filenames) and refactor the
parses_angular_component_templates test to load the generated spec cases instead
of calling HtmlFileSource::angular() directly (referencing parse_html,
HtmlFileSource::angular, and the gen_tests! macro to locate where to change).
In `@crates/biome_service/src/file_handlers/html.rs`:
- Around line 518-526: The Angular structural directive handling currently
treats any attribute starting with '*' as a JS embed; modify the logic around
HtmlAttribute::cast_ref / build_angular_binding_candidate /
EmbedDetectorsRegistry::detect_match / parse_matched_embed so you first validate
the candidate binding form and skip parsing when it is Angular microsyntax (e.g.
patterns containing "let", "of", "as", "else" or semicolon-separated microsyntax
like "cond; else tpl"); implement a small helper (e.g.
is_angular_microsyntax(&candidate) or validate_binding_form(&candidate)) that
returns true for microsyntax and short-circuits before calling
detect_match/parse_matched_embed, thereby narrowing the '*' predicate to only
run the JS embed path for genuine JS expressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a2885f3d-9b9b-4357-b2f9-6c3bef98a893
📒 Files selected for processing (6)
crates/biome_html_parser/src/parser.rscrates/biome_html_parser/tests/spec_test.rscrates/biome_html_parser/tests/spec_tests.rscrates/biome_html_syntax/src/file_source.rscrates/biome_service/src/file_handlers/html.rscrates/biome_service/src/workspace/server.tests.rs
| HtmlVariant::Angular => { | ||
| options = options.with_double_text_expression(); | ||
| } |
There was a problem hiding this comment.
Carry the HTML flag into Angular parser options.
Line 180 enables {{ ... }} but still leaves is_html at its default false. The parser uses p.options().is_html() when deciding whether uppercase tags are HTML or components, so Angular templates currently take the non-HTML path here.
🛠️ Possible fix
HtmlVariant::Angular => {
- options = options.with_double_text_expression();
+ options.is_html = true;
+ options = options.with_double_text_expression();
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_html_parser/src/parser.rs` around lines 180 - 182,
HtmlVariant::Angular currently enables double-text expression but doesn't mark
the options as HTML, so parser.options().is_html() remains false; update the
Angular branch (where options = options.with_double_text_expression() is called)
to also set the HTML flag on the parser options (e.g., call the options method
that marks HTML, such as options.with_is_html(true) or the equivalent setter) so
Angular templates are treated as HTML by parser.options().is_html().
| #[test] | ||
| fn parses_angular_component_templates() { | ||
| let html = r#" | ||
| <input | ||
| [value]="value" | ||
| (input)="handleInput($event)" | ||
| [(ngModel)]="model" | ||
| *ngIf="visible" | ||
| /> | ||
| <p>{{ theme }}</p> | ||
| "#; | ||
| let syntax = parse_html(html, HtmlParserOptions::from(&HtmlFileSource::angular())); | ||
|
|
||
| assert!( | ||
| syntax.diagnostics().is_empty(), | ||
| "expected Angular component template sample to parse without diagnostics: {:?}", | ||
| syntax.diagnostics() | ||
| ); | ||
| assert!( | ||
| syntax | ||
| .tree() | ||
| .syntax() | ||
| .descendants() | ||
| .find_map(HtmlDoubleTextExpression::cast) | ||
| .is_some(), | ||
| "expected Angular component template sample to contain a double text expression" | ||
| ); | ||
| } |
There was a problem hiding this comment.
Please add a real .component.html spec pair.
This test is useful, but Line 132 hardcodes HtmlFileSource::angular(), so it still sidesteps the path-based branch, and there is no invalid Angular fixture exercising diagnostics. I'd add at least one ok and one error *.component.html case under the spec runner, then teach the gen_tests! glob to pick them up.
As per coding guidelines, "All code changes MUST include appropriate tests: lint rules require snapshot tests in 'tests/specs/{group}/{rule}/', formatters require snapshot tests with valid/invalid cases, parsers require test files covering valid and error cases, and bug fixes require tests that reproduce and validate the fix."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_html_parser/tests/spec_tests.rs` around lines 121 - 148, The
test currently hardcodes HtmlFileSource::angular() and doesn't exercise
path-based detection or invalid fixtures; add a real spec pair (one ok and one
error) named with the .component.html suffix under the parser spec runner
(tests/specs/... for the Angular component group) where the error case contains
malformed Angular template to trigger diagnostics, then update the gen_tests!
glob/selector used to generate parser tests so it picks up *.component.html
files (or adjust the path-based detection logic used by parse_html() to infer
HtmlFileSource from filenames) and refactor the
parses_angular_component_templates test to load the generated spec cases instead
of calling HtmlFileSource::angular() directly (referencing parse_html,
HtmlFileSource::angular, and the gen_tests! macro to locate where to change).
| if let Some(attribute) = HtmlAttribute::cast_ref(&element) | ||
| && let Some(candidate) = build_angular_binding_candidate(&attribute) | ||
| && let Some(embed_match) = EmbedDetectorsRegistry::detect_match( | ||
| HostLanguage::Html, | ||
| &candidate, | ||
| &doc_file_source, | ||
| ) | ||
| && let Some(parsed) = | ||
| parse_matched_embed(&candidate, &embed_match, &mut ctx, None) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Angular templates, are structural directive values such as *ngFor="let item of items"and*ngIf="cond; else tpl" valid JavaScript expressions, or do they use Angular microsyntax?
💡 Result:
In Angular templates, structural directive values such as *ngFor="let item of items" and ngIf="cond; else tpl" use Angular microsyntax, not valid standalone JavaScript expressions. The asterisk () is syntactic sugar that Angular desugars into an with the directive applied. The string after the = (e.g., let item of items or cond; else tpl) follows Angular's structural directive microsyntax grammar, which includes special keywords like let, of, as, and else separated by semicolons (;). This microsyntax is parsed by Angular to generate bindings like [ngForOf]="items" let-item="$implicit", which are not valid JavaScript. While parts like conditions in *ngIf="cond" are Angular template expressions (a subset of JavaScript), the full directive syntax with microsyntax elements is Angular-specific and not executable JavaScript.
Citations:
- 1: https://angular.dev/guide/directives/structural-directives
- 2: https://v2.angular.io/docs/ts/latest/guide/structural-directives.html
- 3: https://www.lucaspaganini.com/academy/angular-structural-directives-and-their-microsyntax
- 4: https://goetzrobin.github.io/blog/2023-02-12-mastering-angular-structural-directives-micro-syntax-demystified/
- 5: https://angular.dev/api/common/NgIf
The * predicate will incorrectly classify Angular structural directive microsyntax as JavaScript.
Lines 1118–1121 match any attribute starting with *, and the new Angular flow (lines 518–526) will attempt to parse structural directive values like *ngFor="let item of items" or *ngIf="cond; else tpl" as JavaScript. These expressions use Angular's microsyntax with keywords like let, of, as, and else, and are not valid JavaScript. Narrow the * case to exclude directives with microsyntax, or validate the binding form before treating it as a JS embed.
Also applies to: 1117–1122
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_service/src/file_handlers/html.rs` around lines 518 - 526, The
Angular structural directive handling currently treats any attribute starting
with '*' as a JS embed; modify the logic around HtmlAttribute::cast_ref /
build_angular_binding_candidate / EmbedDetectorsRegistry::detect_match /
parse_matched_embed so you first validate the candidate binding form and skip
parsing when it is Angular microsyntax (e.g. patterns containing "let", "of",
"as", "else" or semicolon-separated microsyntax like "cond; else tpl");
implement a small helper (e.g. is_angular_microsyntax(&candidate) or
validate_binding_form(&candidate)) that returns true for microsyntax and
short-circuits before calling detect_match/parse_matched_embed, thereby
narrowing the '*' predicate to only run the JS embed path for genuine JS
expressions.
Summary
.component.htmlfiles and theangularlanguage id as Angular HTML templatesTesting
cargo fmt -p biome_html_syntax -p biome_html_parser -p biome_servicecargo test -p biome_html_syntax recognizes_angular --lib -j 1blocked locally because the checkout ran out of disk space while buildingtargetcargo test -p biome_html_parser parses_angular_component_templates --test spec_testsblocked locally because the checkout ran out of disk space while buildingtarget