feat(html): improved parsing spread attributes#8894
Conversation
🦋 Changeset detectedLatest commit: d1930a3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
2ee68a3 to
abbfc67
Compare
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
Footnotes
|
WalkthroughThis PR adds parsing support for spread attributes in Svelte and Astro files, enabling syntax like Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@crates/biome_html_parser/src/syntax/mod.rs`:
- Around line 739-758: The current condition in parse_name misuses
should_bail_if and causes tokens to be consumed when callers pass |_| true;
change the early-return to bail when a name is missing AND should_bail_if(p) is
true (i.e., replace `if !p.at(IDENT) && !should_bail_if(p) { return Absent; }`
with `if !p.at(IDENT) && should_bail_if(p) { return Absent; }`) so missing names
fail instead of being consumed, and update any call sites that relied on the old
inverted semantics of the should_bail_if closure.
- Around line 465-472: The match arm ordering currently checks
SingleTextExpressions before Astro, so when both flags are enabled (Astro
frontmatter + SingleTextExpressions) the parse_astro_spread_or_expression branch
is shadowed; swap the two guards so the T!['{'] if Astro.is_supported(p) =>
parse_astro_spread_or_expression(p) arm comes before the T!['{'] if
SingleTextExpressions.is_supported(p) => parse_svelte_spread_or_expression(p)
arm (leave the fallback T!['{'] exclusive parse_exclusive_syntax entry
unchanged) to ensure Astro-specific parsing runs first.
In `@crates/biome_html_parser/src/syntax/svelte.rs`:
- Around line 359-389: parse_svelte_spread_or_expression currently calls
parse_name after consuming "..." which only accepts a single identifier; replace
that with the general expression parser so spread attributes accept arbitrary JS
expressions. Specifically, in parse_svelte_spread_or_expression, instead of
parse_name(p, HtmlLexContext::Svelte, ...), invoke the module's general
expression parsing entry (the same parser used for text expressions—e.g., the
logic behind parse_single_text_expression or a parse_expression(p,
HtmlLexContext::Svelte) helper) to parse any valid expression, keep the existing
diagnostic (.or_add_diagnostic(...)) if parsing fails, and then expect the
closing '}' as before before completing HTML_SPREAD_ATTRIBUTE.
In `@xtask/codegen/html.ungram`:
- Line 212: The grammar rule for spread attribute currently restricts the
argument to an identifier via "HtmlName = 'ident'" / "argument: HtmlName", which
rejects valid JS expressions (e.g., {...foo.bar}, {...getProps()}, {...cond ? a
: b}); change the grammar so the spread attribute argument uses a generic
expression production instead of HtmlName (e.g., use the existing Expression or
JsExpression nonterminal) and update the parser code handling the spread
attribute (the parser branch that consumes "argument" in the spread-attribute
handling code) to parse and store a full expression node rather than only an
identifier; apply the same change to the other spread-attribute occurrences
referenced (the second spread-attribute production area) so both grammar and
parser accept arbitrary expressions.
dyc3
left a comment
There was a problem hiding this comment.
I tested this on a private repo I have and https://github.com/huntabyte/shadcn-svelte. This definitely resolves a lot of the parsing errors, but IMO there's still too many false positives for the rules to fully close #8590.
Yeah definitely. I updated the PR description, and we could use that issue to collect issues/bugs |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_html_parser/src/syntax/mod.rs (1)
427-448: Remove duplicate match arm.The second
T!["{{"]arm (lines 438-448) is unreachable—Rust's match evaluates top-to-bottom, so the first arm at lines 427-436 always wins. This looks like a merge artefact.🐛 Proposed fix
T!["{{"] => { let m = p.start(); DoubleTextExpressions .parse_exclusive_syntax( p, |p| parse_double_text_expression(p, HtmlLexContext::InsideTag), |p, marker| disabled_interpolation(p, marker.range(p)), ) .ok(); Present(m.complete(p, HTML_ATTRIBUTE)) } - T!["{{"] => { - let m = p.start(); - HtmlSyntaxFeatures::DoubleTextExpressions - .parse_exclusive_syntax( - p, - |p| parse_double_text_expression(p, HtmlLexContext::InsideTag), - |p, marker| disabled_interpolation(p, marker.range(p)), - ) - .ok(); - - Present(m.complete(p, HTML_ATTRIBUTE)) - }
🧹 Nitpick comments (1)
crates/biome_html_parser/src/syntax/astro.rs (1)
46-77: LGTM!The implementation correctly handles Astro spread attributes and mirrors the Svelte implementation. The comment explaining why
HtmlLexContext::Svelteis used for lexing...is helpful.The logic is nearly identical to
parse_svelte_spread_or_expressioninsvelte.rs. If spread parsing grows more complex, consider extracting a shared helper that takes the feature check as a parameter.
Summary
Closes #8887
Part of #8590
Closes #8851
I believe we're almost there, and we are stable enough to finally say that Svelte is definitely more stable and usable.
Test Plan
Added new tests
Docs
N/A