feat(html): implement complete Glimmer template syntax support #8106
Conversation
🦋 Changeset detectedLatest commit: 9f762e2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 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 |
github-actions
bot
added
A-Project
wagenet
changed the title
This commit implements Phase 1 of Glimmer template support in Biome's JavaScript parser. The implementation allows Biome to recognize and parse `<template>` blocks in Glimmer Component files (.gjs/.gts), treating them as opaque tokens without parsing their internal Glimmer syntax.
Key changes:
- Added file source support for .gjs (Glimmer JavaScript) and .gts (Glimmer TypeScript) files
- Implemented lexer support for detecting template boundaries (<template>...</template>)
- Added GLIMMER_TEMPLATE token type and JsGlimmerTemplate AST node
- Integrated Glimmer templates as valid expressions, statements, and class members
- Created verbatim formatter to preserve template content exactly as written
- Updated analyzer rules to handle the new AST node type
- Added comprehensive test suite with 6 test files covering various scenarios
Templates are now recognized in:
- Variable assignments: `const Tpl = <template>...</template>;`
- Class bodies: `class C { <template>...</template> }`
- Expression contexts: any place where expressions are valid
- Single unassigned templates (treated as default exports)
The lexer emits helpful diagnostics for unclosed templates.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
…plate support This PR was written primarily by Claude Code. Fixes three critical issues identified in CodeRabbit review of PR biomejs#8099: 1. **Template tag detection bug**: Fixed byte slice comparison in lexer that was checking `b"template>"` but using incorrect offset. Changed to `b"template"` to properly validate characters after `<template`. Now supports whitespace in opening tags (e.g., `<template >`, `<template\t>`). 2. **Missing lookahead support**: Added `GLIMMER_TEMPLATE` to `is_nth_at_expression()` to properly recognize templates in expression contexts like `yield <template>...`, `await <template>...`, etc. 3. **LSP language ID support**: Added "gjs" and "gts" language identifiers to `try_from_language_id()` for proper LSP integration. Also enhanced changeset documentation to clarify Phase 1 implementation treats templates as opaque tokens without internal parsing/linting. All changes follow TDD approach with tests written first to verify failures, then fixes applied, and verification that tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Moved all glimmer template test files and snapshots into dedicated glimmer/ subdirectories under both ok/ and error/ test folders. This addresses review feedback to improve test organization.
…plates Added 6 new error test cases to verify lexer backtracking behavior: - glimmer_lookalike_temp.gjs: Tests <temp> (not "template") - glimmer_lookalike_templatex.gjs: Tests <templatex> (identifier after) - glimmer_lookalike_div.gjs: Tests that JSX is forbidden in .gjs files - glimmer_incomplete_templat.gjs: Tests incomplete "templat" at EOF - glimmer_incomplete_less_than.gjs: Tests bare < at EOF - glimmer_template_self_closing.gjs: Tests self-closing template tag These tests establish baseline behavior before fixing the lexer backtracking issue identified in code review.
…mments and simplified loop - Added comprehensive comments to `try_lex_glimmer_template` documenting the lookahead phase (non-advancing checks) and commit phase - Clarified that lexer position is only advanced after confirming valid template - Simplified the attribute-skipping loop by consolidating duplicate next_byte() calls - All checks use non-advancing operations (peek_byte, byte_at, slicing) until we're certain we have a valid `<template>` tag This addresses review feedback about lexer backtracking behavior and loop simplification.
Implements the architecture pattern where parser options are automatically derived from JsFileSource using `impl From<&JsFileSource> for JsParserOptions`. This follows the HTML parser's design and centralizes configuration logic: - Parser options are now derived from file characteristics (language, variant, embedding kind) rather than being explicitly passed - Main `parse()` functions derive options automatically - Added `parse_with_options()` for cases needing custom options (testing) - Updated `TokenSource::from_str` to derive options internally - Updated `JsParser::new` to derive options, added `with_options` for testing Updated call sites: - Test harness uses `parse_with_options` when custom options are configured - Grit patterns use `parse_with_options` for metavariable support - Coverage tools updated to use new API This addresses review feedback requesting parser options architecture improvements. Note: biome_service file handlers still need to be updated to use the new API - this will be done in a follow-up commit.
Updated all file handlers to use the new parse API where options are derived from file source type: - JavaScript handler: removed options parameter - Astro handler: removed JsParserOptions::default() - Vue handler: removed JsParserOptions::default() - Svelte handler: removed JsParserOptions::default() - Script tag parser: removed JsParserOptions::default() - Test utils: removed JsParserOptions::default() All handlers now automatically derive parser options from JsFileSource using the impl From<&JsFileSource> for JsParserOptions pattern.
Simplify docstring from 16 lines to 3 lines by removing elaborate example with limited practical value. The straightforward implementation just writes the template token verbatim.
…om API changes Fix all compilation errors resulting from parser options refactoring: - Update spec_test.rs to use parse_with_options() - Fix parser.rs test functions to use with_options() constructor - Update all stale doc comments in parse.rs to reflect new API signatures - Remove unused options variable in javascript.rs - Remove unused JsParserOptions imports across multiple files - Fix malformed parse calls in xtask/coverage test files - Simplify options.rs From implementation - Prefix unused parameters with underscores All changes maintain backward compatibility through parse_with_options() for cases requiring explicit options.
Remove unused parse and JsParserOptions imports that are no longer needed after updating to the new parser API.
Add support for Glimmer (.gjs, .gts) files in the HTML parser:
- Added HtmlVariant::Glimmer enum variant
- Added factory method HtmlFileSource::glimmer()
- Added is_glimmer() helper method
- Support for .gjs and .gts file extensions
- Support for glimmer, glimmer-js, glimmer-ts language IDs
- Configure Glimmer to use double text expressions (for {{ }})
- Added comprehensive tests following TDD approach
This is Task 1 of Phase 2 implementation for Glimmer/Ember support.
🤖 Generated with Claude Code
Co-Authored-By: Claude <[email protected]>
Add punctuation tokens needed for Glimmer syntax parsing:
- HASH (#) - for block helpers like {{#if}}
- DOT (.) - for property access like {{this.property}}
- PIPE (|) - for block params like {{#each as |item|}}
- AT (@) - for arguments like {{helper @arg}}
- DOTDOTDOT (...) - for splattributes like ...attributes
- L_PAREN (() - for subexpressions like {{foo (bar baz)}}
- R_PAREN ()) - for subexpressions
Updated html_kinds_src.rs with new punctuation tokens and ran
codegen to generate the corresponding HtmlSyntaxKind variants.
This is Task 2 of Phase 2 implementation for Glimmer/Ember support.
🤖 Generated with Claude Code
Co-Authored-By: Claude <[email protected]>
Add comprehensive Glimmer grammar definition to html.ungram:
- GlimmerMustacheExpression: {{path args}}
- GlimmerBlockHelper: {{#helper}}...{{/helper}}
- GlimmerPath: for identifiers, this.property, @Args
- GlimmerArgumentList: positional and named arguments
- GlimmerSubexpression: (helper arg1 arg2) for nested syntax
- GlimmerSplattribute: ...attributes
- Supporting nodes: block params, literals, etc.
Updated html_kinds_src.rs with all Glimmer node types and ran
codegen to generate the AST node definitions, visitors, and factory
methods.
This addresses the nested syntax question - subexpressions handle
{{foo (bar baz)}} and {{(foo bar)}} patterns.
This is Task 3 of Phase 2 implementation for Glimmer/Ember support.
🤖 Generated with Claude Code
Co-Authored-By: Claude <[email protected]>
Extended Glimmer grammar with missing features from PHASE_2_FINAL_PLAN:
- Added tokens: COLON, L_TRIPLE_CURLY, R_TRIPLE_CURLY, MUSTACHE_COMMENT
- Added grammar nodes:
* GlimmerMustacheComment for {{! }} and {{!-- --}} comments
* GlimmerTripleStashExpression for {{{ }}} unescaped HTML
* GlimmerElementModifier for {{modifier}} in attribute position
* GlimmerNamedBlock with opening/closing for <:name>...</:name>
- Updated AnyGlimmerExpression union to include all expression types
- Updated AnyHtmlAttribute union to include GlimmerElementModifier
- Ran grammar codegen to generate AST nodes and factory methods
This completes Tasks 2 and 3 of Phase 2 implementation.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Implemented full support for Glimmer mustache comments in the HTML parser:
**Lexer changes:**
- Added `at_mustache_comment()` check for {{! patterns
- Added `consume_mustache_comment()` to handle both single-line ({{! }}) and multi-line ({{!-- --}}) comments
- Updated token consumption logic in `consume_token_inside_tag` and `consume_token_outside_tag`
**Parser changes:**
- Added `parse_mustache_comment()` function to create GlimmerMustacheComment nodes
- Integrated mustache comment parsing into ElementList
- Fixed spec_test.rs to support Glimmer variant
**Formatter fixes:**
- Added `mod js;` declaration to lib.rs
- Fixed verbatim_node references in generated formatter code
- Added GlimmerElementModifier and GlimmerSplattribute to attribute list match
**Test:**
- Created test file with single-line and multi-line mustache comments
- Test passes with correct AST generation
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Implements parsing for Glimmer/Ember template block helpers, completing
Phase 2 of the Glimmer implementation plan.
Block helpers are control flow structures in Glimmer templates:
- `{{#if condition}}...{{/if}}` - conditionals
- `{{#each items as |item|}}...{{/each}}` - iteration with block params
- `{{#with context as |var|}}...{{/with}}` - context binding
Changes:
- Added lexer support for `#`, `/`, and `|` tokens in DoubleGlimmer context
- Implemented `parse_glimmer_block_helper()` with opening/closing tag parsing
- Implemented `parse_glimmer_block_params()` for destructuring parameters
- Enhanced argument list parser to detect and stop at block params
- Added manual child content parsing loop to handle nested elements
- Made `ElementList` public for use in Glimmer submodule
Tests:
- Added comprehensive test cases for block helpers with various patterns
- All 75 parser tests pass including HTML, Vue, Svelte, Astro, and Glimmer
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughAdds comprehensive Glimmer template support to Biome, enabling parsing, formatting, and linting of .gjs and .gts Component files. Introduces Glimmer syntax recognition in both HTML and JavaScript contexts, including mustache expressions, block helpers, triple-stash blocks, and named blocks. Phase 1 treats template content as opaque tokens with syntax validation. Updates lexers, parsers, formatters, grammar definitions, and analysis tools across the codebase, plus simplifies the parser API to derive options from file source type. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
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/lexer/mod.rs (1)
39-54: Glimmer identifiers restricted incorrectly;doctypekeyword check fires prematurelyBoth issues are confirmed by the lexer implementation:
Dashes forbidden in Glimmer identifiers
Lines 478–490 restrict
IdentifierContext::Glimmertobyte.is_ascii_alphanumeric() || byte == b'_', so{{my-helper}}tokenises asIDENT("my")+ERROR_TOKEN('-')+IDENT("helper"). Glimmer component/helper names commonly use dashes; this needs fixing.
doctypekeyword fires in Glimmer contextLine 495 shows
b"doctype" | b"DOCTYPE" if !context.is_svelte(). Since Glimmer is not Svelte, this guard is true and producesDOCTYPE_KWinstead of falling through to theIDENTat line 500. Result:{{doctype}}won't parse as a Glimmer identifier.Suggested fixes:
- Tighten the doctype guard to
context.is_doctype()so it only fires in actual DOCTYPE context.- Extend the Glimmer character set to include
-(and possibly other safe characters like:, depending on Glimmer spec).
♻️ Duplicate comments (4)
crates/biome_html_formatter/src/js/auxiliary/block_helper_closing.rs (1)
1-14: Same duplication concern as block_param.rs.This formatter exhibits the same dual-implementation pattern:
FormatNodeRulehere andAsFormat/IntoFormatinglimmer.rs(line 47). See the verification script in the block_param.rs review.crates/biome_html_formatter/src/js/auxiliary/named_block.rs (1)
1-10: Same duplication concern as block_param.rs.Dual implementation detected:
glimmer.rsline 59 also provides formatting forGlimmerNamedBlock.crates/biome_html_formatter/src/js/auxiliary/path.rs (1)
1-10: Same duplication concern as block_param.rs.
glimmer.rsline 50 also implements formatting forGlimmerPath.crates/biome_html_formatter/src/js/auxiliary/element_modifier.rs (1)
1-10: Same duplication concern as block_param.rs.
glimmer.rsline 58 also implements formatting forGlimmerElementModifier.
🧹 Nitpick comments (10)
crates/biome_js_formatter/src/utils/format_node_without_comments.rs (1)
96-96: Double‑check comment handling forJsGlimmerTemplateUsing
node.format().fmt(f)here is slightly different from the other arms that delegate to explicitFormat*types via.fmt_node. Given this is the “without comments” formatter, it would be good to confirm that the Glimmer template’sformat()implementation also omits comments in the same way as the others.If there is a
FormatJsGlimmerTemplate(or similar) rule wired injs::auxiliary::glimmer_template, you may want to mirror the surrounding style and delegate to that instead for consistency.crates/biome_html_parser/src/token_source.rs (1)
60-69: Enum variants look right; consider clarifying commentsAdding
TripleandDoubleGlimmerto distinguish Glimmer’s triple‑stash and mustache forms makes sense and keepsDoubleas the generic{{…}}case.The two comments for
DoubleandDoubleGlimmerboth read as// {{ expr }}, which might be a bit opaque to future readers. It may be worth spelling out the distinction explicitly (e.g. “generic double‑curlies (Vue etc.)” vs “Glimmer mustache inside Glimmer templates”) to avoid head‑scratching later..changeset/glimmer-template-support.md (1)
1-13: Clear documentation of the new feature.The changeset accurately describes the Glimmer template support. One optional enhancement: you might consider explicitly noting that Glimmer syntax within templates receives verbatim formatting in this initial implementation (as opposed to full Glimmer syntax formatting).
crates/biome_html_parser/tests/spec_test.rs (1)
143-150: Ignoredquick_testis fine, but consider documenting intentThe
quick_testharness usingHtmlFileSource::glimmer()is handy for debugging and is safely ignored. If you expect to keep it around long term, a short comment about its purpose might save future head‑scratching.crates/biome_html_formatter/src/js/auxiliary/block_helper.rs (1)
1-10: Verbatim Glimmer block helper formatting looks appropriateUsing
format_html_verbatim_node(node.syntax())keeps Glimmer block helpers faithful to the original template and avoids premature “cleverness”. Nice and simple; future non‑verbatim formatting can always refine this implementation.crates/biome_html_parser/src/syntax/glimmer/mod.rs (4)
19-56: GlimmerPath happily accepts@with no segments
parse_glimmer_pathwill happily build aGLIMMER_PATHfor@with no following segment (the initialATpasses theAbsentguard, and the segment list may be empty). The grammar comment suggests at least oneGlimmerPathSegment, so this ends up as a structurally invalid but syntactically “successful” path.If you want better diagnostics, consider flagging the missing segment when an
@is present but noIDENT/literal follows, e.g. by emitting a parser error and/or using a bogus node variant rather than building an emptyGlimmerPathSegmentList.
65-129: Block-param detection in argument list is a bit too heuristicThe
parse_glimmer_argument_listdisambiguation for block params treats anyIDENTimmediately followed by|as the start of block params and rewinds, regardless of the identifier’s text. That’s fine foras |...|but will also treat something likehelper foo |baras “block params” and stop parsing arguments.Given the grammar and comments are clearly keyed on the
askeyword, it would be safer (and produce nicer errors) to special‑case only an identifier with text"as"here, or otherwise tighten the condition, and leave otherident |sequences to be handled (or rejected) at a higher level.
137-172: Unclosed{{leaves the lexer in Glimmer mode indefinitelyWhen the closing
}}is missing inparse_glimmer_mustache_expression, you emit an error but don’t bump any token withHtmlLexContext::Regular, so subsequent tokens continue to be lexed inDoubleGlimmercontext.That’s good for preserving more Glimmer tokens, but it can also cause fairly wild follow‑on diagnostics. It may be worth considering a recovery path that still forces a context switch back to
Regular(for example by consuming tokens until the next}}/>/newline and then resuming in HTML mode), so a single missing brace doesn’t taint the rest of the file.
255-385: Block params parse/message are slightly misleading for non‑asidents
parse_glimmer_block_helper_openingandparse_glimmer_block_paramstreat anyIDENTfollowed by|as theastoken for block params, but the error message is hard‑coded to"Expected '|' after 'as'".If someone writes
{{#each items foo |item|}}the message mentioning'as'is a bit confusing. Either:
- Check the ident text and only parse block params when it’s actually
as, or- Reword the diagnostic to a neutral “Expected
|after block params introducer”‑style message.The current behaviour isn’t wrong, but the diagnostics could be clearer.
xtask/codegen/html.ungram (1)
69-85: Glimmer comment/triple‑stash are listed twice in the element/content unionsIn the grammar:
AnyHtmlElementincludesGlimmerMustacheCommentandGlimmerTripleStashExpressiondirectly, andAnyHtmlContentincludesAnyGlimmerExpression, whose union also containsGlimmerMustacheCommentandGlimmerTripleStashExpression.That gives two routes for the same node types to appear under
AnyHtmlElement, which is slightly redundant and potentially ambiguous for tooling that reads the ungrammar.It’s not wrong, but if you want to keep the grammar minimal, you could drop the direct entries from either
AnyHtmlElementorAnyGlimmerExpressionand let a single union own those variants.Also applies to: 221-229, 242-252
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (23)
crates/biome_html_factory/src/generated/node_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_html_factory/src/generated/syntax_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_html_parser/tests/html_specs/ok/glimmer/block_helper.gjs.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/ok/glimmer/mustache.gjs.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/ok/glimmer/mustache_comment.html.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/ok/glimmer/triple_stash.gjs.snapis excluded by!**/*.snapand included by**crates/biome_html_syntax/src/generated/kind.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_html_syntax/src/generated/macros.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_html_syntax/src/generated/nodes.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_html_syntax/src/generated/nodes_mut.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_js_factory/src/generated/node_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_js_factory/src/generated/syntax_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_js_parser/tests/js_test_suite/error/glimmer_template_unclosed.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_class.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_default_export.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_simple.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_typescript.gts.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_with_content.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_syntax/src/generated/kind.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_js_syntax/src/generated/macros.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_js_syntax/src/generated/nodes.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_js_syntax/src/generated/nodes_mut.rsis excluded by!**/generated/**,!**/generated/**and included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**
📒 Files selected for processing (88)
.changeset/glimmer-template-support.md(1 hunks)crates/biome_grit_patterns/src/grit_target_language/js_target_language/generated_mappings.rs(1 hunks)crates/biome_html_formatter/src/generated.rs(4 hunks)crates/biome_html_formatter/src/glimmer.rs(1 hunks)crates/biome_html_formatter/src/html/any/attribute.rs(1 hunks)crates/biome_html_formatter/src/html/any/content.rs(1 hunks)crates/biome_html_formatter/src/html/any/element.rs(1 hunks)crates/biome_html_formatter/src/html/lists/attribute_list.rs(1 hunks)crates/biome_html_formatter/src/js/any/argument.rs(1 hunks)crates/biome_html_formatter/src/js/any/argument_value.rs(1 hunks)crates/biome_html_formatter/src/js/any/expression.rs(1 hunks)crates/biome_html_formatter/src/js/any/mod.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/block_helper.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/block_helper_closing.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/block_helper_opening.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/block_param.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/block_params.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/element_modifier.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/literal.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/mod.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/mustache_comment.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/mustache_expression.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/named_argument.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/named_block.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/named_block_closing.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/named_block_opening.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/path.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/path_segment.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/positional_argument.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/splattribute.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/string_literal.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/subexpression.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/triple_stash_expression.rs(1 hunks)crates/biome_html_formatter/src/js/bogus/bogus_expression.rs(1 hunks)crates/biome_html_formatter/src/js/bogus/mod.rs(1 hunks)crates/biome_html_formatter/src/js/lists/argument_list.rs(1 hunks)crates/biome_html_formatter/src/js/lists/block_param_list.rs(1 hunks)crates/biome_html_formatter/src/js/lists/mod.rs(1 hunks)crates/biome_html_formatter/src/js/lists/path_segment_list.rs(1 hunks)crates/biome_html_formatter/src/js/mod.rs(1 hunks)crates/biome_html_formatter/src/lib.rs(1 hunks)crates/biome_html_parser/src/lexer/mod.rs(15 hunks)crates/biome_html_parser/src/parser.rs(2 hunks)crates/biome_html_parser/src/syntax/glimmer/mod.rs(1 hunks)crates/biome_html_parser/src/syntax/mod.rs(5 hunks)crates/biome_html_parser/src/token_source.rs(1 hunks)crates/biome_html_parser/tests/html_specs/ok/glimmer/block_helper.gjs(1 hunks)crates/biome_html_parser/tests/html_specs/ok/glimmer/mustache.gjs(1 hunks)crates/biome_html_parser/tests/html_specs/ok/glimmer/mustache_comment.html(1 hunks)crates/biome_html_parser/tests/html_specs/ok/glimmer/triple_stash.gjs(1 hunks)crates/biome_html_parser/tests/spec_test.rs(2 hunks)crates/biome_html_parser/tests/spec_tests.rs(1 hunks)crates/biome_html_syntax/src/element_ext.rs(3 hunks)crates/biome_html_syntax/src/file_source.rs(5 hunks)crates/biome_js_analyze/src/lint/complexity/no_static_only_class.rs(1 hunks)crates/biome_js_analyze/src/lint/correctness/no_invalid_constructor_super.rs(1 hunks)crates/biome_js_analyze/src/lint/nursery/no_unused_expressions.rs(1 hunks)crates/biome_js_analyze/src/lint/style/use_naming_convention.rs(1 hunks)crates/biome_js_formatter/src/generated.rs(1 hunks)crates/biome_js_formatter/src/js/any/class_member.rs(1 hunks)crates/biome_js_formatter/src/js/any/expression.rs(1 hunks)crates/biome_js_formatter/src/js/any/statement.rs(1 hunks)crates/biome_js_formatter/src/js/auxiliary/glimmer_template.rs(1 hunks)crates/biome_js_formatter/src/js/auxiliary/mod.rs(1 hunks)crates/biome_js_formatter/src/js/classes/property_class_member.rs(2 hunks)crates/biome_js_formatter/src/utils/format_node_without_comments.rs(1 hunks)crates/biome_js_parser/src/lexer/mod.rs(4 hunks)crates/biome_js_parser/src/parser.rs(1 hunks)crates/biome_js_parser/src/syntax/class.rs(1 hunks)crates/biome_js_parser/src/syntax/expr.rs(2 hunks)crates/biome_js_parser/src/syntax/stmt.rs(1 hunks)crates/biome_js_parser/src/token_source.rs(1 hunks)crates/biome_js_parser/tests/js_test_suite/error/glimmer_template_unclosed.gjs(1 hunks)crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_class.gjs(1 hunks)crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_default_export.gjs(1 hunks)crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_simple.gjs(1 hunks)crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_typescript.gts(1 hunks)crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_with_content.gjs(1 hunks)crates/biome_js_parser/tests/spec_tests.rs(1 hunks)crates/biome_js_syntax/src/expr_ext.rs(1 hunks)crates/biome_js_syntax/src/file_source.rs(4 hunks)crates/biome_js_syntax/src/parentheses/expression.rs(1 hunks)crates/biome_js_syntax/src/union_ext.rs(1 hunks)crates/biome_service/src/file_handlers/mod.rs(1 hunks)xtask/codegen/html.ungram(3 hunks)xtask/codegen/js.ungram(4 hunks)xtask/codegen/src/html_kinds_src.rs(3 hunks)xtask/codegen/src/js_kinds_src.rs(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-20T16:24:59.781Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7266
File: crates/biome_js_type_info/src/type.rs:94-102
Timestamp: 2025-08-20T16:24:59.781Z
Learning: In crates/biome_js_type_info/src/type.rs, the flattened_union_variants() method returns TypeReference instances that already have the correct module IDs applied to them. These references should be used directly with resolver.resolve_reference() without applying additional module ID transformations, as variant references may originate from nested unions in different modules.
Applied to files:
crates/biome_html_formatter/src/lib.rscrates/biome_js_syntax/src/union_ext.rscrates/biome_html_formatter/src/js/auxiliary/mod.rs
📚 Learning: 2025-10-03T12:28:56.788Z
Learnt from: ematipico
Repo: biomejs/biome PR: 7670
File: crates/biome_service/src/file_handlers/html.rs:744-748
Timestamp: 2025-10-03T12:28:56.788Z
Learning: In Biome's codebase, when creating tokens with new text content, use the factory pattern with functions like `ident(text)` from the respective `*_factory` crates (e.g., `biome_html_factory::make::ident`). There is no `.with_text()` method on tokens. The `ident()` function creates a new detached token with the IDENT kind, which is the standard approach for token construction.
Applied to files:
crates/biome_html_parser/src/token_source.rscrates/biome_html_parser/src/lexer/mod.rs
📚 Learning: 2025-11-09T12:47:46.298Z
Learnt from: ematipico
Repo: biomejs/biome PR: 8031
File: crates/biome_html_parser/src/syntax/svelte.rs:140-147
Timestamp: 2025-11-09T12:47:46.298Z
Learning: In the Biome HTML parser, `expect` and `expect_with_context` consume the current token and then lex the next token. The context parameter in `expect_with_context` controls how the next token (after the consumed one) is lexed, not the current token being consumed. For example, in Svelte parsing, after `bump_with_context(T!["{:"], HtmlLexContext::Svelte)`, the next token is already lexed in the Svelte context, so `expect(T![else])` is sufficient unless the token after `else` also needs to be lexed in a specific context.
Applied to files:
crates/biome_html_parser/src/syntax/mod.rscrates/biome_html_parser/src/lexer/mod.rs
📚 Learning: 2025-10-25T07:22:18.540Z
Learnt from: ematipico
Repo: biomejs/biome PR: 7852
File: crates/biome_css_parser/src/syntax/property/mod.rs:161-168
Timestamp: 2025-10-25T07:22:18.540Z
Learning: In the Biome CSS parser, lexer token emission should not be gated behind parser options like `is_tailwind_directives_enabled()`. The lexer must emit correct tokens regardless of parser options to enable accurate diagnostics and error messages when the syntax is used incorrectly.
Applied to files:
crates/biome_html_parser/src/lexer/mod.rs
🔇 Additional comments (90)
crates/biome_js_parser/tests/spec_tests.rs (1)
4-4: Verified: Glimmer test files exist in both suites.Confirmation that
.gjsand.gtstest files are present:
- OK suite: 4
.gjsfiles + 1.gtsfile incrates/biome_js_parser/tests/js_test_suite/ok/- Error suite: 1
.gjsfile incrates/biome_js_parser/tests/js_test_suite/error/The glob pattern updates are correct and will properly match the existing Glimmer template tests.
xtask/codegen/src/html_kinds_src.rs (3)
75-107: LGTM! Comprehensive Glimmer node coverage.The new Glimmer nodes follow a consistent naming pattern and cover all the necessary constructs mentioned in the PR objectives. The inclusion of GLIMMER_BOGUS_EXPRESSION ensures proper error handling.
42-44: No action required.The separation is intentional and correct. Triple curlies are in the tokens array because the lexer has dedicated functions (
at_opening_triple_text_expression()andat_closing_triple_text_expression()) that explicitly check for and emit these tokens. Double curlies are in the punct array because they're simple 2-character sequences that can be autoparsed. Different lexing strategies warrant different arrays.
23-31: No actionable issues found.The new Glimmer tokens are correctly integrated. Triple curly tokens are properly in the
tokensarray (notpunct), and compound tokens like{:are positioned to be matched before single:during lexing. The Glimmer context handling is active throughout the codebase, so COLON and DOT will be lexed appropriately within their specific contexts. The codegen structure follows established patterns and poses no lexer conflicts.crates/biome_html_syntax/src/element_ext.rs (2)
31-33: Clarify the HtmlCdataSection addition.
HtmlCdataSectionis included alongside Glimmer variants but isn't mentioned in the AI summary. Was this a necessary fix for existing functionality, or is it related to Glimmer support?Also applies to: 43-45, 55-59
26-48: Verify complete Glimmer variant coverage using Rust compiler exhaustiveness checks.The match statements appear consistent across all three methods, handling seven variants identically. However, the
AnyHtmlElementenum definition could not be accessed in the search results (likely auto-generated). Rust's exhaustiveness checking would catch missing variants at compile time, so if this compiles without warnings, coverage should be complete.Please confirm that all
Glimmer*variants inAnyHtmlElementare represented in the match arms ofis_javascript_tag(),is_style_tag(), andfind_attribute_by_name()methods.crates/biome_js_formatter/src/js/classes/property_class_member.rs (1)
137-137: LGTM — Glimmer template integration looks solid.Both additions are logically sound and consistent with existing patterns:
- Line 137: Glimmer templates correctly grouped with constructs that don't need semicolons (they start with
<template>, unambiguous syntax)- Line 174: Correctly marked as having no modifiers (consistent with empty members and static blocks)
The AST node is already integrated throughout the codebase (syntax, analyser, other formatters), so these changes slot in naturally.
One minor note: the parser tests include
glimmer_template_class.gjs, so consider adding a formatter snapshot test for class members to document the semicolon handling explicitly. Not required for this change, but would be good future coverage.crates/biome_js_formatter/src/js/auxiliary/mod.rs (1)
14-14: Glimmer template auxiliary module wiring looks goodThe new
glimmer_templatemodule fits the existing auxiliary module pattern and is the expected hook point for the formatter wiring. Nothing to change here.crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_with_content.gjs (1)
1-10: Nice, realistic Glimmer fixtureThis fixture gives a decent spread of Glimmer constructs (plain interpolations,
@args, and aneachblock) in a compact example. Looks good as an “ok” parsing case.crates/biome_html_formatter/src/js/auxiliary/path_segment.rs (1)
1-10: Verbatim formatting forGlimmerPathSegmentis a sensible defaultDelegating to
format_html_verbatim_nodekeeps path segments stable and matches the broader “verbatim Glimmer” approach in this PR. This is a clean, minimal formatter for now.crates/biome_js_syntax/src/parentheses/expression.rs (1)
66-66: LGTM! Glimmer templates don't need parentheses.The placement alongside
JsBogusExpressionand before type assertion expressions is appropriate, and returningfalseis consistent with primary expressions.crates/biome_js_syntax/src/expr_ext.rs (1)
916-917: LGTM! Primary precedence is appropriate.Treating
JsGlimmerTemplatewith the same precedence asJsxTagExpressionis sensible, as both are primary expression forms.crates/biome_html_formatter/src/js/auxiliary/positional_argument.rs (1)
1-14: LGTM! Standard verbatim formatter implementation.The verbatim formatting approach is appropriate for preserving Glimmer template content as documented in the changeset.
crates/biome_html_formatter/src/js/bogus/bogus_expression.rs (1)
1-5: LGTM! Standard bogus node boilerplate.The empty
FormatBogusNodeRuleimplementation is correct for error recovery nodes.crates/biome_service/src/file_handlers/mod.rs (1)
820-820: LGTM! Sensible unification with standard JS path.Routing
EmbeddingKind::Glimmerthrough the same capabilities asEmbeddingKind::Noneis appropriate, since Glimmer files are fundamentally JavaScript files with embedded templates.crates/biome_html_formatter/src/js/auxiliary/literal.rs (1)
1-10: LGTM! Consistent with other Glimmer formatters.The verbatim formatting approach maintains consistency across all Glimmer node types.
crates/biome_html_formatter/src/js/auxiliary/mustache_expression.rs (1)
1-14: LGTM! Consistent formatter implementation.The mustache expression formatter follows the same verbatim pattern as the other Glimmer formatters, maintaining consistency across the implementation.
crates/biome_js_syntax/src/file_source.rs (4)
126-127: LGTM!The Glimmer variant addition follows the established pattern for other embedding kinds.
142-144: LGTM!The helper method follows the existing pattern for embedding kind queries.
222-232: LGTM!The Glimmer file source constructors are well-documented and follow the established patterns for other embedding kinds.
363-365: The review concern is incorrect—Glimmer language ID support already exists.Glimmer language IDs (
"glimmer","glimmer-js","glimmer-ts") are correctly implemented incrates/biome_html_syntax/src/file_source.rs(line 151), not the JS syntax file. This is the right place since Glimmer is an HTML template format, not a JavaScript format. The Astro, Vue, and Svelte entries in the JS file source carry TODO comments indicating they're ad hoc and should eventually be removed.Likely an incorrect or invalid review comment.
crates/biome_html_formatter/src/js/auxiliary/block_params.rs (1)
1-10: LGTM!The verbatim formatting approach for Glimmer block parameters is appropriate and follows the pattern established for other Glimmer formatters.
crates/biome_html_formatter/src/js/auxiliary/string_literal.rs (1)
1-10: LGTM!The verbatim formatting for Glimmer string literals matches the pattern used for other Glimmer nodes.
crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_class.gjs (1)
1-17: LGTM!Good test coverage: a simple template-only class and a class combining a constructor with a template.
crates/biome_js_formatter/src/js/any/statement.rs (1)
24-24: LGTM!The JsGlimmerTemplate statement formatting follows the established pattern.
crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_default_export.gjs (1)
1-4: LGTM!This tests the important case of a standalone template at file level.
crates/biome_html_formatter/src/lib.rs (1)
22-22: LGTM!The internal
jsmodule addition appropriately houses the Glimmer formatting infrastructure.crates/biome_js_analyze/src/lint/complexity/no_static_only_class.rs (1)
165-165: LGTM!Correctly filters out Glimmer templates when analysing static-only classes, consistent with other non-member constructs.
crates/biome_js_formatter/src/js/any/expression.rs (1)
23-23: LGTM!The formatting delegation for
JsGlimmerTemplatefollows the established pattern consistently.crates/biome_html_formatter/src/html/any/attribute.rs (1)
11-12: LGTM!The Glimmer attribute variants are properly integrated and follow the existing formatting pattern.
crates/biome_js_syntax/src/union_ext.rs (1)
100-100: LGTM!Returning
Ok(None)forJsGlimmerTemplateis appropriate since template members don't have names, consistent with other unnamed class members.crates/biome_grit_patterns/src/grit_target_language/js_target_language/generated_mappings.rs (1)
301-301: LGTM!The GritQL mapping for
JsGlimmerTemplatecorrectly follows the established pattern for native Biome AST nodes.crates/biome_js_parser/tests/js_test_suite/error/glimmer_template_unclosed.gjs (1)
1-3: LGTM!Clear test case for an unclosed template error. The minimal example is spot-on for error testing.
crates/biome_js_analyze/src/lint/correctness/no_invalid_constructor_super.rs (1)
229-229: LGTM!Correctly identifies
JsGlimmerTemplateas an invalid constructor, grouped appropriately with other non-constructor expressions.xtask/codegen/src/js_kinds_src.rs (2)
179-179: LGTM!The
GLIMMER_TEMPLATEtoken addition is well-documented and correctly placed in the tokens list.
335-335: LGTM!The
JS_GLIMMER_TEMPLATEnode is properly positioned in the nodes list for code generation.crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_typescript.gts (1)
1-13: LGTM!Excellent test coverage for Glimmer templates in TypeScript, including both standalone templates and class member templates with mustache expressions.
crates/biome_html_parser/tests/spec_tests.rs (1)
9-9: LGTM! Test coverage properly extended.The addition of
.gjsand.gtsextensions to bothokanderrortest specs correctly expands test coverage for Glimmer template files.Also applies to: 13-13
crates/biome_html_parser/tests/html_specs/ok/glimmer/mustache.gjs (1)
1-6: LGTM! Good test coverage for mustache syntax.This test file appropriately covers key Glimmer mustache patterns: property access, arguments, helpers with parameters, and named parameters.
crates/biome_html_formatter/src/html/lists/attribute_list.rs (1)
72-77: LGTM! Formatting correctly delegated.The new Glimmer attribute variants follow the established pattern of delegating to their respective formatters, consistent with other attribute types in this module.
crates/biome_js_formatter/src/js/any/class_member.rs (1)
15-15: LGTM! Generated formatting delegation is correct.The new
JsGlimmerTemplateclass member variant correctly delegates formatting, consistent with other class member types.crates/biome_js_analyze/src/lint/style/use_naming_convention.rs (1)
1158-1158: LGTM! Appropriate exclusion from naming conventions.Glimmer templates don't have identifiers subject to naming conventions, so excluding them from this lint rule alongside other structural class members (constructors, static blocks) is correct.
crates/biome_html_formatter/src/html/any/content.rs (1)
11-11: LGTM! Generated content formatting is correct.The
AnyGlimmerExpressionvariant correctly delegates formatting, matching the pattern of other content types.crates/biome_js_parser/src/syntax/stmt.rs (1)
376-381: LGTM! Glimmer template parsing correctly integrated.The new
GLIMMER_TEMPLATEcase appropriately handles Glimmer templates as statements, following the established pattern for statement types. The placement before the expression statement handler ensures explicit recognition.crates/biome_html_formatter/src/html/any/element.rs (1)
16-17: LGTM! Generated element formatting is correct.The Glimmer element variants (
GlimmerMustacheCommentandGlimmerTripleStashExpression) correctly delegate formatting, consistent with other HTML element types.crates/biome_html_parser/tests/html_specs/ok/glimmer/block_helper.gjs (1)
1-11: LGTM - valid Glimmer template test fixture.The block helper syntax is correct, demonstrating conditional rendering, iteration with block parameters, and aliasing patterns.
crates/biome_js_analyze/src/lint/nursery/no_unused_expressions.rs (1)
262-262: LGTM - consistent with JSX handling.Treating Glimmer templates as disallowed expressions aligns with the existing JSX pattern. A standalone template expression isn't a meaningful statement.
crates/biome_html_parser/tests/html_specs/ok/glimmer/triple_stash.gjs (1)
1-5: LGTM - valid triple-stash test fixture.The test covers both standalone unescaped output and helper invocations with triple-stash syntax, providing good coverage for this Glimmer feature.
crates/biome_html_parser/src/parser.rs (2)
116-119: LGTM - consistent API pattern.The new method follows the existing
with_single_text_expressionandwith_double_text_expressionpattern, maintaining API consistency.
162-164: LGTM - clean integration.The Glimmer variant mapping to
DoubleGlimmerexpression mode is straightforward and aligns well with other variant handlers.crates/biome_html_parser/tests/html_specs/ok/glimmer/mustache_comment.html (1)
1-7: LGTM - comprehensive comment test fixture.Tests both single-line (
{{! ... }}) and multi-line ({{!-- ... --}}) Mustache comment syntax in a realistic HTML context.crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_simple.gjs (1)
1-5: LGTM - clean template expression test.Demonstrates templates as first-class expressions, covering both local and exported usage. Good foundation for testing the Glimmer-in-JS integration.
crates/biome_html_formatter/src/js/lists/mod.rs (1)
1-5: LGTM - properly generated file.This file is generated by
cargo codegen formatter. The module declarations follow the expected pattern for formatter list modules.crates/biome_js_parser/src/syntax/class.rs (1)
538-542: Implementation correctly handles class-level Glimmer templates.Verification confirms templates in class bodies are valid Glimmer syntax and used as shown in your implementation—no modifiers, self-contained members. The parser placement is sound. File-type validation (
.gjs/.gts) would belong in semantic analysis or linting, not here.crates/biome_html_formatter/src/js/bogus/mod.rs (1)
1-3: LGTM! Generated module structure is correct.The generated module file properly declares the
bogus_expressionsubmodule with appropriate crate visibility.crates/biome_html_formatter/src/js/any/mod.rs (1)
1-5: LGTM! Generated module structure is correct.The generated module file properly declares the three Glimmer-related submodules with appropriate crate visibility.
crates/biome_js_parser/src/parser.rs (1)
43-44: All callers updated—verification complete.Confirmed: only one call site exists (line 44 in
crates/biome_js_parser/src/parser.rs), and it already uses the new 3-argument signature withsource_type. No old-style calls remain.crates/biome_js_formatter/src/generated.rs (1)
2266-2303: Code wiring is spot on; test coverage is MIA.All three components are correctly wired:
- Module is exported (
pub(crate) mod glimmer_template;✓)FormatJsGlimmerTemplatewithDefaultimpl ✓JsGlimmerTemplatesyntax node exists and is used throughout the codebase ✓However, there are no
.gjs/.gtsfixture files or snapshot tests for glimmer template formatting. The original review asked for test confirmation—please verify whether test coverage will be added separately or if there's another mechanism at play.crates/biome_html_parser/tests/spec_test.rs (1)
42-48: Glimmer snapshot wiring looks consistentThe new
HtmlVariant::Glimmer => "glimmer"arm matches the existing pattern for other variants and should keep the spec snapshots readable. All good here.crates/biome_js_parser/src/syntax/expr.rs (1)
2175-2184: Glimmer template node parse looks clean
parse_glimmer_templateis minimal and matches the usual pattern: guard, start marker,bump, complete asJS_GLIMMER_TEMPLATE. Looks good and keeps Glimmer confined to a single opaque node for now.crates/biome_html_formatter/src/js/mod.rs (1)
1-6: Generated module wiring looks fineThe
jsformatter module fan‑out (any,auxiliary,bogus,lists) is consistent and crate‑private. Nothing to nitpick here—regenerate away.crates/biome_js_parser/src/token_source.rs (1)
28-42: First concern is unfounded—token_sourcemodule is private, not publicThe module is declared as
mod token_source;(notpub mod), soJsTokenSource::from_strisn't exposed in the public API and poses no breaking change risk for external callers.The stylistic suggestion about
JsLexContext::default()vsJsLexContext::Regularis technically sound—they're functionally equivalent (Regular is the default variant), but using the explicit variant would improve clarity. That's optional rather than critical.Likely an incorrect or invalid review comment.
crates/biome_html_formatter/src/js/auxiliary/named_argument.rs (1)
1-10: LGTM – clean verbatim formatter.The implementation correctly delegates to
format_html_verbatim_node, consistent with other Glimmer formatters in the PR.crates/biome_html_formatter/src/js/lists/argument_list.rs (1)
1-10: LGTM – standard list formatter.The
join().entries().finish()pattern is the correct approach for formatting argument lists.crates/biome_html_formatter/src/js/any/argument_value.rs (1)
1-17: LGTM – generated dispatcher.Standard variant dispatch pattern for the
AnyGlimmerArgumentValueenum.crates/biome_html_formatter/src/js/auxiliary/mustache_comment.rs (1)
1-10: LGTM – verbatim formatter for comments.Correctly preserves mustache comment formatting via
format_html_verbatim_node.crates/biome_html_formatter/src/js/auxiliary/named_block_opening.rs (1)
1-14: LGTM – verbatim formatter.Standard implementation for named block opening tags.
crates/biome_html_formatter/src/js/auxiliary/named_block_closing.rs (1)
1-14: LGTM – verbatim formatter.Standard implementation for named block closing tags.
crates/biome_html_formatter/src/js/lists/block_param_list.rs (1)
1-10: LGTM – standard list formatter.Consistent with the other list formatters in this PR.
crates/biome_html_formatter/src/js/lists/path_segment_list.rs (1)
1-8: LGTM – standard setup.Struct and trait implementation follow the expected pattern.
crates/biome_js_formatter/src/js/auxiliary/glimmer_template.rs (1)
1-16: LGTM!Correctly formats the JS-layer Glimmer template by delegating to the template token's formatter. The distinct approach (formatting fields rather than verbatim) is appropriate for the JavaScript AST layer.
crates/biome_html_formatter/src/js/any/argument.rs (1)
1-15: LGTM!Generated dispatcher correctly delegates to the appropriate formatter for each
AnyGlimmerArgumentvariant.crates/biome_html_formatter/src/js/auxiliary/block_param.rs (1)
1-10: No conflict exists; implementations are correctly layered.The
FormatNodeRuleinblock_param.rsis the single source of truth. Thegenerated.rsfile wraps it withFormatRule,AsFormat, andIntoFormattraits—all pointing to the sameFormatGlimmerBlockParam. The macro invocation inglimmer.rs:49appears unused (or shadowed by the generated implementations), which is fine provided the codebase compiles.The architecture is clean: core logic → trait wrappers → formatting pipeline. No precedence ambiguity.
crates/biome_html_formatter/src/glimmer.rs (1)
1-62: Running a quick check on what one of the auxiliary files actually contains:The two implementations serve complementary roles in Biome's architecture—not necessarily redundant.
The script confirms both exist: macro-generated
AsFormat/IntoFormat(in glimmer.rs) and explicitFormatNodeRule(in auxiliary/) for nearly all 20 shared node types. However, this isn't a conflict; Biome's formatting uses FormatRule as a fallback when the Format trait can't be implemented directly on external types due to Rust's orphan rule.The macro generates
FormatRefWithRuleandFormatOwnedWithRulewrappers—lightweight adapters that pair nodes with their corresponding rules. The auxiliary implementations are the actual rules these wrappers delegate to. They coexist by design.The architectural concern hinges on whether auxiliary implementations also do verbatim formatting (as the comment assumes) or perform proper formatting. Without verifying auxiliary file content, the true redundancy cannot be confirmed. If both implementations universally do verbatim formatting, consolidation may be warranted; if auxiliary files contain real formatting logic, the dual approach may be intentional scaffolding or a migration artifact.
Recommendation: Verify the behaviour of 2–3 auxiliary implementations (e.g.,
block_param.rs,path.rs) to confirm they also delegate to verbatim formatting, then reassess whether this duplication needs refactoring.crates/biome_html_formatter/src/js/any/expression.rs (1)
1-19: LGTM!Generated formatter correctly delegates all AnyGlimmerExpression variants to their respective format implementations.
xtask/codegen/js.ungram (1)
890-897: LGTM!Glimmer template grammar correctly added as a statement, expression, and class member variant. The single-token approach aligns well with opaque template handling.
crates/biome_html_formatter/src/js/auxiliary/subexpression.rs (1)
1-10: LGTM!Formatter follows the established pattern for Glimmer nodes, using verbatim HTML formatting to preserve template syntax.
crates/biome_html_formatter/src/js/auxiliary/triple_stash_expression.rs (1)
1-14: LGTM!Consistent verbatim formatting for triple-stash expressions.
crates/biome_html_formatter/src/js/auxiliary/block_helper_opening.rs (1)
1-14: LGTM!Block helper opening formatter follows the established pattern.
crates/biome_html_syntax/src/file_source.rs (2)
34-112: LGTM!Glimmer variant properly integrated with file source detection, including constructor and predicate methods following existing patterns.
174-213: Excellent test coverage!Tests verify variant behaviour, extension mapping, and language ID detection. Nice work.
crates/biome_html_formatter/src/js/auxiliary/splattribute.rs (1)
1-10: LGTM!Splattribute formatter maintains consistency with the verbatim pattern.
crates/biome_html_parser/src/syntax/mod.rs (3)
220-220: LGTM!Making ElementList pub(crate) is appropriate for internal usage with ParseNodeList.
233-248: Well-structured parsing cascade.The or_else chain gracefully tries Glimmer-specific parsing before falling back to generic text expressions. Nice approach for backward compatibility.
619-629: LGTM!Mustache comment parsing is straightforward and correct.
crates/biome_html_formatter/src/js/auxiliary/mod.rs (1)
1-22: Generated auxiliary module wiring looks consistentModule list matches the Glimmer auxiliary formatter surface (block helpers, paths, arguments, modifiers, literals, etc.) and is crate‑private. Nothing to flag here.
crates/biome_html_formatter/src/generated.rs (4)
85-836: Glimmer node formatter wiring looks consistentAll
FormatRule/AsFormat/IntoFormatimpls for the variousGlimmer*nodes mirror the existing Astro/Svelte/HTML patterns, useHtmlFormatContext, and point at the expectedcrate::js::auxiliary::*::FormatGlimmer*types. Nothing obviously off-by-one or mis‑routed here; codegen seems to have earned its keep.
1543-1616: List integrations for Glimmer are aligned with existing list patterns
GlimmerArgumentList,GlimmerBlockParamList, andGlimmerPathSegmentListonly provideAsFormat/IntoFormatand use thecrate::js::lists::*formatters, matching how HTML/Svelte list types are wired just below. Looks good and uniform.
1731-1768: Bogus Glimmer expression formatting hooked up correctly
GlimmerBogusExpressionusesFormatBogusNodeRuleand thecrate::js::bogus::bogus_expression::FormatGlimmerBogusExpressionformatter, consistent with the other bogus-node implementations in this file. No concerns.
1972-2045: AnyGlimmer aggregators correctly routed*
AnyGlimmerArgument,AnyGlimmerArgumentValue, andAnyGlimmerExpressionare wired viaAsFormat/IntoFormatto thecrate::js::any::*::FormatAnyGlimmer*formatters, parallel toAnyAstroFrontmatterElementand the variousAnyHtml*/AnySvelte*types. Nice tidy extension of the public surface.
The lexer was incorrectly returning DOCTYPE_KW for the identifiers "doctype" and "DOCTYPE" in Glimmer context. This prevented valid Glimmer helpers named "doctype" from being parsed correctly. Changed the check from `!context.is_svelte()` to `context.is_doctype()` so the keyword is only recognized in actual DOCTYPE declarations. Test: Added doctype_identifier.gjs test case 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The parser was accepting bare `@` symbols without any following identifier, which is invalid Glimmer syntax. Now emits a clear diagnostic: "Expected identifier after '@'". Test: Added empty_path_after_at.gjs error test case 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The parser was treating any identifier followed by '|' as the start of
block params, which could incorrectly stop parsing arguments. Now it
specifically checks that the identifier is 'as' before treating it as
block params.
This prevents cases like `{{helper foo |bar}}` from being misinterpreted
as having block params when 'foo' is just a regular argument.
Test: Added block_params_with_as.gjs test case
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
The lexer now emits a clear error message when a mustache comment reaches EOF without finding its closing delimiter (}} or --}}). This helps users quickly identify unclosed comments in their templates. Test: Added unterminated_comment.html error test case 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The parser now emits an error when a block helper is missing its
closing tag (e.g., {{/if}}). This helps users quickly identify
unclosed block helpers in their Glimmer templates.
Test: Added missing_block_closing.gjs error test case
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
When a triple-stash expression is missing its closing }}}, the parser now attempts to resynchronize by consuming tokens until it finds }}} or reaches a safe boundary (EOF or <). This prevents the error from cascading and affecting subsequent parsing. Test: Added missing_triple_closing.html error test case 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Changed separator access from field reference (&element.trailing_separator) to method call (element.trailing_separator()) for consistency with other formatters throughout the codebase. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/biome_js_analyze/src/lint/complexity/no_static_only_class.rs (1)
159-187: Fix false positive: only flag "static-only" when there is actually at least one counted member.The current code uses
filter_map(...).all(|is_static| is_static), which returns true on an empty iterator. This means a class containing only ignored members (templates, empty/bogus members) gets incorrectly flagged. The proposed fix collects statuses and enforces!statuses.is_empty()before checking all.- let all_members_static = class_declaration - .members() - .iter() - .filter_map(|member| match member { + let statuses: Vec<bool> = class_declaration + .members() + .iter() + .filter_map(|member| match member { AnyJsClassMember::JsBogusMember(_) | AnyJsClassMember::JsMetavariable(_) | AnyJsClassMember::JsGlimmerTemplate(_) | AnyJsClassMember::JsEmptyClassMember(_) => None, AnyJsClassMember::JsConstructorClassMember(_) => Some(false), // See GH#4482: Constructors are not regarded as static AnyJsClassMember::TsConstructorSignatureClassMember(_) => Some(false), // See GH#4482: Constructors are not regarded as static AnyJsClassMember::JsGetterClassMember(m) => Some(m.has_static_modifier()), AnyJsClassMember::JsMethodClassMember(m) => Some(m.has_static_modifier()), AnyJsClassMember::JsPropertyClassMember(m) => Some(m.has_static_modifier()), AnyJsClassMember::JsSetterClassMember(m) => Some(m.has_static_modifier()), AnyJsClassMember::JsStaticInitializationBlockClassMember(_) => Some(true), // See GH#4482: Static initialization blocks are regarded as static AnyJsClassMember::TsGetterSignatureClassMember(m) => Some(m.has_static_modifier()), AnyJsClassMember::TsIndexSignatureClassMember(m) => Some(m.has_static_modifier()), AnyJsClassMember::TsInitializedPropertySignatureClassMember(m) => { Some(m.has_static_modifier()) } AnyJsClassMember::TsMethodSignatureClassMember(m) => Some(m.has_static_modifier()), AnyJsClassMember::TsPropertySignatureClassMember(m) => { Some(m.has_static_modifier()) } AnyJsClassMember::TsSetterSignatureClassMember(m) => Some(m.has_static_modifier()), }) - .all(|is_static| is_static); + .collect(); + + let all_members_static = !statuses.is_empty() && statuses.into_iter().all(|is_static| is_static); - if all_members_static { Some(()) } else { None } + if all_members_static { Some(()) } else { None }crates/biome_grit_patterns/src/grit_js_parser.rs (1)
28-35: Add .gjs/.gts handling to parse_with_path for correct Glimmer file parsing.Currently Glimmer files default to TypeScript. Both
JsFileSource::gjs()andJsFileSource::gts()constructors are available and tested in the codebase—add explicit arms to handle these extensions:let source_type = match path.extension() { Some("d.ts") => JsFileSource::d_ts(), + Some("gjs") => JsFileSource::gjs(), + Some("gts") => JsFileSource::gts(), Some("js") => JsFileSource::js_module(), Some("jsx") => JsFileSource::jsx(), Some("tsx") => JsFileSource::tsx(), _ => JsFileSource::ts(), };
♻️ Duplicate comments (1)
crates/biome_html_parser/src/lexer/mod.rs (1)
703-751: Mustache comment lexing and diagnostics look goodThe new
consume_mustache_commentcleanly distinguishes{{! ... }}and{{!-- ... --}}, stops at the right terminators, and now reports an “unterminated mustache comment” diagnostic on EOF. That should make common comment typos much easier to track down without complicating parsing.
🧹 Nitpick comments (19)
crates/biome_html_parser/tests/html_specs/ok/glimmer/doctype_identifier.gjs (1)
1-4: Good casing check; consider one more edge case.Nice fixture to prove
{{doctype}}vs{{DOCTYPE}}are identifiers, not an HTML DOCTYPE. For completeness, consider adding:
- A raw
<!DOCTYPE html>inside<template>to lock in intended behaviour.- A triple‑stash variant (
{{{doctype}}}) to ensure tokenizer parity.- A formatter snapshot that asserts casing is preserved.
crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_typescript.gts (1)
1-13: Nice TS fixture coverage.Good smoke test for class‑scoped templates and typed factories. Consider a follow‑up adding a named block or block params in TS to widen coverage.
crates/biome_html_syntax/src/file_source.rs (1)
174-213: Nice targeted Glimmer file source testsThe new tests cover the constructor, extension mapping, and language‑id mapping well and guard against regressions in the variant wiring. If you ever fancy expanding this, a small
TryFrom<&Utf8Path>case using a.gjspath would exercise the well‑known path too, but this is strictly optional.crates/biome_html_parser/tests/spec_test.rs (1)
143-150: Tidy up the quick Glimmer parse options call (optional)
parse_html(code, (&HtmlFileSource::glimmer()).into())works but is a bit noisy. Consider matching the style above with something likeparse_html(code, HtmlParseOptions::from(&HtmlFileSource::glimmer()))for slightly clearer intent.crates/biome_html_formatter/src/js/auxiliary/string_literal.rs (1)
1-10: String literal formatter fits the Glimmer pattern; minor DRY opportunityThis verbatim formatter is correct and matches the other Glimmer helpers. Given several
FormatGlimmer*types now share the same body, you might eventually factor them through a tiny helper or macro, but that is pure nicety.crates/biome_service/src/file_handlers/javascript.rs (1)
539-548: Dropping Settings-derived parser options – intentional?
parsenow ignoresSettings/BiomePathand callsparse_js_with_cachewith onlytextandJsFileSource. That matches the “derive options from source type” direction, but it also meansJsParserSettings/resolve_parse_optionsno longer influence this parse path.If that’s intentional, it might be worth:
- adding a brief comment here to make the behaviour explicit, and
- pruning any now-dead uses of
ParserOptionsin the service layer in a follow-up.If it wasn’t intentional, this is where config-driven flags (e.g. decorators, grit metavariables) stopped taking effect and should be threaded back in.
crates/biome_js_parser/tests/spec_test.rs (1)
9-10: Nice separation between default and options-driven parser pathsThe
options.jsonhook now flows cleanly intoparse_with_options, with the fallback to plainparsefor normal fixtures. That keeps tests expressive without complicating the parser API.If you ever feel like micro‑tidying, you could stash
let has_options = options_path.exists();and reuse it for both the options-loading block and the parser selection, but it’s hardly worth losing sleep over.Also applies to: 42-73, 86-91, 194-195
crates/biome_html_formatter/src/js/lists/block_param_list.rs (1)
7-9: Ensure space separation between block params.Explicitly join with a space to avoid “ab” instead of “a b” when formatting multiple params.
Apply:
- f.join().entries(node.iter().formatted()).finish() + f.join_with(space()) + .entries(node.iter().formatted()) + .finish()Please add/confirm a spec like:
{{#each items as |item index|}}to assert single-space separation..changeset/glimmer-template-support.md (1)
13-20: Update changeset to reflect full Glimmer support (no longer “opaque only”).The text still describes Phase 1, but this PR parses and formats Glimmer constructs.
Apply:
-**Phase 1 Implementation Notes:** -- Template content is treated as **opaque tokens** - the content is preserved exactly as written without internal parsing or linting -- The template syntax itself is validated (e.g., checking for unclosed tags) -- Templates work with whitespace in the opening tag (e.g., `<template >`, `<template\n>`) -- LSP language IDs "gjs" and "gts" are now recognized -- Future phases will add internal template parsing and linting support +**Glimmer support:** +- Full parsing for mustache expressions, triple‑stash, mustache comments, block helpers, named blocks and block params, element modifiers, splattributes, and subexpressions +- Formatting preserves semantics and normalises whitespace where appropriate +- Templates work with whitespace in the opening tag (e.g., `<template >`, `<template\n>`) +- LSP language IDs "gjs" and "gts" are recognised -The template content is preserved as-is during formatting, and the parser provides diagnostics for unclosed template tags. +The formatter preserves template content while applying Glimmer-aware spacing/line‑break rules; the parser provides diagnostics for malformed Glimmer/HTML constructs.crates/biome_js_parser/src/parser.rs (1)
43-58: Deduplicate constructor logic: have new() delegate to with_options().Keeps init in one place and avoids drift.
Apply:
pub fn new(source: &'source str, source_type: JsFileSource) -> Self { - let options = JsParserOptions::from(&source_type); - let source = JsTokenSource::from_str(source, source_type); - - JsParser { - state: JsParserState::new(&source_type), - source_type, - context: ParserContext::default(), - source, - options, - } + Self::with_options(source, source_type, JsParserOptions::from(&source_type)) }Also applies to: 59-77
crates/biome_js_syntax/src/file_source.rs (1)
364-366: Extension and language‑id mapping complete.Adds “gjs”/“gts” in both places; tests below cover it. Consider also accepting a generic “glimmer” id later if editors emit it.
Also applies to: 399-401
crates/biome_js_parser/src/token_source.rs (1)
28-48: Publicfrom_strAPI now depends onJsFileSource– check external callersDeriving
JsParserOptionsfromJsFileSourceand wiring.with_source_type(source_type)is a nice simplification and keeps lexer/parser configuration in sync.The one thing to watch is that
from_strispuband its signature changed from takingJsParserOptionstoJsFileSource. If any downstream crates constructJsTokenSourcedirectly, this is a breaking change.If you care about external consumers, you might consider a compatibility constructor (e.g.
from_str_with_optionsthat wraps this) or explicitly documenting the break.crates/biome_js_parser/src/lexer/mod.rs (1)
1741-1825: Glimmer<template>lexing is mostly solid; consider tiny tidy-upsThe overall flow in
try_lex_glimmer_template+resolve_less_thanlooks correct:
- Only runs when
source_type.as_embedding_kind().is_glimmer(), so you don’t accidentally eat<template>in normal JS.- Uses lookahead-only checks before committing, so falling back to normal
<lexing is safe.- Once committed, consumes the full
<template ...> ... </template>block into a singleGLIMMER_TEMPLATEtoken and emits a diagnostic for missing closing tags, which is friendly for error recovery.A couple of small nits you might want to tighten:
- The
9inbyte_at(9)is a bit magic now that you’ve switched totemplate_bytes = b"template". Using1 + template_bytes.len()would keep the code self-documenting and safer against future tweaks to the tag string.- By design,
<template/>will not be treated as a Glimmer template (the first char aftertemplatemust be>or whitespace). If that’s intentional for Glimmer, all good; if not, you may want to extend the follow-up check to handle the self-closing case too.Functionally this is in a good place; the above are purely polish.
Also applies to: 1845-1851
crates/biome_html_parser/src/syntax/mod.rs (1)
579-612: Triple/DoubleGlimmer branch relies entirely on lexer tokenisationRouting
TextExpressionKind::Triple | DoubleGlimmerthroughbump_any_with_contextassumes the lexer already emits fully‑typed Glimmer tokens for these contexts, which matches the new lexer paths. That’s fine, but it does mean all shape/validation has to live in the Glimmer parsers; there’s no remapping safety net here. Worth keeping in mind if you later add more token kinds for Glimmer expressions.crates/biome_html_parser/src/syntax/glimmer/mod.rs (3)
19-60: Path parsing covers common Glimmer forms with reasonable error reportingSupporting
AT,IDENT, andHTML_LITERALplus dotted segments handlesthis.foo,@arg, and helper names nicely. The “@with no following ident” diagnostic is also welcome. If you ever want stricter recovery, you might consider emitting an error when you see.with no following segment as well, but that’s sugar rather than a blocker.
69-134: Argument list correctly distinguishes named/positional and block params sentinelThe checkpoint‑based lookahead for
as |...|andname=is a tidy way to keep block params, named, and positional arguments sharing a single loop. The only subtlety is that any “unknown” token (e.g. unexpected punctuation) silently terminates the argument list; that’s probably fine for a first cut, but if you later want sharper diagnostics you could emit a small error before breaking.
179-251: Block helper parsing looks solid, but closing tags aren’t structurally checkedThe block helper detection (
{{+#under DoubleGlimmer) and the child loop usingElementListare both nicely done – it should handle nesting and mixed HTML/Glimmer content cleanly. You now also emit a dedicated diagnostic when the closing{{/...}}is missing, which addresses the earlier review comment.One limitation to note: there’s no check that the closing helper path matches the opening helper (unlike element names in
parse_element). If you’d like stricter structural validation, a future enhancement could compare those paths and either error or produce a bogus node when they differ; for now, a lint can also handle it.crates/biome_js_parser/src/parse.rs (2)
185-239: Updated examples and script/module wrappers look consistentThe rewritten docs and the new
parse_script/parse_modulewrappers that delegate toparsewith the appropriateJsFileSourcekeep the high‑level entry points straightforward. Just be aware that this is a breaking change for any users still passingJsParserOptionsdirectly; worth flagging prominently in the release notes if you haven’t already.
327-337: Cache and offset paths now share the same core machinery
parse_js_with_cacheusingparse_common, and the offset variants usingparse_common_with_options, keeps the code paths nicely aligned while still letting embedded callers pass explicit options. The extraJsOffsetParse::new(parse.root, parse.diagnostics)in the script/module offset helpers is a tiny duplication but harmless – you could return the inner parse directly if you feel like a small tidy‑up later.Also applies to: 436-449
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (37)
crates/biome_html_factory/src/generated/node_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_html_factory/src/generated/syntax_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_html_parser/tests/html_specs/error/glimmer/empty_path_after_at.gjs.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/error/glimmer/missing_block_closing.gjs.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/error/glimmer/missing_triple_closing.html.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/error/glimmer/unterminated_comment.html.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/ok/glimmer/block_helper.gjs.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/ok/glimmer/block_params_with_as.gjs.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/ok/glimmer/doctype_identifier.gjs.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/ok/glimmer/mustache.gjs.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/ok/glimmer/mustache_comment.html.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/ok/glimmer/triple_stash.gjs.snapis excluded by!**/*.snapand included by**crates/biome_html_syntax/src/generated/kind.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_html_syntax/src/generated/macros.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_html_syntax/src/generated/nodes.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_html_syntax/src/generated/nodes_mut.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_js_factory/src/generated/node_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_js_factory/src/generated/syntax_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_incomplete_less_than.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_incomplete_templat.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_lookalike_div.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_lookalike_temp.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_lookalike_templatex.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_template_self_closing.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_template_unclosed.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_class.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_default_export.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_simple.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_typescript.gts.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_whitespace.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_with_content.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_yield_await.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_syntax/src/generated/kind.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_js_syntax/src/generated/macros.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_js_syntax/src/generated/nodes.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_js_syntax/src/generated/nodes_mut.rsis excluded by!**/generated/**,!**/generated/**and included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**
📒 Files selected for processing (107)
.changeset/glimmer-template-support.md(1 hunks)crates/biome_grit_patterns/src/grit_js_parser.rs(4 hunks)crates/biome_grit_patterns/src/grit_target_language/js_target_language/generated_mappings.rs(1 hunks)crates/biome_html_formatter/src/generated.rs(4 hunks)crates/biome_html_formatter/src/glimmer.rs(1 hunks)crates/biome_html_formatter/src/html/any/attribute.rs(1 hunks)crates/biome_html_formatter/src/html/any/content.rs(1 hunks)crates/biome_html_formatter/src/html/any/element.rs(1 hunks)crates/biome_html_formatter/src/html/lists/attribute_list.rs(1 hunks)crates/biome_html_formatter/src/js/any/argument.rs(1 hunks)crates/biome_html_formatter/src/js/any/argument_value.rs(1 hunks)crates/biome_html_formatter/src/js/any/expression.rs(1 hunks)crates/biome_html_formatter/src/js/any/mod.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/block_helper.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/block_helper_closing.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/block_helper_opening.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/block_param.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/block_params.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/element_modifier.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/literal.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/mod.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/mustache_comment.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/mustache_expression.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/named_argument.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/named_block.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/named_block_closing.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/named_block_opening.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/path.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/path_segment.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/positional_argument.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/splattribute.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/string_literal.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/subexpression.rs(1 hunks)crates/biome_html_formatter/src/js/auxiliary/triple_stash_expression.rs(1 hunks)crates/biome_html_formatter/src/js/bogus/bogus_expression.rs(1 hunks)crates/biome_html_formatter/src/js/bogus/mod.rs(1 hunks)crates/biome_html_formatter/src/js/lists/argument_list.rs(1 hunks)crates/biome_html_formatter/src/js/lists/block_param_list.rs(1 hunks)crates/biome_html_formatter/src/js/lists/mod.rs(1 hunks)crates/biome_html_formatter/src/js/lists/path_segment_list.rs(1 hunks)crates/biome_html_formatter/src/js/mod.rs(1 hunks)crates/biome_html_formatter/src/lib.rs(1 hunks)crates/biome_html_parser/src/lexer/mod.rs(14 hunks)crates/biome_html_parser/src/parser.rs(2 hunks)crates/biome_html_parser/src/syntax/glimmer/mod.rs(1 hunks)crates/biome_html_parser/src/syntax/mod.rs(5 hunks)crates/biome_html_parser/src/token_source.rs(1 hunks)crates/biome_html_parser/tests/html_specs/error/glimmer/empty_path_after_at.gjs(1 hunks)crates/biome_html_parser/tests/html_specs/error/glimmer/missing_block_closing.gjs(1 hunks)crates/biome_html_parser/tests/html_specs/error/glimmer/missing_triple_closing.html(1 hunks)crates/biome_html_parser/tests/html_specs/error/glimmer/unterminated_comment.html(1 hunks)crates/biome_html_parser/tests/html_specs/ok/glimmer/block_helper.gjs(1 hunks)crates/biome_html_parser/tests/html_specs/ok/glimmer/block_params_with_as.gjs(1 hunks)crates/biome_html_parser/tests/html_specs/ok/glimmer/doctype_identifier.gjs(1 hunks)crates/biome_html_parser/tests/html_specs/ok/glimmer/mustache.gjs(1 hunks)crates/biome_html_parser/tests/html_specs/ok/glimmer/mustache_comment.html(1 hunks)crates/biome_html_parser/tests/html_specs/ok/glimmer/triple_stash.gjs(1 hunks)crates/biome_html_parser/tests/spec_test.rs(2 hunks)crates/biome_html_parser/tests/spec_tests.rs(1 hunks)crates/biome_html_syntax/src/element_ext.rs(3 hunks)crates/biome_html_syntax/src/file_source.rs(5 hunks)crates/biome_js_analyze/src/lint/complexity/no_static_only_class.rs(1 hunks)crates/biome_js_analyze/src/lint/correctness/no_invalid_constructor_super.rs(1 hunks)crates/biome_js_analyze/src/lint/nursery/no_unused_expressions.rs(1 hunks)crates/biome_js_analyze/src/lint/style/use_naming_convention.rs(1 hunks)crates/biome_js_formatter/src/generated.rs(1 hunks)crates/biome_js_formatter/src/js/any/class_member.rs(1 hunks)crates/biome_js_formatter/src/js/any/expression.rs(1 hunks)crates/biome_js_formatter/src/js/any/statement.rs(1 hunks)crates/biome_js_formatter/src/js/auxiliary/glimmer_template.rs(1 hunks)crates/biome_js_formatter/src/js/auxiliary/mod.rs(1 hunks)crates/biome_js_formatter/src/js/classes/property_class_member.rs(2 hunks)crates/biome_js_formatter/src/utils/format_node_without_comments.rs(1 hunks)crates/biome_js_parser/src/lexer/mod.rs(4 hunks)crates/biome_js_parser/src/options.rs(2 hunks)crates/biome_js_parser/src/parse.rs(8 hunks)crates/biome_js_parser/src/parser.rs(4 hunks)crates/biome_js_parser/src/syntax/class.rs(1 hunks)crates/biome_js_parser/src/syntax/expr.rs(3 hunks)crates/biome_js_parser/src/syntax/stmt.rs(1 hunks)crates/biome_js_parser/src/token_source.rs(1 hunks)crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_incomplete_less_than.gjs(1 hunks)crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_incomplete_templat.gjs(1 hunks)crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_lookalike_div.gjs(1 hunks)crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_lookalike_temp.gjs(1 hunks)crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_lookalike_templatex.gjs(1 hunks)crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_template_self_closing.gjs(1 hunks)crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_template_unclosed.gjs(1 hunks)crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_class.gjs(1 hunks)crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_default_export.gjs(1 hunks)crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_simple.gjs(1 hunks)crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_typescript.gts(1 hunks)crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_whitespace.gjs(1 hunks)crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_with_content.gjs(1 hunks)crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_yield_await.gjs(1 hunks)crates/biome_js_parser/tests/spec_test.rs(3 hunks)crates/biome_js_parser/tests/spec_tests.rs(1 hunks)crates/biome_js_syntax/src/expr_ext.rs(1 hunks)crates/biome_js_syntax/src/file_source.rs(6 hunks)crates/biome_js_syntax/src/parentheses/expression.rs(1 hunks)crates/biome_js_syntax/src/union_ext.rs(1 hunks)crates/biome_service/src/file_handlers/astro.rs(1 hunks)crates/biome_service/src/file_handlers/javascript.rs(1 hunks)crates/biome_service/src/file_handlers/mod.rs(2 hunks)crates/biome_service/src/file_handlers/svelte.rs(2 hunks)crates/biome_service/src/file_handlers/vue.rs(2 hunks)crates/biome_test_utils/src/lib.rs(2 hunks)
⛔ Files not processed due to max files limit (10)
- xtask/codegen/html.ungram
- xtask/codegen/js.ungram
- xtask/codegen/src/html_kinds_src.rs
- xtask/codegen/src/js_kinds_src.rs
- xtask/coverage/src/js/test262.rs
- xtask/coverage/src/jsx/jsx_babel.rs
- xtask/coverage/src/runner.rs
- xtask/coverage/src/symbols/msts.rs
- xtask/coverage/src/ts/ts_babel.rs
- xtask/rules_check/src/lib.rs
✅ Files skipped from review due to trivial changes (3)
- crates/biome_html_parser/tests/html_specs/error/glimmer/empty_path_after_at.gjs
- crates/biome_html_parser/tests/html_specs/error/glimmer/missing_triple_closing.html
- crates/biome_html_parser/tests/html_specs/ok/glimmer/block_params_with_as.gjs
🚧 Files skipped from review as they are similar to previous changes (39)
- crates/biome_html_formatter/src/js/auxiliary/path_segment.rs
- crates/biome_js_syntax/src/parentheses/expression.rs
- crates/biome_js_formatter/src/js/any/statement.rs
- crates/biome_js_parser/tests/spec_tests.rs
- crates/biome_html_formatter/src/js/auxiliary/splattribute.rs
- crates/biome_js_analyze/src/lint/style/use_naming_convention.rs
- crates/biome_html_formatter/src/html/any/attribute.rs
- crates/biome_html_formatter/src/js/any/mod.rs
- crates/biome_html_formatter/src/js/auxiliary/element_modifier.rs
- crates/biome_html_parser/tests/html_specs/ok/glimmer/triple_stash.gjs
- crates/biome_html_formatter/src/js/auxiliary/named_block_closing.rs
- crates/biome_html_formatter/src/js/auxiliary/literal.rs
- crates/biome_html_formatter/src/js/mod.rs
- crates/biome_html_formatter/src/js/auxiliary/block_param.rs
- crates/biome_html_formatter/src/js/any/argument.rs
- crates/biome_html_formatter/src/js/lists/path_segment_list.rs
- crates/biome_html_formatter/src/js/any/expression.rs
- crates/biome_html_formatter/src/html/lists/attribute_list.rs
- crates/biome_html_formatter/src/html/any/content.rs
- crates/biome_html_formatter/src/js/auxiliary/named_block_opening.rs
- crates/biome_html_formatter/src/js/auxiliary/positional_argument.rs
- crates/biome_html_parser/tests/html_specs/ok/glimmer/mustache.gjs
- crates/biome_html_formatter/src/js/auxiliary/named_block.rs
- crates/biome_html_formatter/src/js/lists/mod.rs
- crates/biome_js_analyze/src/lint/correctness/no_invalid_constructor_super.rs
- crates/biome_html_formatter/src/js/auxiliary/block_helper_opening.rs
- crates/biome_js_formatter/src/js/classes/property_class_member.rs
- crates/biome_html_parser/tests/spec_tests.rs
- crates/biome_js_formatter/src/generated.rs
- crates/biome_js_formatter/src/js/any/class_member.rs
- crates/biome_js_syntax/src/expr_ext.rs
- crates/biome_html_formatter/src/js/auxiliary/mod.rs
- crates/biome_js_parser/src/syntax/class.rs
- crates/biome_html_formatter/src/lib.rs
- crates/biome_js_formatter/src/js/auxiliary/mod.rs
- crates/biome_html_formatter/src/js/auxiliary/mustache_expression.rs
- crates/biome_js_parser/src/syntax/stmt.rs
- crates/biome_html_formatter/src/js/auxiliary/named_argument.rs
- crates/biome_html_parser/tests/html_specs/ok/glimmer/mustache_comment.html
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-11-09T12:47:46.298Z
Learnt from: ematipico
Repo: biomejs/biome PR: 8031
File: crates/biome_html_parser/src/syntax/svelte.rs:140-147
Timestamp: 2025-11-09T12:47:46.298Z
Learning: In the Biome HTML parser, `expect` and `expect_with_context` consume the current token and then lex the next token. The context parameter in `expect_with_context` controls how the next token (after the consumed one) is lexed, not the current token being consumed. For example, in Svelte parsing, after `bump_with_context(T!["{:"], HtmlLexContext::Svelte)`, the next token is already lexed in the Svelte context, so `expect(T![else])` is sufficient unless the token after `else` also needs to be lexed in a specific context.
Applied to files:
crates/biome_js_parser/src/lexer/mod.rscrates/biome_html_parser/src/syntax/mod.rscrates/biome_html_parser/src/lexer/mod.rs
📚 Learning: 2025-10-25T07:22:18.540Z
Learnt from: ematipico
Repo: biomejs/biome PR: 7852
File: crates/biome_css_parser/src/syntax/property/mod.rs:161-168
Timestamp: 2025-10-25T07:22:18.540Z
Learning: In the Biome CSS parser, lexer token emission should not be gated behind parser options like `is_tailwind_directives_enabled()`. The lexer must emit correct tokens regardless of parser options to enable accurate diagnostics and error messages when the syntax is used incorrectly.
Applied to files:
crates/biome_js_parser/src/lexer/mod.rscrates/biome_html_parser/src/syntax/mod.rscrates/biome_html_parser/src/lexer/mod.rs
📚 Learning: 2025-10-03T12:28:56.788Z
Learnt from: ematipico
Repo: biomejs/biome PR: 7670
File: crates/biome_service/src/file_handlers/html.rs:744-748
Timestamp: 2025-10-03T12:28:56.788Z
Learning: In Biome's codebase, when creating tokens with new text content, use the factory pattern with functions like `ident(text)` from the respective `*_factory` crates (e.g., `biome_html_factory::make::ident`). There is no `.with_text()` method on tokens. The `ident()` function creates a new detached token with the IDENT kind, which is the standard approach for token construction.
Applied to files:
crates/biome_html_parser/src/token_source.rscrates/biome_html_parser/src/lexer/mod.rs
📚 Learning: 2025-08-20T16:24:59.781Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7266
File: crates/biome_js_type_info/src/type.rs:94-102
Timestamp: 2025-08-20T16:24:59.781Z
Learning: In crates/biome_js_type_info/src/type.rs, the flattened_union_variants() method returns TypeReference instances that already have the correct module IDs applied to them. These references should be used directly with resolver.resolve_reference() without applying additional module ID transformations, as variant references may originate from nested unions in different modules.
Applied to files:
crates/biome_js_syntax/src/union_ext.rs
🔇 Additional comments (57)
crates/biome_html_parser/tests/html_specs/error/glimmer/unterminated_comment.html (1)
1-9: Test fixture structure looks sound.The fixture properly covers both inline (
{{!) and block ({{!--) unterminated comment error cases. Each test case is appropriately structured with nested HTML elements to exercise the parser's error recovery in realistic contexts.crates/biome_html_parser/tests/html_specs/error/glimmer/missing_block_closing.gjs (1)
1-4: Test fixture is properly configured.The snapshot file exists, confirming the error case is fully integrated with its expected parser output. All set.
crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_lookalike_templatex.gjs (1)
1-3: Snapshot file confirmed—no issues found.The companion snapshot file exists at the expected location (
glimmer_lookalike_templatex.gjs.snap), so the error test is properly configured with expected output validation.crates/biome_js_analyze/src/lint/nursery/no_unused_expressions.rs (1)
252-266: LGTM — Glimmer templates should be invalid in statement position.Good parity with JSX handling. Please add parser/lint fixtures for a bare template in .gjs/.gts to lock this in.
You can verify coverage by adding an
expect_diagnosticcase to the existingno_unused_expressionstests under the Glimmer fixtures.crates/biome_grit_patterns/src/grit_target_language/js_target_language/generated_mappings.rs (1)
301-301: All checks pass — no issues found.The
JsGlimmerTemplatesymbol exists across the codebase, it's not behind feature gates, and the single mapping entry at line 301 follows the exact pattern of neighbouring entries with correct alphabetical ordering.crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_class.gjs (1)
1-17: LGTM! Good test coverage for templates as class members.The test demonstrates both standalone template members and templates alongside traditional class members.
crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_default_export.gjs (1)
1-4: LGTM! Clean test for default export behaviour.crates/biome_js_syntax/src/union_ext.rs (1)
100-100: LGTM! Correct handling for unnamed member.Returning
Ok(None)forJsGlimmerTemplateis consistent with other unnamed class members.crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_simple.gjs (1)
1-5: LGTM! Covers both local and exported template scenarios.crates/biome_html_parser/tests/html_specs/ok/glimmer/block_helper.gjs (1)
1-11: LGTM! Good coverage of Glimmer block helpers.Tests conditional rendering, iteration with parameters, and scoped aliasing.
crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_with_content.gjs (1)
1-10: LGTM! Comprehensive test mixing HTML and Glimmer constructs.Covers interpolations (
{{name}}), component arguments ({{@description}}), and block helpers.crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_template_self_closing.gjs (1)
1-3: LGTM! Valid error test case.Correctly tests that self-closing template tags are rejected, as Glimmer requires explicit closing tags.
crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_whitespace.gjs (1)
1-13: LGTM! Excellent edge case coverage for whitespace handling.Tests space, tab, newline, and multiple spaces after the
templatekeyword, ensuring the parser is robust to formatting variations.crates/biome_html_formatter/src/js/bogus/bogus_expression.rs (1)
1-5: Bogus wiring looks good.Matches the existing pattern for bogus nodes; nothing to add.
crates/biome_html_formatter/src/js/bogus/mod.rs (1)
1-3: Generated module is fine.Module surfaced correctly; re‑gen guidance present. All good.
crates/biome_html_formatter/src/js/any/argument_value.rs (1)
7-16: Variant delegation LGTM.Straight‑through to inner formatters; future variants will fail to compile, which is what we want.
crates/biome_html_formatter/src/js/auxiliary/block_helper.rs (1)
1-10: Verbatim Glimmer block helper formatter looks soundThis follows the existing Glimmer verbatim pattern cleanly; no issues spotted here.
crates/biome_html_syntax/src/file_source.rs (1)
24-36: Glimmer variant wiring and resolution look consistentAdding
HtmlVariant::Glimmerplusglimmer()/is_glimmer()and the"gjs"/"gts"+ language‑id mappings lines up neatly with the existing Astro/Vue/Svelte handling. Behaviour fromTryFrom<&Utf8Path>flows through correctly to the new variant.Also applies to: 104-113, 124-133, 145-152
crates/biome_html_parser/tests/spec_test.rs (1)
42-48: Snapshot labelling for Glimmer is spot onMapping
HtmlVariant::Glimmerto a"glimmer"code block keeps the test output clear and in line with the other variants.crates/biome_html_formatter/src/js/auxiliary/subexpression.rs (1)
1-10: Glimmer subexpression formatter is consistent and safeUsing verbatim formatting here mirrors the other Glimmer helpers and keeps things correct while the richer formatting story evolves.
crates/biome_js_formatter/src/js/any/expression.rs (1)
23-23: Wiring for JsGlimmerTemplate looks goodDelegation to node.format() matches surrounding style and should route to the dedicated rule. Nice and tidy.
crates/biome_html_formatter/src/html/any/element.rs (1)
16-17: Good additions; double‑check coverage for other Glimmer nodesThese arms look correct. Please confirm that plain Glimmer mustache expressions route via AnyHtmlContent (or add a direct arm if intended here) so formatting stays exhaustive.
crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_lookalike_div.gjs (1)
1-3: Solid negative testGood guard to keep plain JSX out of .gjs. Carry on.
crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_incomplete_less_than.gjs (1)
1-3: Nice EOF edge caseThis should catch regressions in lexer recovery. 👍
crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_incomplete_templat.gjs (1)
1-3: Good backtracking testCovers partial "<templat" token at EOF. Exactly the sort of paper cut that bites later.
crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_lookalike_temp.gjs (1)
1-3: Clever lookalike guardEnsures doesn’t sneak through. Neat.
crates/biome_html_parser/src/token_source.rs (1)
60-69: Exhaustiveness verified ✓All matches on
TextExpressionKindcorrectly handleTripleandDoubleGlimmer:
- Lexer (lines 1043–1047) handles all four variants
- Parser syntax module (lines 587–611) handles all four variants
No incomplete pattern matching found. The optional refactoring suggestion stands, but the current design is sound and exhaustive.
crates/biome_html_syntax/src/element_ext.rs (1)
25-60: Glimmer nodes correctly excluded from script/style and attribute pathsMarking
HtmlCdataSection,GlimmerMustacheComment, andGlimmerTripleStashExpressionas non‑script/style and non‑attribute‑bearing fits their semantics and keeps the helpers from probing for attributes where none exist. Nice bit of future‑proofing for Glimmer in the HTML layer.crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_template_unclosed.gjs (1)
1-3: Error fixture looks goodThe snippet is nicely minimal and clearly unclosed, so it should exercise the intended Glimmer template error path without extra noise.
crates/biome_html_parser/src/parser.rs (1)
116-119: Glimmer parse options wiring looks consistentThe dedicated
with_double_glimmer_expressionhelper and theHtmlVariant::Glimmerbranch both align with the existingsingle/doubleexpression options. Behaviour for other variants stays unchanged, so this is a clean extension. If you ever fancy it, a short doc comment on the Glimmer helper would make its intent even clearer.Also applies to: 162-165
crates/biome_test_utils/src/lib.rs (1)
15-15: Test helper updated cleanly to new JS parse APISwitching to
parse(&content, file_source)matches the new signature and keeps behaviour in line with how other callers now derive options fromJsFileSource. Nice catch updating the shared test helper so it doesn’t go feral later.Also applies to: 173-189
crates/biome_service/src/file_handlers/svelte.rs (1)
14-15: Svelte handler’s JS parse call is in step with the new APIUsing
parse_js_with_cache(script, file_source, cache)keeps the Svelte path aligned with the rest of the service now that options come fromJsFileSource. No hidden behaviour changes here, just one less knob to forget to wire up.Also applies to: 133-136
crates/biome_js_parser/src/syntax/expr.rs (1)
1218-1254: Glimmer template expression plumbing looks consistentTreating
GLIMMER_TEMPLATEas an expression starter and funnelling it throughparse_glimmer_templateintoJS_GLIMMER_TEMPLATEkeeps lookahead and primary expression parsing in sync, while keeping the template payload opaque. Nicely minimal and matches the intended “embedded template as expression” model.Also applies to: 1396-1398, 2176-2185
crates/biome_service/src/file_handlers/mod.rs (3)
37-40: Importingparsehere is fineThe direct
parseimport is used only byparse_lang_from_script_opening_tagand keeps the coupling local; no concerns from my side.
812-821: Routing Glimmer-embedded JS through the JS handler makes senseTreating
EmbeddingKind::Glimmerthe same asNonefor capabilities keeps.gjs/.gtson the standard JS pipeline, while still letting the embedding kind influence parsing elsewhere. Good trade-off without proliferating handlers.
865-907: Switch toparse(..., JsFileSource::jsx())inparse_lang_from_script_opening_tagUsing the simplified
parseAPI withJsFileSource::jsx()for this tiny JSX snippet is a neat clean-up and keeps the function focused on extracting thelangattribute, not parser plumbing. Behaviour should remain the same as long asparsemirrors the old defaults.crates/biome_service/src/file_handlers/astro.rs (1)
11-12: Astro frontmatter parse call aligned with new parser APISwitching to the slimmer
parse_js_with_cache(frontmatter, js_file_source, cache)signature while still reportingJsFileSource::astro()as the language keeps Astro frontmatter on the JS/TS parser without extra config plumbing. Looks consistent with the rest of the service changes.Also applies to: 117-123
crates/biome_html_formatter/src/js/auxiliary/mustache_comment.rs (1)
6-9: Verbatim is the right call here.Preserving mustache comments verbatim matches user expectations.
crates/biome_html_formatter/src/js/auxiliary/block_helper_closing.rs (1)
7-13: Consistent verbatim handling for closing blocks.Matches the approach for other Glimmer nodes. All good.
crates/biome_js_formatter/src/js/auxiliary/glimmer_template.rs (1)
8-18: Pass‑through formatting looks right.Verbatim write of the template token is appropriate here.
crates/biome_js_syntax/src/file_source.rs (3)
122-129: EmbeddingKind gains Glimmer cleanly.Nice, concise addition with helper is_glimmer().
Also applies to: 142-145
222-233: gjs/gts constructors read well.Defaults (JS/TS module + Glimmer embedding) match ecosystem expectations.
456-477: Tests cover the new ids.Good guardrails. Thanks for adding them.
crates/biome_html_formatter/src/js/auxiliary/block_params.rs (1)
1-10: Verbatim formatting for Glimmer block params is consistentThis follows the existing Glimmer pattern (delegate to
format_html_verbatim_node), so behaviour should be predictable and safe until we want smarter layout here.crates/biome_html_formatter/src/js/auxiliary/triple_stash_expression.rs (1)
1-14: Triple-stash formatter matches the conservative Glimmer strategyFormatting the whole
GlimmerTripleStashExpressionverbatim is the sensible default, especially given its “unsafe HTML” semantics. Looks good.crates/biome_js_parser/src/lexer/mod.rs (1)
135-138: Source-type plumbing for the lexer looks soundStoring
source_type: JsFileSourceonJsLexerand initialising it viafrom_str(...).with_source_type(source_type)gives you a clean per-file hook for Glimmer (and any future embeddings). Defaulting bothoptionsandsource_typekeeps plain.jsbehaviour unchanged.No issues spotted here.
Also applies to: 304-331
crates/biome_html_formatter/src/js/auxiliary/path.rs (1)
1-10: Glimmer path formatter is aligned with the other Glimmer nodesAgain, delegating
GlimmerPathtoformat_html_verbatim_nodekeeps the formatter conservative and consistent with the rest of the Glimmer stack. No objections.crates/biome_html_parser/src/syntax/mod.rs (2)
39-42: Nice extension ofDoubleTextExpressionsto cover GlimmerIncluding
TextExpressionKind::DoubleGlimmerhere keeps feature-gating consistent and lets Glimmer reuse the existing “double text expressions enabled” switch without special-casing elsewhere. Looks good.
219-248: Glimmer dispatch order inElementListlooks sensibleThe “block helper → mustache → legacy double text expression” chain is a neat way to keep Glimmer strictly opt‑in while preserving existing
{{ }}behaviour elsewhere. The only thing to double‑check is thatparse_glimmer_block_helper/parse_glimmer_mustache_expressionalways returnAbsentwhen Glimmer mode is off, so non‑Glimmer files still pick up the legacy branch as before.crates/biome_html_parser/src/syntax/glimmer/mod.rs (1)
359-392: Block params parsing matches Glimmer’sas |...|shapeThe
askeyword check plus|param1 param2|list matches Glimmer block params syntax well, and the diagnostics for missing pipes are clear. Looks good as a first implementation.crates/biome_html_parser/src/lexer/mod.rs (4)
39-54: IdentifierContext::Glimmer keeps Glimmer idents distinct without leaking keywordsUsing a dedicated
IdentifierContext::Glimmerand mapping those identifiers straight toIDENTwhile leaving keyword handling to higher layers is a good choice – it avoids conflating Glimmer names with HTML/Svelte keywords but gives the parser enough structure to work with.Also applies to: 443-502
71-99: Brace dispatch and new text-expression contexts are wired up cleanlyThe ordering for
{in bothconsume_token_inside_tagandconsume_token(triple → mustache comment → Svelte → double → literal) looks correct and avoids mis‑classifying{{!--or{{{sequences. Likewise, the newTextExpressionKind::Triple/DoubleGlimmerbranches innext_tokentie in neatly with the new consumer functions.All of this keeps the lexer free of parser options, in line with the existing style.
Also applies to: 115-141, 793-831, 1043-1048
650-655: Triple brace tokens are minimal and to the pointThe
consume_l_triple_text_expression/consume_r_triple_text_expressionhelpers are straightforward and mirror the existing double‑brace helpers nicely. No issues here.Also applies to: 689-694
268-290: Numeric literal handling in Glimmer expressions needs scope clarificationThe concern is factually accurate: both
consume_double_glimmer_expressionandconsume_triple_text_expressionlack numeric handling, so digits now generate lexer errors instead of being silently consumed asHTML_LITERAL. Test fixtures contain no numeric literals (only identifiers, strings, and operators), suggesting this narrowing is intentional—but it should be explicit.Either add numeric support to the match statements or document that only identifiers and string literals are supported in Glimmer expressions for now. The former requires handling
current.is_ascii_digit(), similar to identifier handling.crates/biome_html_formatter/src/glimmer.rs (1)
9-62: Verbatim Glimmer formatting is a sensible first stepThe macro‑based
FormatVerbatimNodewiring for all Glimmer node types is a neat way to integrate them into the formatter without risking mangling users’ templates. Once you’re ready to pretty‑print Glimmer, you can drop in proper rules behind the same traits.crates/biome_js_parser/src/parse.rs (1)
141-151: Splitting default vs explicit-options parsing cleans up the APIUsing
parse_common/JsParser::newfor the standardparse/parse_script/parse_modulepath andparse_common_with_optionsfor tests/embedded scenarios is a nice separation of concerns. Deriving options fromJsFileSourceby default makes the public API easier to use while still leavingparse_with_optionsavailable when you genuinely need custom flags.Also applies to: 269-302
crates/biome_html_formatter/src/generated.rs (1)
85-2046: Generated code looks correct.The formatter wiring for all Glimmer nodes follows the established patterns consistently. Since this is generated code (as noted in the file header), verification that the code generator produced correct output is complete.
| impl FormatRule<GlimmerArgumentList> for FormatGlimmerArgumentList { | ||
| type Context = HtmlFormatContext; | ||
| fn fmt(&self, node: &GlimmerArgumentList, f: &mut HtmlFormatter) -> FormatResult<()> { | ||
| f.join().entries(node.iter().formatted()).finish() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Arguments will print without separators.
Joiner has no separator; arguments risk being concatenated (e.g. helper foo bar -> foobar). Join with spaces (and allow wrapping).
fn fmt(&self, node: &GlimmerArgumentList, f: &mut HtmlFormatter) -> FormatResult<()> {
- f.join().entries(node.iter().formatted()).finish()
+ f.join()
+ .entries(node.iter().formatted())
+ .separated(space()) // or: soft_line_break_or_space()
+ .finish()
}Please confirm the exact API name used elsewhere (separated(space) vs with_space). Quick grep:
🏁 Script executed:
#!/bin/bash
rg -nP -C2 'f\.join\(\).*\.entries\([^)]*\)\.(separated|with_space|separator)\(' crates/biome_html_formatterLength of output: 0
🏁 Script executed:
#!/bin/bash
# Find all join() usage patterns in the HTML formatter
rg -nP -C3 'f\.join\(\)' crates/biome_html_formatter --type=rs | head -50Length of output: 26
🏁 Script executed:
#!/bin/bash
# Search for separator-related methods in the formatter codebase
rg -nP '\.(separated|with_space|space_separator)\(' crates/biome_html_formatter --type=rs | head -30Length of output: 26
🏁 Script executed:
#!/bin/bash
# Look for other argument list formatting examples
rg -nP 'ArgumentList|argument_list' crates/biome_html_formatter --type=rs -C2 | head -40Length of output: 26
🏁 Script executed:
#!/bin/bash
# Find join() usage patterns in the HTML formatter
rg -nP -C3 'f\.join\(\)' crates/biome_html_formatter/ | head -50Length of output: 1278
🏁 Script executed:
#!/bin/bash
# Search for separator methods across the formatter
rg -nP '\.(separated|with_space|space_separator)\(' crates/biome_html_formatter/ | head -30Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check the actual file under review
cat crates/biome_html_formatter/src/js/lists/argument_list.rsLength of output: 409
🏁 Script executed:
#!/bin/bash
# Find HtmlFormatter definition
rg -nP 'impl.*HtmlFormatter|struct HtmlFormatter' crates/biome_html_formatter/ -C2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find join() method definition in HtmlFormatter
rg -nP 'fn join\(' crates/biome_html_formatter/ -C4Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for joiner type/trait to understand available methods
rg -nP '(type.*Joiner|trait.*Joiner|impl.*Joiner)' crates/biome_html_formatter/ -C3Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check what the joiner returns and what methods exist on it
rg -nP 'entries|finish' crates/biome_html_formatter/src/js/lists/ -B2 -A1Length of output: 884
🏁 Script executed:
#!/bin/bash
# Find the prelude to see where HtmlFormatter comes from
find crates/biome_html_formatter -name "prelude.rs" -o -name "mod.rs" | head -5Length of output: 247
🏁 Script executed:
#!/bin/bash
# Check the main lib.rs/mod.rs for prelude definition
cat crates/biome_html_formatter/src/lib.rs 2>/dev/null || cat crates/biome_html_formatter/src/mod.rs 2>/dev/nullLength of output: 11255
🏁 Script executed:
#!/bin/bash
# Search in parent formatter crate for joiner/join method
rg -nP 'fn join\(|impl.*join' crates/biome_formatter/ -C3 | head -40Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check the separated module in HTML formatter
cat crates/biome_html_formatter/src/separated.rs 2>/dev/null | head -50Length of output: 1752
🏁 Script executed:
#!/bin/bash
# Search for joiner-related code in biome_formatter crate
rg -nP 'join' crates/biome_formatter/src --type-list | head -5Length of output: 389
🏁 Script executed:
#!/bin/bash
# Broader search for joiner in biome_formatter
fd -e rs . crates/biome_formatter/src | xargs grep -l "join" | head -10Length of output: 120
🏁 Script executed:
#!/bin/bash
# Look for entries/finish methods to understand joiner API
rg -nP '\.(entries|finish)\(' crates/biome_formatter/src -B2 | head -50Length of output: 3934
🏁 Script executed:
#!/bin/bash
# Search for join() implementation in builders.rs
rg -nP 'fn join|pub.*join|impl.*Joiner' crates/biome_formatter/src/builders.rs -C5 | head -80Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for Joiner struct/type definition
rg -nP 'struct Joiner|pub Joiner' crates/biome_formatter/src/builders.rs -A10 | head -60Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for methods on Joiner like entries, separated, finish
rg -nP 'impl Joiner|fn (entries|separated|finish|with_space)' crates/biome_formatter/src/builders.rs -B2 -A2 | head -80Length of output: 1206
🏁 Script executed:
#!/bin/bash
# Get the Join struct definition and full context
rg -nP 'struct Join' crates/biome_formatter/src/builders.rs -A30 | head -80Length of output: 2394
🏁 Script executed:
#!/bin/bash
# Find what join() method returns
rg -nP 'fn join\(' crates/biome_formatter/src -B2 -A5 | head -50Length of output: 0
🏁 Script executed:
#!/bin/bash
# Get full context around line 2465 (Join::entries)
sed -n '2430,2520p' crates/biome_formatter/src/builders.rsLength of output: 2762
🏁 Script executed:
#!/bin/bash
# Search for join() method on Formatter
rg -nP 'impl.*Formatter.*{' crates/biome_formatter/src/formatter.rs -A200 | grep -E '(fn join|pub fn)' | head -20Length of output: 948
🏁 Script executed:
#!/bin/bash
# Find all join-related methods on Formatter
rg -nP 'pub fn.*(join|join_with)' crates/biome_formatter/src/formatter.rs -B1 -A3 | head -60Length of output: 1007
🏁 Script executed:
#!/bin/bash
# Look at the Formatter impl block for join methods
sed -n '300,600p' crates/biome_formatter/src/formatter.rs | grep -A5 'join'Length of output: 0
Use join_with() to add separator between arguments.
The current code concatenates arguments without separators because f.join() has no separator. Use f.join_with(soft_line_break_or_space()) instead:
fn fmt(&self, node: &GlimmerArgumentList, f: &mut HtmlFormatter) -> FormatResult<()> {
- f.join().entries(node.iter().formatted()).finish()
+ f.join_with(soft_line_break_or_space())
+ .entries(node.iter().formatted())
+ .finish()
}The .separated() method referenced in the original suggestion does not exist; the API uses join_with() to set a separator upfront.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In crates/biome_html_formatter/src/js/lists/argument_list.rs around lines 5 to
9, the code uses f.join() which produces concatenated arguments with no
separator; replace the join call with f.join_with(soft_line_break_or_space()) so
the formatter inserts a soft line break or space between arguments, i.e. call
f.join_with(soft_line_break_or_space()).entries(node.iter().formatted()).finish();
ensure soft_line_break_or_space is in scope or imported if necessary.
| pub(crate) fn parse_glimmer_mustache_expression(p: &mut HtmlParser) -> ParsedSyntax { | ||
| use crate::token_source::TextExpressionKind; | ||
|
|
||
| if !p.at(T!["{{"]) { | ||
| return Absent; | ||
| } | ||
|
|
||
| // Only parse as Glimmer if DoubleGlimmer context is enabled (i.e., in .gjs/.gts files) | ||
| // For other files (Vue, etc.), return Absent to fall back to generic parsing | ||
| if p.options().text_expression != Some(TextExpressionKind::DoubleGlimmer) { | ||
| return Absent; | ||
| } | ||
|
|
||
| let m = p.start(); | ||
|
|
||
| // Use DoubleGlimmer context for proper Glimmer tokenization | ||
| let context = HtmlLexContext::TextExpression(TextExpressionKind::DoubleGlimmer); | ||
|
|
||
| // Bump opening {{ with Glimmer context | ||
| p.bump_with_context(T!["{{"], context); | ||
|
|
||
| // Parse the path (e.g., this.foo, helper, @arg) | ||
| let _ = parse_glimmer_path(p, context); | ||
|
|
||
| // Parse the argument list (can be empty) | ||
| let _ = parse_glimmer_argument_list(p, context); | ||
|
|
||
| // Bump closing }} and switch back to regular context | ||
| if p.at(T!["}}"]) { | ||
| p.bump_with_context(T!["}}"], HtmlLexContext::Regular); | ||
| } else { | ||
| p.error(p.err_builder("Expected closing }}", p.cur_range())); | ||
| } | ||
|
|
||
| Present(m.complete(p, GLIMMER_MUSTACHE_EXPRESSION)) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Mustache expression gating is good; missing }} recovery is shallow
Gating on TextExpressionKind::DoubleGlimmer ensures Glimmer parsing only kicks in for .gjs/.gts, which is spot‑on. When the closing }} is missing, though, you only emit an error and keep going without any resynchronisation or context reset; combined with the lexer behaviour, that likely leaves the parser in Glimmer expression mode for the rest of the file.
It would be safer to either:
- recover to a bogus node using a token set (similar to
parse_double_text_expression), or - explicitly reset the lex context to regular HTML so later content isn’t tokenised as Glimmer by accident.
That would make unterminated mustaches much less catastrophic.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find and examine bump_with_context implementation and HtmlLexContext
rg -n "fn bump_with_context" crates/biome_html_parser/src/ -A 8
Length of output: 764
🏁 Script executed:
#!/bin/bash
# Find parse_double_text_expression to see comparison recovery pattern
fd "\.rs$" crates/biome_html_parser/src/ --exec grep -l "parse_double_text_expression" {} \;Length of output: 42
🏁 Script executed:
#!/bin/bash
# Examine HtmlLexContext definition and usage
rg -n "enum HtmlLexContext|struct HtmlLexContext" crates/biome_html_parser/src/ -A 5Length of output: 621
🏁 Script executed:
#!/bin/bash
# Search for error recovery patterns in HTML parser
rg -n "bump_with_context.*context" crates/biome_html_parser/src/syntax/ -B 2 -A 2Length of output: 7582
🏁 Script executed:
#!/bin/bash
# Look for test cases related to glimmer or mustache
fd "test" crates/biome_html_parser/ -type f -name "*.rs" --exec grep -l "glimmer\|mustache" {} \;Length of output: 170
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full HtmlLexContext enum definition
rg -n "pub\(crate\) enum HtmlLexContext" crates/biome_html_parser/src/token_source.rs -A 20
Length of output: 1184
🏁 Script executed:
#!/bin/bash
# Find and examine parse_double_text_expression implementation
rg -n "fn parse_double_text_expression" crates/biome_html_parser/src/syntax/mod.rs -A 30Length of output: 1306
🏁 Script executed:
#!/bin/bash
# Search for context reset or context management patterns
rg -n "HtmlLexContext::Regular\|set.*context\|context.*reset" crates/biome_html_parser/src/ -B 2 -A 2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Examine token_source.rs next_non_trivia_token to understand context lifecycle
rg -n "fn next_non_trivia_token" crates/biome_html_parser/src/token_source.rs -A 15Length of output: 594
Context not reset when closing }} missing—tokenisation remains in Glimmer mode
The review concern is well-founded. When parse_glimmer_mustache_expression encounters a missing }}, it emits an error (line 174) but doesn't call bump_with_context() or reset the lex context. Since context is passed per-token-fetch to the lexer, skipping that call means the next token is fetched with the stale TextExpression(DoubleGlimmer) context still active.
Compare with parse_double_text_expression (mod.rs:490), which calls expect_with_context() to handle context switching, and includes recovery patterns (lines 500–505 with ParseRecoveryTokenSet). Glimmer has neither—no explicit reset and no bogus-node recovery.
Fix: either bump the missing }} with HtmlLexContext::Regular context (recording the error), or add a recovery fallback. Without one, subsequent file content tokenises as Glimmer—exactly the cascading corruption the reviewer flagged.
🤖 Prompt for AI Agents
crates/biome_html_parser/src/syntax/glimmer/mod.rs around lines 142–177: when
the closing "}}" is missing we currently call p.error(...) but never reset the
lexing context, leaving the lexer in TextExpression(DoubleGlimmer) for
subsequent tokens; fix by switching to the same recovery pattern used in
parse_double_text_expression: replace the current missing-"}}"-branch with an
expect_with_context(T!["}}"], HtmlLexContext::Regular) (or, if you prefer
explicit steps, emit the error then call a bump/expect with
HtmlLexContext::Regular so the lexer context is reset) and add a small
ParseRecoveryTokenSet (matching the other function) to consume/skip until a safe
recovery point so tokenisation doesn’t remain stuck in Glimmer mode.
| fn parse_glimmer_block_helper_opening(p: &mut HtmlParser) -> ParsedSyntax { | ||
| use crate::token_source::TextExpressionKind; | ||
|
|
||
| if !p.at(T!["{{"]) { | ||
| return Absent; | ||
| } | ||
|
|
||
| let m = p.start(); | ||
| let context = HtmlLexContext::TextExpression(TextExpressionKind::DoubleGlimmer); | ||
|
|
||
| // Bump opening {{ | ||
| p.bump_with_context(T!["{{"], context); | ||
|
|
||
| // Bump # token | ||
| if p.at(T![#]) { | ||
| p.bump_with_context(T![#], context); | ||
| } else { | ||
| p.error(p.err_builder("Expected '#' for block helper", p.cur_range())); | ||
| } | ||
|
|
||
| // Parse helper path (e.g., 'if', 'each', 'let') | ||
| let _ = parse_glimmer_path(p, context); | ||
|
|
||
| // Parse arguments | ||
| let _ = parse_glimmer_argument_list(p, context); | ||
|
|
||
| // Parse optional block params (as |param1 param2|) | ||
| if p.at(IDENT) { | ||
| // Check if this is 'as' keyword | ||
| let checkpoint = p.checkpoint(); | ||
| p.bump_with_context(IDENT, context); | ||
|
|
||
| if p.at(T![|]) { | ||
| // This looks like block params, rewind and parse properly | ||
| p.rewind(checkpoint); | ||
| let _ = parse_glimmer_block_params(p, context); | ||
| } else { | ||
| // Not block params, rewind | ||
| p.rewind(checkpoint); | ||
| } | ||
| } | ||
|
|
||
| // Bump closing }} | ||
| if p.at(T!["}}"]) { | ||
| p.bump_with_context(T!["}}"], HtmlLexContext::Regular); | ||
| } else { | ||
| p.error(p.err_builder("Expected closing }}", p.cur_range())); | ||
| } | ||
|
|
||
| Present(m.complete(p, GLIMMER_BLOCK_HELPER_OPENING)) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Opening block helper doesn’t reset context when }} is missing
The overall flow inside parse_glimmer_block_helper_opening is good – especially the as |...| block params detection – but similar to the mustache/triple cases, the missing‑}} branch only emits an error and doesn’t reset the lex context. That means children may be lexed as if they were still inside a Glimmer expression after an unterminated opening tag.
I’d suggest aligning this with whatever recovery strategy you choose for plain mustaches/triple‑stash so unterminated openings don’t poison the rest of the template.
🏁 Script executed:
#!/bin/bash
# Locate and examine the glimmer parsing code
fd 'mod\.rs' crates/biome_html_parser/src/syntax/glimmer \
--exec cat {} \;Length of output: 12312
🏁 Script executed:
#!/bin/bash
# Find mustache and triple-stash parsing functions to understand recovery strategy
rg -n -A 20 'parse_mustache|parse_triple' crates/biome_html_parser/src/syntax/ --type rust | head -100Length of output: 5220
🏁 Script executed:
#!/bin/bash
# Search for error handling patterns when closing tokens are missing
rg -n -B 5 -A 5 'Expected closing|unterminated|recovery' crates/biome_html_parser/src/syntax/ --type rust | head -80Length of output: 6555
Glimmer block helper opening needs error recovery for unterminated expressions
The concern is well-founded. When }} is missing in parse_glimmer_block_helper_opening (lines 303–307), the function emits an error but leaves the lex context in TextExpression(DoubleGlimmer) instead of resetting to Regular. This poisons subsequent token lexing.
The triple-stash expression (lines 655–665 in mod.rs) already demonstrates a recovery pattern: it consumes tokens until finding the closing token or reaching a safe boundary. The Glimmer block helper opening—and the mustache expression—should follow suit.
Consider adding resynchronisation logic similar to triple-stash's recovery loop before completing the marker, or at minimum explicitly reset context to HtmlLexContext::Regular in the error branch.
🤖 Prompt for AI Agents
In crates/biome_html_parser/src/syntax/glimmer/mod.rs around lines 261 to 311,
the parser sets HtmlLexContext::TextExpression(DoubleGlimmer) for the opening
glimmer block but on the missing closing "}}" (lines ~303–307) only emits an
error and leaves the lex context in TextExpression, which corrupts subsequent
lexing; add error-recovery: after emitting the error, run a resynchronisation
loop that consumes tokens (bumping them with the current TextExpression context)
until you find the closing "}}" or a safe boundary (e.g., EOF or a tag boundary
such as '<' or start of a new mustache), then if you see the "}}" bump it and
set the lex context back to HtmlLexContext::Regular (or even if you hit
EOF/boundary, explicitly set context to Regular), and only then complete the
GLIMMER_BLOCK_HELPER_OPENING node so downstream lexing is not poisoned.
| /// Parse a Glimmer mustache comment. | ||
| /// Grammar: GlimmerMustacheComment = comment_token: 'mustache_comment' | ||
| fn parse_mustache_comment(p: &mut HtmlParser) -> ParsedSyntax { | ||
| if !p.at(MUSTACHE_COMMENT) { | ||
| return Absent; | ||
| } | ||
|
|
||
| let m = p.start(); | ||
| p.bump(MUSTACHE_COMMENT); | ||
| Present(m.complete(p, GLIMMER_MUSTACHE_COMMENT)) | ||
| } | ||
|
|
||
| /// Parse a Glimmer triple-stash expression (unescaped HTML). | ||
| /// Grammar: GlimmerTripleStashExpression = | ||
| /// l_curly3_token: 'l_triple_curly' | ||
| /// path: GlimmerPath | ||
| /// arguments: GlimmerArgumentList | ||
| /// r_curly3_token: 'r_triple_curly' | ||
| fn parse_triple_stash_expression(p: &mut HtmlParser) -> ParsedSyntax { | ||
| if !p.at(L_TRIPLE_CURLY) { | ||
| return Absent; | ||
| } | ||
|
|
||
| let m = p.start(); | ||
|
|
||
| let context = HtmlLexContext::TextExpression(TextExpressionKind::Triple); | ||
|
|
||
| // Bump with context to switch lexer to expression mode | ||
| p.bump_with_context(L_TRIPLE_CURLY, context); | ||
|
|
||
| // Parse the path (e.g., this.foo, helper, @arg) | ||
| let _ = parse_glimmer_path(p, context); | ||
|
|
||
| // Parse the argument list (can be empty) | ||
| let _ = parse_glimmer_argument_list(p, context); | ||
|
|
||
| // Bump closing }}} and switch back to regular context | ||
| if p.at(R_TRIPLE_CURLY) { | ||
| p.bump_with_context(R_TRIPLE_CURLY, HtmlLexContext::Regular); | ||
| } else { | ||
| // Missing closing }}}, emit error and try to recover | ||
| p.error(p.err_builder("Expected closing }}}", p.cur_range())); | ||
|
|
||
| // Try to resynchronize: consume tokens until we find }}} or reach a safe boundary | ||
| while !p.at(R_TRIPLE_CURLY) && !p.at(EOF) && !p.at(T![<]) { | ||
| p.bump_any(); | ||
| } | ||
|
|
||
| // If we found }}}, consume it | ||
| if p.at(R_TRIPLE_CURLY) { | ||
| p.bump_with_context(R_TRIPLE_CURLY, HtmlLexContext::Regular); | ||
| } else { | ||
| // Reached EOF or safe boundary, switch context back to Regular | ||
| // (Note: We don't bump here since we're at a boundary token) | ||
| } | ||
| } | ||
|
|
||
| Present(m.complete(p, GLIMMER_TRIPLE_STASH_EXPRESSION)) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Triple-stash recovery doesn’t appear to reset lexing context
The recovery loop for unterminated triple‑stash expressions is a good start, but when R_TRIPLE_CURLY is missing you never seem to switch the lexer back to HtmlLexContext::Regular – only the successful R_TRIPLE_CURLY path calls bump_with_context(..., HtmlLexContext::Regular). That risks leaving the lexer stuck in TextExpression(Triple) for the rest of the document after a single missing }}}, which could badly skew downstream diagnostics.
Similarly, parse_glimmer_mustache_expression emits an error for a missing }} but doesn’t reset the context either.
I’d recommend ensuring both paths explicitly restore the regular HTML lex context when they decide “we’re done with this expression”, even if the closer is missing (e.g. by inserting a synthetic closer or directly resetting the context) so recovery is local to the expression.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the full implementations of both functions
rg -n "fn parse_glimmer_mustache_expression|fn parse_triple_stash_expression" crates/biome_html_parser/src/syntax/mod.rs
Length of output: 74
🏁 Script executed:
#!/bin/bash
# Get the full parse_glimmer_mustache_expression function
rg -A 30 "fn parse_glimmer_mustache_expression" crates/biome_html_parser/src/syntax/mod.rsLength of output: 0
🏁 Script executed:
#!/bin/bash
# Search for how context restoration is handled in similar error cases elsewhere
rg -n "bump_with_context.*Regular" crates/biome_html_parser/src/syntax/ -A 2 -B 2Length of output: 2843
🏁 Script executed:
#!/bin/bash
# Check if there are patterns for synthetic token insertion or explicit context switching in error recovery
rg -n "resync|recover" crates/biome_html_parser/src/syntax/mod.rs -C 3Length of output: 3216
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full context around the glimmer mustache and block functions in glimmer/mod.rs
rg -A 15 "fn parse_glimmer_mustache_expression" crates/biome_html_parser/src/syntax/glimmer/mod.rs
Length of output: 563
🏁 Script executed:
#!/bin/bash
# Also check the block expression function which might have similar issues
rg -A 20 "fn parse_glimmer_block_expression" crates/biome_html_parser/src/syntax/glimmer/mod.rsLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check what happens after line 671 in mod.rs to understand the full context
sed -n '655,680p' crates/biome_html_parser/src/syntax/mod.rsLength of output: 894
---
🏁 Script executed:
```shell
#!/bin/bash
# Get all the error handling sections from glimmer/mod.rs functions
rg -B 5 -A 10 'Expected closing }}' crates/biome_html_parser/src/syntax/glimmer/mod.rs
Length of output: 1336
Context reset missing in error recovery paths across multiple Glimmer and triple-stash functions
The lexer context isn't restored when these expressions close abnormally, leaving the lexer stuck in expression mode for the rest of the document:
parse_triple_stash_expression(mod.rs:671): recovery path reaching EOF/boundaryparse_glimmer_mustache_expression(glimmer/mod.rs:~173): error path after missing}}parse_glimmer_block_helper_opening(glimmer/mod.rs:~307): error path after missing}}parse_glimmer_block_helper_closing(glimmer/mod.rs:~346): error path after missing}}
Ensure all error paths explicitly restore HtmlLexContext::Regular (via bump_with_context or by resetting the lexer state directly) so recovery doesn't poison downstream token lexing.
🤖 Prompt for AI Agents
crates/biome_html_parser/src/syntax/mod.rs lines 619-677: the
parse_triple_stash_expression recovery path can exit without restoring the
lexing context, leaving the lexer in TextExpression mode; update the error
branches so that any path which does not consume the closing token explicitly
resets the lexer to HtmlLexContext::Regular (use
p.bump_with_context(R_TRIPLE_CURLY, HtmlLexContext::Regular) when consuming the
token and when you cannot consume it, call the lexer-reset API (e.g., set the
lex context back to Regular or call a no-op bump_with_context on the boundary
token) before returning) and apply the same fix pattern to the other functions
mentioned (parse_glimmer_mustache_expression,
parse_glimmer_block_helper_opening, parse_glimmer_block_helper_closing) so every
error-recovery exit point restores HtmlLexContext::Regular.
| AnyJsExpression::JsGlimmerTemplate(node) => node.format().fmt(f), | ||
| AnyJsExpression::JsMetavariable(node) => FormatJsMetavariable.fmt_node(node, f), |
There was a problem hiding this comment.
Respect the "without comments" contract for Glimmer templates
This path uses node.format().fmt(f), which may include comments. For consistency with siblings, call the specific rule’s fmt_node.
Apply this diff within the match:
- AnyJsExpression::JsGlimmerTemplate(node) => node.format().fmt(f),
+ AnyJsExpression::JsGlimmerTemplate(node) => {
+ FormatJsGlimmerTemplate.fmt_node(node, f)
+ }And add the missing import near the other uses:
use crate::js::auxiliary::glimmer_template::FormatJsGlimmerTemplate;| impl From<&JsFileSource> for JsParserOptions { | ||
| /// Derive parser options from the file source type. | ||
| /// | ||
| /// This allows parser configuration to be automatically determined based on | ||
| /// file characteristics (language, variant, embedding kind, etc.) rather than | ||
| /// requiring explicit option passing. | ||
| /// | ||
| /// Currently returns default options for all file types, but provides a | ||
| /// centralized place to configure file-type-specific parsing behavior in the future. | ||
| /// For example, Glimmer template parsing (.gjs/.gts files) is handled at the | ||
| /// lexer level by checking the embedding kind. | ||
| fn from(_file_source: &JsFileSource) -> Self { | ||
| // File-type-specific options could be configured here based on: | ||
| // - _file_source.language() - JavaScript vs TypeScript | ||
| // - _file_source.variant() - Standard vs JSX | ||
| // - _file_source.module_kind() - Script vs Module | ||
| // - _file_source.as_embedding_kind() - Astro, Vue, Svelte, Glimmer, etc. | ||
| // | ||
| // For now, Glimmer-specific behavior (template lexing) is handled in the lexer | ||
| // by checking _file_source.as_embedding_kind().is_glimmer() | ||
|
|
||
| Self::default() | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
No‑op From<&JsFileSource> is fine; add a guard test.
LGTM as a staging hook. Please add a small test asserting JsParserOptions::from(&JsFileSource::gjs()) == default(), so future toggles don’t silently alter JS parsing in Glimmer files.
🏁 Script executed:
#!/bin/bash
# Minimal guard: ensure the From impl currently returns defaults for gjs/gts
rg -n "impl From<&JsFileSource> for JsParserOptions" -C2Length of output: 319
---
🏁 Script executed:
```shell
#!/bin/bash
# Check for existing tests related to JsParserOptions::from or gjs/gts
rg -n "JsParserOptions.*from|gjs|gts" crates/biome_js_parser/src/ --type rust -C2 | head -40
Length of output: 2992
🏁 Script executed:
#!/bin/bash
# Search for test modules in biome_js_parser
fd -e rs -path "*/tests/*" . crates/biome_js_parser/ | head -20Length of output: 168
🏁 Script executed:
#!/bin/bash
# Check for #[test] or #[cfg(test)] blocks in options.rs
rg -n "#\[test\]|#\[cfg\(test\)\]|mod test" crates/biome_js_parser/src/options.rsLength of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Look for test files in biome_js_parser crate
find crates/biome_js_parser -name "*test*" -o -name "tests.rs"
Length of output: 198
🏁 Script executed:
#!/bin/bash
# Check if tests module exists at the end of options.rs
tail -30 crates/biome_js_parser/src/options.rsLength of output: 1340
🏁 Script executed:
#!/bin/bash
# Search for any #[test] or test functions related to JsParserOptions across entire crate
rg -n "test.*JsParserOptions|JsParserOptions.*test" crates/biome_js_parser/ --type rustLength of output: 0
Add guard test for From impl to prevent silent future changes.
The implementation is sound, but no test exists for the From<&JsFileSource> impl. Add a small test module (within options.rs or in lexer/tests.rs) asserting that JsParserOptions::from(&JsFileSource::gjs()) == JsParserOptions::default(), so future toggles won't silently alter Glimmer parsing behaviour.
🤖 Prompt for AI Agents
In crates/biome_js_parser/src/options.rs around lines 52–75, add a unit test to
lock the From<&JsFileSource> behavior: create a #[cfg(test)] mod tests block
(either in this file or in lexer/tests.rs) that brings the types into scope and
adds a test like fn from_jsfile_source_defaults() which asserts
JsParserOptions::from(&JsFileSource::gjs()) == JsParserOptions::default() using
assert_eq!; ensure necessary imports (use super::* or crate path) are present so
the test compiles and fails if the impl changes.
| // Template with delete (unusual but valid expression) | ||
| const obj = { prop: <template>Prop</template> }; |
There was a problem hiding this comment.
Comment/code mismatch; add explicit delete case.
The comment mentions delete, but this is an object property. Either fix the comment or add a delete test to match it.
-// Template with delete (unusual but valid expression)
-const obj = { prop: <template>Prop</template> };
+// Template as object property initialiser
+const obj = { prop: <template>Prop</template> };
+
+// Template with delete (also valid)
+const deleted = delete <template>Deleted</template>;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_yield_await.gjs
around lines 20-21: the comment says "Template with delete" but the code creates
an object property — either update the comment to reflect that this is an object
property with a template value, or add an explicit delete case to match the
comment (for example add a line that performs a delete on the property such as
deleting the created property or using delete obj.prop or delete obj['prop']
after the object definition); choose one and make the file consistent.
| use biome_js_parser::parse_js_with_cache; | ||
| use biome_js_syntax::{EmbeddingKind, JsFileSource, JsLanguage, TextRange, TextSize}; |
There was a problem hiding this comment.
🧩 Analysis chain
Signature change wired correctly; scan for stragglers.
Import and call sites match the new parse_js_with_cache API. Nice tidy-up.
Run to spot any outdated call sites:
Also applies to: 135-136
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Call sites of parse_js_with_cache:"
rg -n --stats -C2 $'parse_js_with_cache\\s*\\('
echo
echo "Leftover direct uses of JsParserOptions in services/parsers (may indicate old flow):"
rg -n 'JsParserOptions' crates/biome_{service,js_parser}/ -SLength of output: 8691
astro.rs call site is missing the cache parameter—this breaks the function signature.
The script revealed one outstanding issue: crates/biome_service/src/file_handlers/astro.rs:117 calls parse_js_with_cache(frontmatter, file_source) with only 2 arguments. The updated signature requires a third parameter (cache: &mut NodeCache). Lines 135 in svelte.rs and vue.rs, and line 547 in javascript.rs, all correctly pass the cache—astro.rs does not.
🤖 Prompt for AI Agents
In crates/biome_service/src/file_handlers/astro.rs around line 117 the call
parse_js_with_cache(frontmatter, file_source) is missing the required third
parameter; update the call to pass the mutable cache (e.g.
parse_js_with_cache(frontmatter, file_source, &mut cache)) and ensure a
NodeCache instance is in scope or threaded into the function signature (add a
parameter or obtain it the same way svelte.rs/vue.rs/javascript.rs do), and add
any necessary use/import for NodeCache to match the other handlers.
1 participant
Includes #8099
Important
This PR was written primarily by Claude Code.
Summary
This PR builds on #8099 to complete Glimmer template support in Biome. While #8099 added support for
.gjsand.gtsfile types with basic<template>tag recognition, this PR implements the full Glimmer template syntax that appears within those templates.What's New
Added parsing and formatting support for all Glimmer template constructs:
{{this.title}},{{@arg}},{{helper arg key=value}}{{{unsafe-html}}}{{! comment }},{{!-- block comment --}}{{#if}}...{{/if}},{{#each items as |item|}}...{{/each}}{{#let value as |v|}}...{{/let}}<div {{modifier}} ...><div ...attributes>{{helper (nested arg)}}{{#each items as |item index|}}Implementation Details
html.ungrambiome_html_syntax{{,}},{{{, etc.)biome_html_parser/src/syntax/glimmer/mod.rsExample
Test Plan
crates/biome_html_parser/tests/html_specs/ok/glimmer/).gjsand.gtsfile support from feat(js_parser): add Glimmer template support for .gjs and .gts files #8099Docs
Documentation updates are not required as this extends the existing Glimmer/Ember support. The changeset in
.changeset/glimmer-template-support.mddocuments the user-facing changes.