feat(js_parser): add Glimmer template support for .gjs and .gts files#8099
feat(js_parser): add Glimmer template support for .gjs and .gts files#8099wagenet wants to merge 14 commits intobiomejs:nextfrom
Conversation
🦋 Changeset detectedLatest commit: 287779d 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
| pub(crate) struct FormatJsGlimmerTemplate; | ||
|
|
||
| impl FormatNodeRule<JsGlimmerTemplate> for FormatJsGlimmerTemplate { | ||
| fn fmt_fields(&self, node: &JsGlimmerTemplate, f: &mut JsFormatter) -> FormatResult<()> { |
There was a problem hiding this comment.
We should enhance this further in the future.
|
|
||
| /// Attempts to lex a Glimmer template block `<template>...</template>`. | ||
| /// Returns Some(GLIMMER_TEMPLATE) if a template was found, None otherwise. | ||
| fn try_lex_glimmer_template(&mut self) -> Option<JsSyntaxKind> { |
There was a problem hiding this comment.
This method works as advertised, but I could easily be convinced that we could clean it up.
dyc3
left a comment
dyc3
left a comment
There was a problem hiding this comment.
A good start
- update your pr title to our format
- this could use a changeset, since this makes glimmer files processable by biome.
| let template_bytes = b"template>"; | ||
| for (i, &expected) in template_bytes.iter().enumerate() { | ||
| if self.byte_at(i + 1) != Some(expected) { | ||
| return None; | ||
| } | ||
| } |
There was a problem hiding this comment.
this loop isn't necessary. you can simply take a slice from self.source
| if let Some(b'<') = self.current_byte() { | ||
| // Check if this is "</template>" | ||
| let mut matches = true; | ||
| for (i, &expected) in closing_tag.iter().enumerate() { | ||
| if self.byte_at(i) != Some(expected) { | ||
| matches = false; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
same here. the if let Some(b'<') = self.current_byte() check is a decent gate for this logic, but it can be simplified by just comparing a slice from self.source
|
|
||
| if !found_closing { | ||
| // Unclosed template - create diagnostic | ||
| let err = ParseDiagnostic::new("Unclosed `<template>` tag", start..self.position) |
There was a problem hiding this comment.
nit: i would limit the range to just the start tag and not the entire template so it doesn't flag the entire rest of the file
CodSpeed Performance ReportMerging #8099 will not alter performanceComparing Summary
Footnotes
|
wagenet
changed the title
|
When you're ready for another review, bring the PR out of draft |
WalkthroughAdds first-class support for Glimmer template files (.gjs and .gts). Introduces a new Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_js_syntax/src/file_source.rs (1)
377-401: Add language ID handlers for Glimmer template files.Standard language IDs exist for .gjs and .gts files: "gjs" and "gts". The
try_from_language_id()method should include these entries to match the handling of other framework files (Astro, Vue, Svelte) and ensure consistent LSP behaviour across editors.
🧹 Nitpick comments (4)
.changeset/glimmer-template-support.md (1)
1-13: Consider calling out current limitations of Glimmer supportThe summary is clear, but you might want to mention explicitly that the
<template>content is currently treated as opaque (no inner parsing / linting yet). That’ll help Ember users understand why Biome doesn’t surface template-level diagnostics in this phase.crates/biome_js_parser/src/token_source.rs (1)
29-37: source_type plumbing into the lexer looks soundWiring
JsFileSourcethroughfrom_strand intoJsLexer::with_source_typeis a clean way to toggle Glimmer-aware lexing based on file type. No functional concerns from this change itself.If
JsTokenSource::from_stris used outside this crate, you may want to consider a small helper (e.g.from_str_default(source, options)) to ease callers that don’t care aboutJsFileSource, but that’s strictly optional.crates/biome_js_parser/src/lexer/mod.rs (2)
1775-1782: Attribute handling is simplistic but acceptable for Phase 1.The loop consumes everything until
>without considering quoted strings in attributes. This could theoretically break with<template foo=">">, but treating the opening tag content as opaque is reasonable for this phase.Consider enhancing in future phases if attribute parsing becomes necessary.
1791-1805: Closing tag search is functional but could be optimised.The byte-by-byte iteration to find
</template>works correctly. For future optimisation, consider usingmemchrto find candidate<positions, though the current approach is clear and unlikely to be a bottleneck for typical template sizes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
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 (34)
.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_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/js.ungram(4 hunks)xtask/codegen/src/js_kinds_src.rs(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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
🧬 Code graph analysis (6)
crates/biome_js_parser/tests/spec_tests.rs (1)
crates/tests_macros/src/lib.rs (1)
gen_tests(305-312)
crates/biome_js_parser/src/parser.rs (2)
crates/biome_js_parser/src/lexer/mod.rs (2)
source(148-150)from_str(306-319)crates/biome_js_parser/src/token_source.rs (1)
from_str(29-42)
crates/biome_js_formatter/src/js/auxiliary/glimmer_template.rs (1)
crates/biome_js_formatter/src/js/classes/property_class_member.rs (1)
fmt_fields(15-27)
crates/biome_js_parser/src/token_source.rs (2)
crates/biome_js_parser/src/lexer/mod.rs (2)
from_str(306-319)source(148-150)crates/biome_js_parser/src/parser.rs (3)
source(165-167)source_type(67-69)options(59-61)
crates/biome_js_formatter/src/generated.rs (4)
crates/biome_js_formatter/src/js/any/class_member.rs (1)
fmt(9-31)crates/biome_js_formatter/src/js/any/expression.rs (1)
fmt(9-51)crates/biome_js_formatter/src/js/any/statement.rs (1)
fmt(9-46)crates/biome_js_formatter/src/utils/format_node_without_comments.rs (1)
fmt(51-156)
crates/biome_js_parser/src/lexer/mod.rs (2)
crates/biome_js_syntax/src/file_source.rs (1)
default(28-30)crates/biome_js_parser/src/parser.rs (3)
options(59-61)source_type(67-69)new(43-53)
🔇 Additional comments (44)
crates/biome_js_syntax/src/union_ext.rs (1)
100-100: LGTM!Returning
Ok(None)for Glimmer templates is consistent with other non-semantic class members that lack extractable names.crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_default_export.gjs (1)
1-4: LGTM!Clean test case demonstrating standalone Glimmer template behaviour.
crates/biome_grit_patterns/src/grit_target_language/js_target_language/generated_mappings.rs (1)
301-301: LGTM!Generated mapping for JsGlimmerTemplate follows the established pattern.
crates/biome_js_formatter/src/js/any/statement.rs (1)
24-24: LGTM!Generated formatting correctly integrates Glimmer templates as statements.
xtask/codegen/src/js_kinds_src.rs (2)
179-179: LGTM!Clear token definition with helpful comment.
335-335: LGTM!Node definition properly added to the codegen source.
crates/biome_js_formatter/src/js/any/expression.rs (1)
23-23: LGTM!Generated formatting correctly integrates Glimmer templates as expressions.
crates/biome_js_syntax/src/expr_ext.rs (1)
916-917: LGTM!Primary precedence is appropriate for Glimmer templates, consistent with JSX treatment.
crates/biome_js_analyze/src/lint/nursery/no_unused_expressions.rs (1)
262-262: Review comment is incorrect – no action needed.The concern about false positives doesn't apply here. The parser confirms that standalone Glimmer templates are parsed directly as
JsModuleItemnodes in the module item list, not wrapped inExpressionStatement. Sinceno_unused_expressionsonly flags expressions withinExpressionStatementcontexts, standalone templates (like default exports) never reach theis_disallowed()check. The change to excludeJsGlimmerTemplateproperly handles the rare case where a template appears as a pure expression statement, which is indeed ambiguous and should be flagged.crates/biome_js_formatter/src/utils/format_node_without_comments.rs (1)
96-96: LGTM! Glimmer template formatting wired correctly.The delegation to
node.format().fmt(f)is consistent with how other expression types are handled in this match.crates/biome_js_parser/src/syntax/class.rs (1)
538-542: LGTM! Correct placement before modifier parsing.Handling Glimmer templates early (before parsing modifiers) correctly prevents invalid combinations and aligns with how empty class members are handled.
crates/biome_js_formatter/src/js/any/class_member.rs (1)
15-15: LGTM!The formatting dispatch for Glimmer template class members is correctly integrated.
crates/biome_js_formatter/src/js/classes/property_class_member.rs (2)
137-137: LGTM! Glimmer templates correctly excluded from semicolon insertion.Treating Glimmer templates similarly to constructors and static blocks is appropriate.
174-174: LGTM!The logic correctly indicates Glimmer templates have no modifiers (the
truehere means "is_empty", which gets negated to returnhas_modifiers() = false). Whilst the double-negative is slightly confusing, it's consistent with other modifier-less members likeJsEmptyClassMember.crates/biome_service/src/file_handlers/mod.rs (1)
820-820: LGTM! Glimmer embedding correctly mapped to JS capabilities.Since Glimmer templates are treated as opaque tokens (no internal parsing), using JavaScript capabilities is appropriate.
crates/biome_js_syntax/src/parentheses/expression.rs (1)
66-66: LGTM! Glimmer templates correctly exempt from parentheses.Similar to JSX tag expressions, Glimmer templates have their own delimiters and don't need parentheses.
crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_simple.gjs (1)
1-5: LGTM! Good basic test coverage.These fixtures correctly demonstrate Glimmer template usage in assignments and exports.
crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_with_content.gjs (1)
1-10: LGTM! Good coverage of opaque template content.This fixture correctly validates that the parser treats template content (including Glimmer interpolations and block helpers) as opaque tokens without attempting to parse them as JavaScript.
crates/biome_js_parser/tests/spec_tests.rs (1)
4-4: LGTM! Test coverage extended for Glimmer files.The addition of gjs and gts extensions to both ok and error test suites is consistent and appropriate for the new Glimmer template support.
Also applies to: 7-7
crates/biome_js_formatter/src/js/auxiliary/mod.rs (1)
14-14: LGTM! Module declaration properly added.The glimmer_template module is correctly declared with appropriate crate-level visibility and maintains alphabetical ordering.
crates/biome_js_analyze/src/lint/style/use_naming_convention.rs (1)
1158-1158: LGTM! Glimmer templates correctly excluded from naming conventions.Returning
NoneforJsGlimmerTemplateis consistent with the treatment of other non-nameable class members like constructors and static initialisation blocks.crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_typescript.gts (1)
1-13: LGTM! Comprehensive TypeScript test fixture.The test file appropriately covers Glimmer templates in TypeScript contexts: const assignments with type annotations and class members with private properties.
crates/biome_js_analyze/src/lint/complexity/no_static_only_class.rs (1)
165-165: LGTM! Glimmer templates correctly excluded from static-only check.Returning
NoneforJsGlimmerTemplateappropriately excludes these members from static-only class detection, consistent with the treatment of other non-contributing members.crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_class.gjs (1)
1-17: LGTM! Good test coverage for templates in class bodies.The test file appropriately demonstrates Glimmer templates as class members in both simple and complex class scenarios.
crates/biome_js_parser/tests/js_test_suite/error/glimmer_template_unclosed.gjs (1)
1-3: LGTM! Error test appropriately demonstrates unclosed template.The test fixture correctly exercises the error handling path for unclosed Glimmer templates, which should trigger diagnostics as mentioned in the PR description.
crates/biome_js_parser/src/syntax/stmt.rs (1)
376-381: LGTM! Glimmer template statement parsing correctly implemented.The implementation correctly handles top-level Glimmer templates by consuming the token and completing the node. The placement before the generic expression handler ensures templates are recognised first.
crates/biome_js_analyze/src/lint/correctness/no_invalid_constructor_super.rs (1)
222-236: Glimmer templates correctly treated as non-constructorsAdding
AnyJsExpression::JsGlimmerTemplate(_)to theSome(false)group matches the intent that<template>blocks can’t be used as validextendstargets, and keeps the match exhaustive. All good here.crates/biome_js_parser/src/syntax/expr.rs (1)
2175-2183: parse_glimmer_template implementation matches the “opaque node” designThe helper cleanly wraps a single
GLIMMER_TEMPLATEtoken into aJS_GLIMMER_TEMPLATEnode, which aligns with the phase‑1 “treat template contents as opaque” strategy. The extraif !p.at(GLIMMER_TEMPLATE)guard is slightly redundant for the current call site but harmless and keeps the function reusable.crates/biome_js_parser/src/parser.rs (1)
43-52: JsParser now correctly passes source_type into the token sourceUpdating
JsParser::newto callJsTokenSource::from_str(source, source_type, options)ties the parser’sJsFileSourceand the lexer’s source_type together, which is exactly what Glimmer-aware lexing needs. No issues spotted here.crates/biome_js_syntax/src/file_source.rs (4)
126-127: LGTM!The Glimmer variant follows the established pattern for framework-specific embedding kinds.
142-144: LGTM!The helper method is consistent with existing embedding kind predicates.
222-232: LGTM!Both constructors follow the established pattern and correctly configure the base language with Glimmer embedding.
363-365: LGTM!Extension mappings are correct and consistent with the existing framework file handling.
crates/biome_js_formatter/src/generated.rs (1)
2266-2303: Glimmer template formatter wiring looks spot‑onMatches the generated patterns: FormatRule delegates via FormatNodeRule, and AsFormat/IntoFormat provide both borrowed/owned paths. No nits on this hunk. As this file is generated, just keep running codegen if the rule changes.
Tiny check: confirm FormatJsGlimmerTemplate has a Default impl and that it truly prints the template verbatim (incl. CRLF and non‑ASCII) across expression/statement/class‑member contexts. If that’s all green, ship it.
xtask/codegen/js.ungram (4)
123-123: LGTM!Correctly adds Glimmer template support to statements.
336-337: LGTM!Well-organised addition with clear section comment.
666-666: LGTM!Appropriate addition for Glimmer component class members.
890-898: LGTM!Clean node definition that appropriately treats templates as opaque tokens for Phase 1.
crates/biome_js_formatter/src/js/auxiliary/glimmer_template.rs (1)
1-16: LGTM!Clean formatter implementation that correctly preserves template content verbatim. Appropriate for Phase 1's opaque token approach.
crates/biome_js_parser/src/lexer/mod.rs (5)
136-137: LGTM!Clear field addition with appropriate documentation.
317-317: LGTM!Appropriate default initialisation.
325-330: LGTM!Clean builder method for propagating source type context.
1807-1812: LGTM!Diagnostic correctly limits the range to the opening tag (not the entire rest of the file), addressing previous reviewer feedback.
1834-1839: LGTM!Proper integration with appropriate file-type gating. Only attempts Glimmer lexing when source type indicates it's relevant.
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @wagenet. * #8099 (comment) The following files were modified: * `crates/biome_grit_patterns/src/grit_target_language/js_target_language/generated_mappings.rs` * `crates/biome_js_analyze/src/lint/complexity/no_static_only_class.rs` * `crates/biome_js_analyze/src/lint/correctness/no_invalid_constructor_super.rs` * `crates/biome_js_analyze/src/lint/nursery/no_unused_expressions.rs` * `crates/biome_js_analyze/src/lint/style/use_naming_convention.rs` * `crates/biome_js_formatter/src/generated.rs` * `crates/biome_js_formatter/src/js/any/class_member.rs` * `crates/biome_js_formatter/src/js/any/expression.rs` * `crates/biome_js_formatter/src/js/any/statement.rs` * `crates/biome_js_formatter/src/js/auxiliary/glimmer_template.rs` * `crates/biome_js_formatter/src/js/classes/property_class_member.rs` * `crates/biome_js_formatter/src/utils/format_node_without_comments.rs` * `crates/biome_js_parser/src/lexer/mod.rs` * `crates/biome_js_parser/src/parser.rs` * `crates/biome_js_parser/src/syntax/class.rs` * `crates/biome_js_parser/src/syntax/expr.rs` * `crates/biome_js_parser/src/syntax/stmt.rs` * `crates/biome_js_parser/src/token_source.rs` * `crates/biome_js_syntax/src/expr_ext.rs` * `crates/biome_js_syntax/src/file_source.rs` * `crates/biome_js_syntax/src/parentheses/expression.rs` * `crates/biome_js_syntax/src/union_ext.rs` * `crates/biome_service/src/file_handlers/mod.rs`
|
Hmm... didn't realize that CodeRabbit would make a new PR. |
There was a problem hiding this comment.
would be nice for these test files to go in a glimmer/ sub folder, inside both the ok and error folders.
| return None; | ||
| } | ||
| if &self.source.as_bytes()[self.position + 1..end_pos] != template_bytes { | ||
| return None; |
There was a problem hiding this comment.
When try_lex_glimmer_template returns None, does the lexer back up to the start properly?
The error tests need to be comprehensive enough to answer this question.
| // Skip any attributes/whitespace until '>' | ||
| while let Some(chr) = self.current_byte() { | ||
| if chr == b'>' { | ||
| self.next_byte(); // consume '>' |
There was a problem hiding this comment.
nit: next_byte returns the next byte in addition to advancing the lexer, so you could probably simplify this loop even further
| pub fn from_str( | ||
| source: &'l str, | ||
| source_type: biome_js_syntax::JsFileSource, | ||
| options: JsParserOptions, | ||
| ) -> Self { | ||
| let lexer = JsLexer::from_str(source) | ||
| .with_options(options) | ||
| .with_source_type(source_type); |
There was a problem hiding this comment.
For other languages, we usually use the source type to derive the parser options, instead of using the SourceFile directly.
For example, the html parser has a vue option. Whenever we build options for the parser, we send it through a impl From<&HtmlFileSource> for HtmlParseOptions first to enable the things we need. (note that the vue option is not currently present in this branch, you need to rebase to see it)
yeah it does that. rather annoying, and a bit overzealous with what it does imo |
…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 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
crates/biome_js_formatter/src/js/auxiliary/glimmer_template.rs (1)
9-24: Docstring is quite verbose for a simple pass-through formatter.The docstring is thorough but perhaps more detailed than needed for this straightforward implementation. The example with
no_runis fine, though it serves limited practical value for an internal API. Consider simplifying in future iterations.crates/biome_js_parser/src/lexer/mod.rs (2)
1776-1782: Consider simplifying the loop.Since
next_byte()advances and returns the next byte, you can simplify this loop.Based on past review comments.
- // Skip any attributes/whitespace until '>' - while let Some(chr) = self.current_byte() { - if chr == b'>' { - self.next_byte(); // consume '>' - break; - } - self.next_byte(); - } + // Skip any attributes/whitespace until '>' + while let Some(chr) = self.next_byte() { + if chr == b'>' { + break; + } + }
1791-1804: Consider simplifying the search loop.The closing tag search could use
next_byte()more directly to avoid redundant calls.Based on past review comments.
let mut found_closing = false; while self.position < self.source.len() { - if let Some(b'<') = self.current_byte() { - // Check if this is "</template>" - let end_pos = self.position + closing_tag.len(); - if end_pos <= self.source.len() - && &self.source.as_bytes()[self.position..end_pos] == closing_tag - { - // Found closing tag, consume it - self.advance(closing_tag.len()); - found_closing = true; - break; - } + let end_pos = self.position + closing_tag.len(); + if end_pos <= self.source.len() + && &self.source.as_bytes()[self.position..end_pos] == closing_tag + { + // Found closing tag, consume it + self.advance(closing_tag.len()); + found_closing = true; + break; } self.next_byte(); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_whitespace.gjs.snapis excluded by!**/*.snapand included by**crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_yield_await.gjs.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (8)
.changeset/glimmer-template-support.md(1 hunks)crates/biome_js_formatter/src/generated.rs(1 hunks)crates/biome_js_formatter/src/js/auxiliary/glimmer_template.rs(1 hunks)crates/biome_js_parser/src/lexer/mod.rs(4 hunks)crates/biome_js_parser/src/syntax/expr.rs(3 hunks)crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_whitespace.gjs(1 hunks)crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_yield_await.gjs(1 hunks)crates/biome_js_syntax/src/file_source.rs(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/biome_js_parser/src/syntax/expr.rs
- .changeset/glimmer-template-support.md
🧰 Additional context used
🧬 Code graph analysis (4)
crates/biome_js_formatter/src/generated.rs (4)
crates/biome_js_formatter/src/js/any/expression.rs (1)
fmt(9-51)crates/biome_js_formatter/src/js/any/statement.rs (1)
fmt(9-46)crates/biome_js_formatter/src/js/any/class_member.rs (1)
fmt(9-31)crates/biome_js_formatter/src/utils/format_node_without_comments.rs (1)
fmt(51-156)
crates/biome_js_syntax/src/file_source.rs (4)
crates/biome_service/src/file_handlers/mod.rs (1)
try_from_language_id(212-232)crates/biome_html_syntax/src/file_source.rs (1)
try_from_language_id(132-140)crates/biome_service/src/file_handlers/vue.rs (1)
file_source(67-80)crates/biome_service/src/file_handlers/svelte.rs (1)
file_source(67-80)
crates/biome_js_formatter/src/js/auxiliary/glimmer_template.rs (2)
crates/biome_js_formatter/src/js/classes/property_class_member.rs (1)
fmt_fields(15-27)crates/biome_js_formatter/src/generated.rs (16)
format(28-33)format(66-68)format(82-84)format(95-97)format(124-129)format(162-164)format(178-180)format(207-212)format(241-246)format(279-281)format(311-316)format(349-354)format(387-389)format(419-424)format(457-462)format(495-497)
crates/biome_js_parser/src/lexer/mod.rs (2)
crates/biome_js_syntax/src/file_source.rs (1)
default(28-30)crates/biome_js_parser/src/parser.rs (3)
options(59-61)source_type(67-69)new(43-53)
🔇 Additional comments (11)
crates/biome_js_formatter/src/js/auxiliary/glimmer_template.rs (1)
25-31: LGTM! Clean verbatim formatter implementation.The implementation correctly delegates to the template token's formatter, preserving Glimmer template content as intended for Phase 1. The approach aligns with the PR objective of treating templates as opaque tokens.
crates/biome_js_formatter/src/generated.rs (1)
2266-2289: Glimmer formatter wiring looks spot‑on.Impls mirror the established pattern and delegate via FormatNodeRule; consistent with other nodes. Nicely done.
crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_whitespace.gjs (1)
1-13: LGTM!Good whitespace edge case coverage for the template tag parser.
crates/biome_js_syntax/src/file_source.rs (5)
126-144: LGTM!Glimmer embedding kind follows the established pattern for Astro, Vue, and Svelte.
222-232: LGTM!Glimmer file constructors correctly use JS module for .gjs and TypeScript for .gts.
363-365: LGTM!Extension mappings correctly added for .gjs and .gts files.
399-400: LGTM!Language ID mappings correctly handle gjs and gts identifiers.
456-477: LGTM!Tests confirm language ID resolution works correctly for both .gjs and .gts files.
crates/biome_js_parser/src/lexer/mod.rs (3)
136-137: LGTM!Source type tracking correctly added to the lexer state.
Also applies to: 317-317, 325-330
1834-1839: LGTM!Glimmer template detection correctly guards on file type and returns early when a template is found.
1741-1815: The review comment is incorrect—early returns don't advance the lexer position.All the early
Nonereturns (lines 1750, 1758, 1760, 1769) occur before the first position advance at line 1772. The lookahead checks usepeek_byte()andbyte_at(), which don't modify position. If any check fails, the lexer state remains unchanged, and the caller's fallthrough toself.next_byte()at line 1841 correctly handles the<as a normal token. The code is sound.Likely an incorrect or invalid review comment.
| // Template with delete (unusual but valid expression) | ||
| const obj = { prop: <template>Prop</template> }; |
There was a problem hiding this comment.
Minor comment inconsistency.
The comment mentions "Template with delete" but the code tests an object property assignment instead. Consider updating the comment to match the actual test.
-// Template with delete (unusual but valid expression)
+// Template as object property value
const obj = { prop: <template>Prop</template> };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Template with delete (unusual but valid expression) | |
| const obj = { prop: <template>Prop</template> }; | |
| // Template as object property value | |
| const obj = { prop: <template>Prop</template> }; |
🤖 Prompt for AI Agents
In
crates/biome_js_parser/tests/js_test_suite/ok/glimmer_template_yield_await.gjs
around lines 20 to 21, the inline comment "Template with delete (unusual but
valid expression)" is inconsistent with the code which assigns a template to an
object property; update the comment to accurately describe the test (e.g.,
"Template used as an object property value" or "Template expression assigned to
object property") so the comment matches the code under test.
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 <noreply@anthropic.com>
…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 <noreply@anthropic.com>
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.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
crates/biome_js_parser/tests/spec_test.rs (1)
194-194: Fix the function call to match the new API.The
parsefunction now accepts only two arguments (text and source type), but this call is passing three arguments including explicit options.Apply this diff to fix the call:
- let root = parse(code, JsFileSource::ts(), JsParserOptions::default()); + let root = parse_with_options(code, JsFileSource::ts(), JsParserOptions::default());crates/biome_js_parser/src/parser.rs (1)
243-286: Critical: Tests won't compile with new constructor signature.All three test functions call
JsParser::newwith three arguments, but the constructor now only accepts two parameters (options are derived automatically). These tests need updating to either:
- Use the new two-parameter signature:
JsParser::new(source, file_source)- Use the internal
with_optionsconstructor for explicit option controlApply this diff to fix the tests:
fn uncompleted_markers_panic() { - let mut parser = JsParser::new( - "'use strict'", - JsFileSource::default(), - JsParserOptions::default(), - ); + let mut parser = JsParser::with_options( + "'use strict'", + JsFileSource::default(), + JsParserOptions::default(), + ); let _ = parser.start(); // drop the marker without calling complete or abandon } #[test] fn completed_marker_doesnt_panic() { - let mut p = JsParser::new( - "'use strict'", - JsFileSource::default(), - JsParserOptions::default(), - ); + let mut p = JsParser::with_options( + "'use strict'", + JsFileSource::default(), + JsParserOptions::default(), + ); let m = p.start(); p.expect(JsSyntaxKind::JS_STRING_LITERAL); m.complete(&mut p, JsSyntaxKind::JS_STRING_LITERAL_EXPRESSION); } #[test] fn abandoned_marker_doesnt_panic() { - let mut p = JsParser::new( - "'use strict'", - JsFileSource::default(), - JsParserOptions::default(), - ); + let mut p = JsParser::with_options( + "'use strict'", + JsFileSource::default(), + JsParserOptions::default(), + ); let m = p.start(); m.abandon(&mut p); }crates/biome_js_parser/src/parse.rs (6)
48-59: Update stale doc example.The doc comment example still references the old
parse_scriptsignature withJsParserOptions, but the function no longer accepts this parameter.Apply this diff:
/// The syntax node represented by this Parse result /// /// ``` -/// use biome_js_parser::{JsParserOptions, parse_script}; +/// use biome_js_parser::parse_script; /// use biome_js_syntax::{JsIfStatement, JsSyntaxKind}; /// use biome_rowan::{AstNode, AstNodeList}; /// /// let parse = parse_script( /// " /// if (a > 5) { /// /* something */ /// } -/// ", -/// JsParserOptions::default() +/// " /// ); ///
158-184: Update stale doc example.Similar to above, this doc comment example references the old signature.
Apply this diff:
/// Parse text into a [`Parse`](Parse) which can then be turned into an untyped root [`JsSyntaxNode`](JsSyntaxNode). /// Or turned into a typed [`JsScript`](JsScript) with [`tree`](Parse::tree). /// /// ``` -/// use biome_js_parser::{JsParserOptions, parse_script}; +/// use biome_js_parser::parse_script; /// use biome_js_syntax::{JsSyntaxToken, JsFileSource, JsSyntaxList, JsComputedMemberExpression}; /// use biome_rowan::{AstNode, Direction}; /// -/// let parse = parse_script("foo.bar[2]", JsParserOptions::default()); +/// let parse = parse_script("foo.bar[2]");
204-214: Update stale doc example.Another instance of the outdated signature in the doc comment.
Apply this diff:
/// ### Examples /// /// Check the diagnostics emitted by the code /// ``` -/// use biome_js_parser::{JsParserOptions, parse_module}; +/// use biome_js_parser::parse_module; /// let source = r#" /// import { someModule } from "./someModule.js"; /// /// someModule(); /// "#; /// -/// let parse = parse_module(source, JsParserOptions::default()); +/// let parse = parse_module(source);
217-231: Update stale doc example.Yet another outdated doc comment example.
Apply this diff:
/// Retrieve the emitted AST and check its kind: /// ``` -/// use biome_js_parser::{JsParserOptions, parse_module}; +/// use biome_js_parser::parse_module; /// use biome_js_syntax::JsSyntaxKind; /// use biome_rowan::AstNode; /// let source = r#" /// import { someModule } from "./someModule.js"; /// /// someModule(); /// "#; -/// let parse = parse_module(source, JsParserOptions::default()); +/// let parse = parse_module(source);
246-264: Update stale doc example.The
parsefunction doc example still shows the old signature with explicitJsParserOptions.Apply this diff:
/// ### Examples /// /// ``` -/// use biome_js_parser::{JsParserOptions, parse}; +/// use biome_js_parser::parse; /// use biome_js_syntax::{LanguageVariant, LanguageVersion, ModuleKind, JsFileSource}; /// // parse source text as TypeScript /// let mut module = JsFileSource::ts(); -/// let mut parsed = parse("type F = {}", module, JsParserOptions::default()); +/// let mut parsed = parse("type F = {}", module); /// assert_eq!(parsed.diagnostics().len(), 0); /// // parse source text as JSX /// module = JsFileSource::jsx(); -/// parsed = parse("<Component></Component>", module, JsParserOptions::default()); +/// parsed = parse("<Component></Component>", module); /// assert_eq!(parsed.diagnostics().len(), 0); /// // parse source text with granular control /// module = JsFileSource::default() /// .with_version(LanguageVersion::ESNext) /// .with_module_kind(ModuleKind::Module) /// .with_variant(LanguageVariant::Jsx); -/// parsed = parse("foo[bar]", module, JsParserOptions::default()); +/// parsed = parse("foo[bar]", module); /// assert_eq!(parsed.diagnostics().len(), 0); /// ```
309-324: Update stale doc example.The
parse_js_with_cachedoc comment example references the old signature.Apply this diff:
/// ### Examples /// /// ``` -/// use biome_js_parser::{JsParserOptions, parse_js_with_cache}; +/// use biome_js_parser::parse_js_with_cache; /// use biome_js_syntax::JsFileSource; /// use biome_rowan::NodeCache; /// /// let source_type = JsFileSource::js_module(); /// let mut cache = NodeCache::default(); /// let mut source = "function f() { return 2 }"; /// -/// let parsed = parse_js_with_cache(source, source_type, JsParserOptions::default(), &mut cache); +/// let parsed = parse_js_with_cache(source, source_type, &mut cache); /// assert_eq!(parsed.diagnostics().len(), 0); /// /// source = "function bar() { return 3 }"; -/// let parsed = parse_js_with_cache(source, source_type, JsParserOptions::default(), &mut cache); +/// let parsed = parse_js_with_cache(source, source_type, &mut cache); /// assert_eq!(parsed.diagnostics().len(), 0); /// ```
♻️ Duplicate comments (1)
crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_yield_await.gjs (1)
20-21: Comment still inconsistent with code.The comment mentions "delete" but the code assigns a template as an object property value. Update the comment to match the actual test.
🧹 Nitpick comments (2)
crates/biome_service/src/file_handlers/javascript.rs (1)
546-549: Parser API update applied correctly.The removal of the
optionsparameter fromparse_js_with_cacheis correct.However, line 546 still retrieves
optionsbut no longer uses them. Consider removing the unused variable:- let options = settings.parse_options::<JsLanguage>(biome_path, &file_source); - let file_source = file_source.to_js_file_source().unwrap_or_default(); let parse = biome_js_parser::parse_js_with_cache(text, file_source, cache);crates/biome_js_parser/src/options.rs (1)
52-77: Consider simplifying the documentation.The implementation is correct, but the 14 lines of comments explaining future possibilities could be reduced to a brief note about the extension point. The detailed examples of what could be configured aren't necessary until they're actually implemented.
Apply this diff for a more concise approach:
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. + /// Currently returns default options; file-type-specific behaviour + /// (e.g., Glimmer template lexing) is handled in the lexer via embedding kind. fn from(file_source: &JsFileSource) -> Self { - let mut options = Self::default(); - - // 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() - - options + Self::default() } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (14)
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**
📒 Files selected for processing (32)
crates/biome_grit_patterns/src/grit_js_parser.rs(4 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(6 hunks)crates/biome_js_parser/src/parser.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(2 hunks)crates/biome_service/src/file_handlers/astro.rs(0 hunks)crates/biome_service/src/file_handlers/javascript.rs(1 hunks)crates/biome_service/src/file_handlers/mod.rs(1 hunks)crates/biome_service/src/file_handlers/svelte.rs(1 hunks)crates/biome_service/src/file_handlers/vue.rs(1 hunks)crates/biome_test_utils/src/lib.rs(1 hunks)xtask/coverage/src/js/test262.rs(2 hunks)xtask/coverage/src/jsx/jsx_babel.rs(2 hunks)xtask/coverage/src/runner.rs(2 hunks)xtask/coverage/src/symbols/msts.rs(1 hunks)xtask/coverage/src/ts/ts_babel.rs(1 hunks)
💤 Files with no reviewable changes (1)
- crates/biome_service/src/file_handlers/astro.rs
✅ Files skipped from review due to trivial changes (2)
- crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_class.gjs
- crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_default_export.gjs
🧰 Additional context used
🧬 Code graph analysis (16)
crates/biome_js_parser/src/options.rs (4)
crates/biome_service/src/file_handlers/javascript.rs (5)
from(87-106)from(118-124)from(135-140)from(151-156)from(166-170)crates/biome_service/src/file_handlers/svelte.rs (1)
file_source(67-80)crates/biome_service/src/file_handlers/vue.rs (1)
file_source(67-80)crates/biome_js_parser/src/parser.rs (1)
options(83-85)
xtask/coverage/src/runner.rs (1)
crates/biome_js_parser/src/parse.rs (1)
parse_with_options(279-290)
xtask/coverage/src/symbols/msts.rs (1)
crates/biome_js_parser/src/parse.rs (4)
parse_with_options(279-290)parse(189-193)parse(237-238)parse(269-272)
crates/biome_js_parser/src/parser.rs (2)
crates/biome_js_parser/src/token_source.rs (2)
new(21-26)from_str(33-48)crates/biome_js_parser/src/options.rs (1)
from(63-76)
crates/biome_js_parser/src/token_source.rs (2)
crates/biome_js_parser/src/lexer/mod.rs (2)
from_str(306-319)source(148-150)crates/biome_js_parser/src/options.rs (1)
from(63-76)
crates/biome_service/src/file_handlers/svelte.rs (3)
crates/biome_service/src/file_handlers/javascript.rs (1)
parse(539-554)crates/biome_service/src/file_handlers/vue.rs (2)
parse(123-141)file_source(67-80)crates/biome_js_parser/src/parse.rs (1)
parse_js_with_cache(328-338)
crates/biome_service/src/file_handlers/javascript.rs (4)
crates/biome_service/src/file_handlers/svelte.rs (2)
parse(123-141)file_source(67-80)crates/biome_service/src/file_handlers/vue.rs (2)
parse(123-141)file_source(67-80)crates/biome_service/src/file_handlers/astro.rs (1)
parse(109-129)crates/biome_js_parser/src/parse.rs (1)
parse_js_with_cache(328-338)
xtask/coverage/src/ts/ts_babel.rs (2)
crates/biome_js_parser/src/parse.rs (4)
parse_with_options(279-290)parse(189-193)parse(237-238)parse(269-272)xtask/coverage/src/runner.rs (1)
parse(87-89)
crates/biome_grit_patterns/src/grit_js_parser.rs (2)
crates/biome_js_parser/src/parse.rs (4)
parse(189-193)parse(237-238)parse(269-272)parse_with_options(279-290)xtask/coverage/src/runner.rs (1)
parse(87-89)
crates/biome_js_parser/tests/spec_test.rs (2)
crates/biome_js_parser/src/parse.rs (4)
parse(189-193)parse(237-238)parse(269-272)parse_with_options(279-290)xtask/coverage/src/runner.rs (1)
parse(87-89)
crates/biome_service/src/file_handlers/vue.rs (3)
crates/biome_service/src/file_handlers/javascript.rs (1)
parse(539-554)crates/biome_service/src/file_handlers/svelte.rs (2)
parse(123-141)file_source(67-80)crates/biome_js_parser/src/parse.rs (1)
parse_js_with_cache(328-338)
crates/biome_js_parser/src/lexer/mod.rs (2)
crates/biome_js_syntax/src/file_source.rs (1)
default(28-30)crates/biome_js_parser/src/parser.rs (4)
with_options(63-77)options(83-85)source_type(91-93)new(46-57)
xtask/coverage/src/js/test262.rs (2)
crates/biome_js_parser/src/parse.rs (4)
parse(189-193)parse(237-238)parse(269-272)parse_with_options(279-290)xtask/coverage/src/runner.rs (2)
parse(87-89)code(95-97)
xtask/coverage/src/jsx/jsx_babel.rs (2)
crates/biome_js_parser/src/parse.rs (4)
parse(189-193)parse(237-238)parse(269-272)parse_with_options(279-290)xtask/coverage/src/runner.rs (1)
parse(87-89)
crates/biome_test_utils/src/lib.rs (2)
crates/biome_service/src/file_handlers/javascript.rs (1)
parse(539-554)crates/biome_service/src/file_handlers/svelte.rs (2)
parse(123-141)file_source(67-80)
crates/biome_js_parser/src/parse.rs (3)
crates/biome_js_parser/src/parser.rs (4)
new(46-57)source_type(91-93)options(83-85)with_options(63-77)xtask/coverage/src/runner.rs (3)
new(131-133)new(196-201)parse(87-89)crates/biome_service/src/file_handlers/javascript.rs (1)
parse(539-554)
🔇 Additional comments (30)
crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_lookalike_templatex.gjs (1)
1-3: Well-structured edge case test.The test correctly validates that the parser rejects
<templatex>(where the identifier extends beyond "template") and properly exercises lexer backtracking. The comments clearly document the intent.crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_incomplete_templat.gjs (1)
1-3: Let me verify this pattern by checking the content of a related file:The filename is intentionally abbreviated to match the test content.
Analysis of the Glimmer error test suite shows a consistent naming pattern where test filenames reflect their content:
glimmer_lookalike_temp.gjstests incomplete "temp" syntax, whilst files testing complete tags use the full word (e.g.,glimmer_template_self_closing.gjs). The filenameglimmer_incomplete_templat.gjsdeliberately uses the abbreviated form to match the incomplete tag<templaton line 3, making this appropriate naming rather than a typo.crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_lookalike_temp.gjs (1)
1-3: Test verified as correct.The snapshot file exists and generates clear, specific error messages. The parser correctly rejects
<temp>as a TypeScript type assertion (invalid in.gjsfiles) rather than attempting to parse it as JSX. Both diagnostic messages are informative and properly positioned. No changes needed.crates/biome_js_parser/src/lexer/mod.rs (1)
136-137: Glimmer template lexing implementation looks solid.The addition of
source_typetracking, the lookahead-then-commit pattern intry_lex_glimmer_template, and proper integration inresolve_less_thanwith embedding checks are all well done. Error handling for unclosed templates is present.Also applies to: 325-330, 1741-1826, 1844-1850
crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_lookalike_div.gjs (1)
1-3: Appropriate negative test case.Testing that JSX syntax is forbidden in
.gjsfiles is a sensible boundary check for Glimmer template support.crates/biome_service/src/file_handlers/vue.rs (1)
135-135: Correct adaptation to updated parser API.Removing the explicit
JsParserOptionsparameter aligns with the newparse_js_with_cachesignature that derives options fromJsFileSource.xtask/coverage/src/runner.rs (1)
8-8: Correct migration to options-aware parsing.The change from
parsetoparse_with_optionsproperly enables explicit option passing in the test runner.Also applies to: 88-88
crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_template_unclosed.gjs (1)
1-3: Test fixture looks correct.The unclosed template properly demonstrates the error condition.
crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_incomplete_less_than.gjs (1)
1-3: Good edge case coverage.Testing EOF after incomplete syntax is important for parser robustness.
crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_with_content.gjs (1)
1-10: Comprehensive test coverage.The template demonstrates various Glimmer features (interpolations, arguments, block helpers).
crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_template_self_closing.gjs (1)
1-3: Error test looks correct.Properly validates that self-closing template tags are rejected.
crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_typescript.gts (1)
1-13: TypeScript integration test looks solid.Covers both typed constant assignment and class member template usage.
crates/biome_service/src/file_handlers/svelte.rs (1)
135-135: API update correctly applied.Removal of explicit options aligns with the new option derivation from
JsFileSourceintroduced in this PR.crates/biome_service/src/file_handlers/mod.rs (2)
820-820: LGTM!Routing Glimmer embedding to JS capabilities is the correct approach.
867-870: LGTM!The removal of explicit parser options aligns with the new architecture where options are derived from the file source.
crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_simple.gjs (1)
1-5: LGTM!Clear test cases covering template-as-expression and exported template scenarios.
crates/biome_test_utils/src/lib.rs (1)
179-179: LGTM!Correctly updated to use the two-argument parse API.
crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_whitespace.gjs (1)
1-13: LGTM!Excellent coverage of whitespace edge cases for template parsing.
crates/biome_js_parser/tests/spec_test.rs (1)
86-91: LGTM!Good approach to conditionally use explicit options when available whilst falling back to derived options.
crates/biome_grit_patterns/src/grit_js_parser.rs (3)
37-37: LGTM!Correctly updated to the two-argument parse API.
51-55: LGTM!Appropriate use of
parse_with_optionswhen metavariables support is required.
78-82: LGTM!Correctly using
parse_with_optionswith metavariables for snippet parsing.crates/biome_js_parser/src/token_source.rs (1)
33-48: LGTM!The refactoring to derive parser options from the file source type is well-implemented and addresses the architectural concern raised in past reviews.
crates/biome_js_parser/src/parser.rs (2)
43-57: LGTM – Clean option derivation.The automatic derivation of parser options from
source_typesimplifies the API nicely whilst supporting the new Glimmer embedding paths.
59-77: LGTM – Testing constructor is appropriately scoped.Good decision to keep explicit options available for testing via a
pub(crate)constructor.crates/biome_js_parser/src/parse.rs (5)
142-152: LGTM – Correct option derivation.
parse_commonnow correctly usesJsParser::new, which derives options from the source type.
185-195: LGTM – Simplified API surface.The automatic option derivation simplifies the public API nicely.
233-240: LGTM – Consistent with parse_script changes.
265-303: LGTM – Well-structured refactor with testing hooks.The split between
parse(derives options) andparse_with_options(explicit options for tests) is clean and well-documented. The internalparse_common_with_optionshelper avoids duplication.
325-338: LGTM – Correctly updated to use parse_common.
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.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/biome_js_parser/tests/spec_test.rs (1)
194-194: Fix the argument count mismatch.
parseonly accepts 2 arguments(text, source_type), but you're passing 3 arguments includingJsParserOptions. Useparse_with_optionsinstead.Apply this diff:
- let root = parse(code, JsFileSource::ts(), JsParserOptions::default()); + let root = parse_with_options(code, JsFileSource::ts(), JsParserOptions::default());crates/biome_js_parser/src/parser.rs (1)
243-286: Fix compilation errors in test code.The tests still call
JsParser::newwith 3 parameters, but the updated signature only accepts 2 parameters. This will cause compilation failures.Apply this diff to fix all three tests:
#[should_panic( expected = "Marker must either be `completed` or `abandoned` to avoid that children are implicitly attached to a marker's parent." )] fn uncompleted_markers_panic() { let mut parser = JsParser::new( "'use strict'", JsFileSource::default(), - JsParserOptions::default(), ); let _ = parser.start(); // drop the marker without calling complete or abandon } #[test] fn completed_marker_doesnt_panic() { let mut p = JsParser::new( "'use strict'", JsFileSource::default(), - JsParserOptions::default(), ); let m = p.start(); p.expect(JsSyntaxKind::JS_STRING_LITERAL); m.complete(&mut p, JsSyntaxKind::JS_STRING_LITERAL_EXPRESSION); } #[test] fn abandoned_marker_doesnt_panic() { let mut p = JsParser::new( "'use strict'", JsFileSource::default(), - JsParserOptions::default(), ); let m = p.start(); m.abandon(&mut p); }Alternatively, if explicit options are needed for testing, use
with_optionsinstead:let mut parser = JsParser::with_options( "'use strict'", JsFileSource::default(), JsParserOptions::default(), );crates/biome_js_parser/src/parse.rs (1)
142-152: Update docs and tests to match the new parse API (current code won’t compile).The parsing API refactor itself looks good, but a few call sites still assume the old signatures:
parse_js_with_cachenow takes(text, source_type, cache), but the tests at the bottom still passJsParserOptions::default()as a third argument, which will fail to compile.- Several doc examples (for
parse_script,parse_module,parse, andparse_js_with_cache) still showJsParserOptionsbeing passed directly into these functions.I’d suggest at least fixing the tests like this:
- let normal_parse = parse_js_with_cache( - js_code, - JsFileSource::js_module(), - JsParserOptions::default(), - &mut biome_rowan::NodeCache::default(), - ); + let normal_parse = parse_js_with_cache( + js_code, + JsFileSource::js_module(), + &mut biome_rowan::NodeCache::default(), + );And similarly in the
parse_js_with_cachedoc example:-/// let parsed = parse_js_with_cache(source, source_type, JsParserOptions::default(), &mut cache); +/// let parsed = parse_js_with_cache(source, source_type, &mut cache);For the
parse_script/parse_module/parsedocs, either dropJsParserOptionsfrom the examples or switch them to demonstrateparse_with_optionsexplicitly.Also applies to: 185-195, 233-240, 265-272, 274-303, 325-338, 481-553
♻️ Duplicate comments (3)
xtask/coverage/src/symbols/msts.rs (1)
88-88: Fix the syntax error.This line has mismatched parentheses and incorrect call nesting.
parse_with_optionsexpects(&str, JsFileSource, JsParserOptions), not the result ofparse(...).Apply this diff:
- let r = biome_js_parser::parse_with_options(parse(&code, JsFileSource::tsx(), options); + let r = biome_js_parser::parse_with_options(&code, JsFileSource::tsx(), options);xtask/coverage/src/ts/ts_babel.rs (1)
57-57: Fix the syntax error.Incorrect call nesting:
parse_with_optionsexpects(&str, JsFileSource, JsParserOptions), not the result ofparse(...).Apply this diff:
- let result = biome_js_parser::parse_with_options(parse(&self.code, source_type, options); + let result = biome_js_parser::parse_with_options(&self.code, source_type, options);crates/biome_js_formatter/src/generated.rs (1)
2274-2281: Make doctest examples non-compiling or move them out of a generated fileBare code fences default to Rust and will be compiled; the snippets have undefined identifiers and one uses
JsGlimmerTemplate::default()which likely isn’t implemented. In a generated file, this is brittle. Mark the fences as ignored, or relocate these examples tojs/auxiliary/glimmer_template.rsnext to the real rule impl.Suggested minimal patch:
- /// ``` + /// ```rust,ignore @@ - /// ``` + /// ```rust,ignore @@ - /// ``` + /// ```rust,ignoreIf you keep the examples, also avoid
JsGlimmerTemplate::default()(construct a real node or keep it commented).Run to verify assumptions:
#!/bin/bash # 1) Do we implement Default for JsGlimmerTemplate? rg -nP '(?m)^\s*impl\s+Default\s+for\s+JsGlimmerTemplate|derive\([^)]*\bDefault\b' -C2 # 2) Show the three fence sites in generated.rs sed -n '2266,2332p' crates/biome_js_formatter/src/generated.rs | nl -ba | rg -n '```' -n -C1Also applies to: 2300-2304, 2320-2326
🧹 Nitpick comments (4)
crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_whitespace.gjs (1)
1-13: Well-structured whitespace test cases.The test cases correctly cover key whitespace scenarios (space, tab, newline, multiple spaces) in the opening tag. Clear comments and consistent naming make the intent obvious.
For future iterations, consider adding tests for:
- Whitespace before the closing tag (e.g.,
</template >)- Mixed whitespace (spaces + tabs)
- Whitespace between content and tags
These aren't necessary for Phase 1, but would round out coverage.
crates/biome_js_formatter/src/js/auxiliary/glimmer_template.rs (1)
5-18: Verbatim Glimmer template formatting looks good.The rule cleanly passes through
template_token_tokenand matches the “preserve content exactly” goal; a good baseline to extend later if you decide to normalise anything.crates/biome_js_syntax/src/file_source.rs (1)
122-145: Glimmer file-source wiring is consistent with existing embeddings.
EmbeddingKind::Glimmer,JsFileSource::gjs/gts, and the"gjs"/"gts"extension and language‑ID mappings all line up, and the new tests cover the language‑ID path. If you fancy belt‑and‑braces, you could add small tests fortry_from_extension("gjs"/"gts")andJsFileSource::try_fromon.gjs/.gtspaths, but the current changes look sound.Also applies to: 222-233, 345-369, 377-403, 456-477
crates/biome_service/src/file_handlers/vue.rs (1)
14-14: Remove unused import.
JsParserOptionsis no longer used after removing the explicit parser options argument from theparse_js_with_cachecall.Apply this diff:
-use biome_js_parser::{JsParserOptions, parse_js_with_cache}; +use biome_js_parser::parse_js_with_cache;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (21)
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 (56)
.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_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(6 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(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(2 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(0 hunks)crates/biome_service/src/file_handlers/javascript.rs(1 hunks)crates/biome_service/src/file_handlers/mod.rs(1 hunks)crates/biome_service/src/file_handlers/svelte.rs(1 hunks)crates/biome_service/src/file_handlers/vue.rs(1 hunks)crates/biome_test_utils/src/lib.rs(1 hunks)xtask/codegen/js.ungram(4 hunks)xtask/codegen/src/js_kinds_src.rs(2 hunks)xtask/coverage/src/js/test262.rs(2 hunks)xtask/coverage/src/jsx/jsx_babel.rs(2 hunks)xtask/coverage/src/runner.rs(2 hunks)xtask/coverage/src/symbols/msts.rs(1 hunks)xtask/coverage/src/ts/ts_babel.rs(1 hunks)
💤 Files with no reviewable changes (1)
- crates/biome_service/src/file_handlers/astro.rs
🚧 Files skipped from review as they are similar to previous changes (24)
- crates/biome_js_analyze/src/lint/style/use_naming_convention.rs
- crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_with_content.gjs
- crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_class.gjs
- xtask/codegen/src/js_kinds_src.rs
- crates/biome_js_syntax/src/parentheses/expression.rs
- crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_typescript.gts
- crates/biome_test_utils/src/lib.rs
- xtask/coverage/src/js/test262.rs
- xtask/coverage/src/runner.rs
- crates/biome_js_analyze/src/lint/nursery/no_unused_expressions.rs
- crates/biome_js_parser/tests/spec_tests.rs
- crates/biome_js_parser/src/options.rs
- crates/biome_js_formatter/src/utils/format_node_without_comments.rs
- crates/biome_js_parser/src/syntax/stmt.rs
- crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_yield_await.gjs
- crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_incomplete_less_than.gjs
- crates/biome_js_parser/src/syntax/class.rs
- crates/biome_service/src/file_handlers/javascript.rs
- xtask/coverage/src/jsx/jsx_babel.rs
- crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_incomplete_templat.gjs
- xtask/codegen/js.ungram
- crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_template_unclosed.gjs
- crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_default_export.gjs
- crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_lookalike_templatex.gjs
🧰 Additional context used
🧠 Learnings (1)
📚 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
🧬 Code graph analysis (13)
crates/biome_service/src/file_handlers/vue.rs (4)
crates/biome_service/src/file_handlers/javascript.rs (1)
parse(539-554)crates/biome_service/src/file_handlers/svelte.rs (2)
parse(123-141)file_source(67-80)crates/biome_service/src/file_handlers/astro.rs (1)
parse(109-129)crates/biome_js_parser/src/parse.rs (1)
parse_js_with_cache(328-338)
crates/biome_js_parser/src/token_source.rs (2)
crates/biome_js_parser/src/lexer/mod.rs (2)
from_str(306-319)source(148-150)crates/biome_js_parser/src/options.rs (1)
from(63-76)
crates/biome_js_parser/tests/spec_test.rs (2)
crates/biome_js_parser/src/parse.rs (4)
parse(189-193)parse(237-238)parse(269-272)parse_with_options(279-290)xtask/coverage/src/runner.rs (1)
parse(87-89)
crates/biome_js_syntax/src/file_source.rs (4)
crates/biome_service/src/file_handlers/mod.rs (1)
try_from_language_id(212-232)crates/biome_html_syntax/src/file_source.rs (1)
try_from_language_id(132-140)crates/biome_service/src/file_handlers/svelte.rs (1)
file_source(67-80)crates/biome_service/src/file_handlers/vue.rs (1)
file_source(67-80)
xtask/coverage/src/symbols/msts.rs (3)
crates/biome_js_parser/src/parse.rs (4)
parse_with_options(279-290)parse(189-193)parse(237-238)parse(269-272)xtask/coverage/src/runner.rs (2)
parse(87-89)code(95-97)crates/biome_js_syntax/src/file_source.rs (1)
tsx(193-195)
crates/biome_js_formatter/src/js/auxiliary/glimmer_template.rs (1)
crates/biome_js_formatter/src/generated.rs (16)
format(28-33)format(66-68)format(82-84)format(95-97)format(124-129)format(162-164)format(178-180)format(207-212)format(241-246)format(279-281)format(311-316)format(349-354)format(387-389)format(419-424)format(457-462)format(495-497)
crates/biome_service/src/file_handlers/svelte.rs (2)
crates/biome_service/src/file_handlers/javascript.rs (1)
parse(539-554)crates/biome_js_parser/src/parse.rs (1)
parse_js_with_cache(328-338)
crates/biome_js_parser/src/lexer/mod.rs (3)
crates/biome_js_syntax/src/file_source.rs (1)
default(28-30)crates/biome_js_parser/src/parser.rs (4)
with_options(63-77)options(83-85)source_type(91-93)new(46-57)crates/biome_js_parser/src/token_source.rs (1)
new(21-26)
crates/biome_js_formatter/src/generated.rs (4)
crates/biome_js_formatter/src/js/any/class_member.rs (1)
fmt(9-31)crates/biome_js_formatter/src/js/any/expression.rs (1)
fmt(9-51)crates/biome_js_formatter/src/js/any/statement.rs (1)
fmt(9-46)crates/biome_js_formatter/src/utils/format_node_without_comments.rs (1)
fmt(51-156)
crates/biome_js_parser/src/parser.rs (3)
crates/biome_js_parser/src/token_source.rs (2)
new(21-26)from_str(33-48)crates/biome_js_parser/src/lexer/mod.rs (3)
source(148-150)from_str(306-319)with_options(321-323)crates/biome_js_parser/src/options.rs (1)
from(63-76)
crates/biome_js_parser/src/parse.rs (3)
crates/biome_js_parser/src/parser.rs (4)
new(46-57)source_type(91-93)options(83-85)with_options(63-77)xtask/coverage/src/runner.rs (3)
new(131-133)new(196-201)parse(87-89)crates/biome_service/src/file_handlers/javascript.rs (1)
parse(539-554)
crates/biome_grit_patterns/src/grit_js_parser.rs (2)
crates/biome_js_parser/src/parse.rs (4)
parse(189-193)parse(237-238)parse(269-272)parse_with_options(279-290)xtask/coverage/src/runner.rs (1)
parse(87-89)
xtask/coverage/src/ts/ts_babel.rs (2)
crates/biome_js_parser/src/parse.rs (4)
parse_with_options(279-290)parse(189-193)parse(237-238)parse(269-272)xtask/coverage/src/runner.rs (1)
parse(87-89)
🔇 Additional comments (27)
crates/biome_js_syntax/src/union_ext.rs (1)
100-100: LGTM! Consistent handling of non-named class members.Returning
Ok(None)forJsGlimmerTemplateis correct—templates in class bodies don't have traditional member names, just like static initialization blocks and empty members.crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_template_self_closing.gjs (1)
1-3: Error fixture looks spot onNice focused case for a self‑closing
<template />being rejected; this should keep the lexer/parser honest about requiring explicit closing tags.crates/biome_service/src/file_handlers/svelte.rs (1)
135-140: parse_js_with_cache wiring matches new APIUsing
parse_js_with_cache(script, file_source, cache)here aligns with the centralised “derive options from JsFileSource” approach and keeps Svelte in step with the JavaScript handler.crates/biome_js_syntax/src/expr_ext.rs (1)
908-918: Glimmer templates as primary expressions makes senseAdding
JsGlimmerTemplateto the Primary precedence bucket is the right call so operators around templates associate correctly, matching how JSX tags are treated.crates/biome_js_analyze/src/lint/correctness/no_invalid_constructor_super.rs (1)
223-237: Correctly treating Glimmer templates as non‑constructorsIncluding
AnyJsExpression::JsGlimmerTemplate(_)in theSome(false)arm keepsnoInvalidConstructorSuperconservative: Glimmer templates can’t sensibly be used as constructors, so treating them like other non‑constructible expressions is appropriate.crates/biome_js_parser/src/syntax/expr.rs (3)
1218-1255: Lookahead now correctly recognises Glimmer templates (and null) as expression startsExtending
is_nth_at_expressionto include bothNULL_KWandGLIMMER_TEMPLATEkeeps yield and other lookahead‑driven paths in sync with whatparse_primary_expressioncan actually handle. That should avoid odd corner cases likeyield <template>…</template>oryield nullmis‑classifying as argument‑less yield.
1265-1415: Primary‑expression branch for GLIMMER_TEMPLATE is well‑placedRouting
GLIMMER_TEMPLATEthroughparse_primary_expression(right next to other primary forms and ahead of the contextual‑keyword fallback) fits the existing structure and keeps Glimmer templates available wherever an expression is expected in .gjs/.gts files.
2176-2184: parse_glimmer_template helper is appropriately minimalThe dedicated
parse_glimmer_templatejust wraps the singleGLIMMER_TEMPLATEtoken into aJS_GLIMMER_TEMPLATEnode, leaving all the heavy lifting to the lexer and formatter, which is exactly what you want for an opaque template block..changeset/glimmer-template-support.md (1)
1-20: LGTM! Clear documentation.The changeset provides excellent user-facing documentation with appropriate caveats about Phase 1 limitations. Well done.
crates/biome_grit_patterns/src/grit_target_language/js_target_language/generated_mappings.rs (1)
301-301: LGTM! Generated mapping follows established pattern.The JsGlimmerTemplate mapping is consistent with other node type mappings in this generated file.
crates/biome_js_formatter/src/js/any/expression.rs (1)
23-23: LGTM! Generated formatter dispatch.The JsGlimmerTemplate formatting follows the established pattern for expression variants.
crates/biome_js_formatter/src/js/any/statement.rs (1)
24-24: LGTM! Generated formatter dispatch.The JsGlimmerTemplate statement formatting follows the established pattern.
crates/biome_js_formatter/src/js/classes/property_class_member.rs (1)
137-137: LGTM! Correct semicolon and modifier handling.The placement of JsGlimmerTemplate in both functions is semantically correct:
- No semicolon needed (like getters/setters/constructors)
- No modifiers (templates can't be static/async/etc.)
Also applies to: 174-174
crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_lookalike_div.gjs (1)
1-3: LGTM! Clear test case.The test clearly documents its intent and provides a proper error case for JSX usage in .gjs files.
crates/biome_service/src/file_handlers/mod.rs (2)
820-820: LGTM! Appropriate for Phase 1.Treating Glimmer files with standard JS capabilities is correct for Phase 1, where templates are opaque tokens. Future phases may require dedicated handling.
869-869: LGTM! Aligned with parser API changes.The removal of explicit
JsParserOptionsis consistent with the API changes seen throughout this PR.crates/biome_js_formatter/src/js/any/class_member.rs (1)
15-15: LGTM!Correctly routes the new Glimmer template variant through its formatter, consistent with other class member formatting.
crates/biome_js_formatter/src/js/auxiliary/mod.rs (1)
14-14: LGTM!Properly exposes the new Glimmer template formatter module, following the established pattern.
crates/biome_js_parser/tests/js_test_suite/error/glimmer/glimmer_lookalike_temp.gjs (1)
1-3: Good negative test case.Ensures the parser correctly rejects lookalike tags and exercises the lexer backtracking logic.
crates/biome_js_parser/tests/js_test_suite/ok/glimmer/glimmer_template_simple.gjs (1)
1-5: LGTM!Solid test coverage for templates as expressions and exported templates, aligning well with the PR's phase 1 objectives.
crates/biome_js_parser/tests/spec_test.rs (1)
85-91: LGTM!Clean conditional logic to use option-driven parsing when test-specific configuration is present, with sensible fallback to default parsing.
crates/biome_js_parser/src/token_source.rs (1)
28-48: Nice refactoring!Properly derives parser options from file source type, following the pattern from other language parsers. This addresses the previous feedback and ensures consistent configuration based on file characteristics.
Based on learnings
crates/biome_js_parser/src/parser.rs (2)
43-57: LGTM! Clean API simplification.The automatic option derivation from
source_typesimplifies the public API and ensures consistency with howJsTokenSourceis constructed. The documentation clearly explains the behaviour.
59-77: LGTM! Sensible test harness.Keeping the explicit options constructor for testing whilst simplifying the public API is a good design choice.
crates/biome_grit_patterns/src/grit_js_parser.rs (1)
5-5: Nice adaptation toparse_with_optionsfor Grit metavariables.Using
parse_with_options(... JsFileSource::tsx(), JsParserOptions::default().with_metavariables())inparse_file/parse_snippetkeeps the old behaviour despite theparseAPI change, whileparse_with_pathcorrectly relies on option derivation fromJsFileSource. Nothing to fix here.Also applies to: 51-55, 78-82
crates/biome_js_parser/src/lexer/mod.rs (1)
136-138: Source type properly wired; Glimmer gating looks sound.
JsTokenSource::from_strcorrectly applies.with_source_type(source_type)to the lexer (token_source.rs:42), ensuring the Glimmer-aware behaviour is active when needed. The gating itself is tied tosource_type.as_embedding_kind().is_glimmer()(file characteristics), not to parser options, which keeps it orthogonal to optional settings—exactly as the lexer should work.crates/biome_service/src/file_handlers/vue.rs (1)
135-140: LGTM!The changes correctly align with the refactored
parse_js_with_cacheAPI and follow the same pattern as the Svelte and Astro handlers. Parser options are now derived from the file source rather than passed explicitly.
crates/biome_js_analyze/src/lint/complexity/no_static_only_class.rs
Outdated
Show resolved
Hide resolved
…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.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/biome_js_formatter/src/generated.rs (1)
2321-2325: AvoidJsGlimmerTemplate::default()in the exampleEven with
rust,ignore, showing::default()implies it exists. Prefer a placeholder or parsed node reference, e.g.:-/// let node = JsGlimmerTemplate::default(); -/// let owned_formatter = node.into_format(); +/// // Assuming `node: JsGlimmerTemplate` parsed from a .gjs/.gts file +/// let owned_formatter = node.into_format();As this file is generated, please tweak the codegen or move the example to
js/auxiliary/glimmer_template.rs.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
crates/biome_js_analyze/tests/plugin/preferObjectSpread.grit.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/plugin/preferObjectSpreadSuppression.grit.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/complexity/noStaticOnlyClass/valid.gjs.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (13)
crates/biome_js_analyze/benches/js_analyzer.rs(1 hunks)crates/biome_js_analyze/src/ast_utils.rs(1 hunks)crates/biome_js_analyze/src/lint/complexity/no_static_only_class.rs(1 hunks)crates/biome_js_analyze/src/react/components.rs(1 hunks)crates/biome_js_analyze/src/react/hooks.rs(4 hunks)crates/biome_js_analyze/src/services/semantic_class.rs(2 hunks)crates/biome_js_analyze/src/suppressions.tests.rs(20 hunks)crates/biome_js_analyze/src/utils.rs(4 hunks)crates/biome_js_analyze/src/utils/tests.rs(5 hunks)crates/biome_js_analyze/tests/quick_test.rs(3 hunks)crates/biome_js_analyze/tests/spec_tests.rs(4 hunks)crates/biome_js_analyze/tests/specs/complexity/noStaticOnlyClass/valid.gjs(1 hunks)crates/biome_js_formatter/src/generated.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/biome_js_analyze/src/utils/tests.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_js_analyze/src/lint/complexity/no_static_only_class.rs
🧰 Additional context used
🧬 Code graph analysis (10)
crates/biome_js_analyze/src/ast_utils.rs (2)
crates/biome_js_parser/src/parse.rs (1)
parse_with_options(278-289)crates/biome_js_syntax/src/file_source.rs (1)
tsx(193-195)
crates/biome_js_analyze/src/react/components.rs (2)
crates/biome_js_parser/src/parse.rs (1)
parse_with_options(278-289)crates/biome_js_syntax/src/file_source.rs (1)
jsx(173-175)
crates/biome_js_analyze/src/react/hooks.rs (1)
crates/biome_js_parser/src/parse.rs (1)
parse_with_options(278-289)
crates/biome_js_analyze/tests/quick_test.rs (1)
crates/biome_js_parser/src/parse.rs (1)
parse_with_options(278-289)
crates/biome_js_analyze/src/services/semantic_class.rs (2)
crates/biome_js_parser/src/parse.rs (1)
parse_with_options(278-289)crates/biome_js_syntax/src/file_source.rs (1)
ts(178-185)
crates/biome_js_analyze/src/utils.rs (1)
crates/biome_js_parser/src/parse.rs (1)
parse_with_options(278-289)
crates/biome_js_analyze/benches/js_analyzer.rs (1)
crates/biome_js_parser/src/parse.rs (1)
parse_with_options(278-289)
crates/biome_js_analyze/src/suppressions.tests.rs (2)
crates/biome_js_parser/src/parse.rs (1)
parse_with_options(278-289)crates/biome_js_syntax/src/file_source.rs (2)
tsx(193-195)ts(178-185)
crates/biome_js_formatter/src/generated.rs (4)
crates/biome_js_formatter/src/js/any/statement.rs (1)
fmt(9-46)crates/biome_js_formatter/src/js/any/class_member.rs (1)
fmt(9-31)crates/biome_js_formatter/src/js/any/expression.rs (1)
fmt(9-51)crates/biome_js_formatter/src/utils/format_node_without_comments.rs (1)
fmt(51-156)
crates/biome_js_analyze/tests/spec_tests.rs (1)
crates/biome_js_parser/src/parse.rs (4)
parse(188-192)parse(236-237)parse(268-271)parse_with_options(278-289)
🔇 Additional comments (12)
crates/biome_js_analyze/src/suppressions.tests.rs (1)
5-5: LGTM! Clean mechanical refactor.The systematic migration from
parsetoparse_with_optionswithJsParserOptions::default()is correct and preserves existing test behaviour. Using default options for suppression tests is appropriate since they don't require Glimmer-specific parser configuration.Also applies to: 15-15, 73-77, 155-159, 224-228, 267-271, 318-322, 364-368, 412-416, 461-465, 508-512, 553-557, 601-605, 648-652, 698-702, 751-755, 797-797, 843-847, 887-891, 929-933
crates/biome_js_analyze/benches/js_analyzer.rs (1)
50-50: LGTM!The switch to
parse_with_optionswith default options is correct and maintains existing behaviour whilst enabling the new parsing pathway.crates/biome_js_analyze/src/utils.rs (1)
86-86: LGTM!All test parsing calls correctly updated to use
parse_with_options. The changes are consistent and maintain existing test behaviour.Also applies to: 93-97, 116-120, 139-143, 162-166
crates/biome_js_analyze/tests/specs/complexity/noStaticOnlyClass/valid.gjs (1)
1-19: LGTM!Excellent test coverage for Glimmer templates in the
noStaticOnlyClassrule. The test correctly validates that<template>blocks are treated as non-static members, preventing false positives.crates/biome_js_analyze/src/react/hooks.rs (1)
340-344: LGTM!Test parsing calls consistently updated to
parse_with_options. The changes maintain test behaviour whilst adopting the new parsing API.Also applies to: 355-359, 373-380, 403-410
crates/biome_js_analyze/src/ast_utils.rs (1)
361-361: LGTM!Test helper correctly updated to use
parse_with_options, maintaining consistency across the test suite.crates/biome_js_analyze/tests/quick_test.rs (1)
9-9: LGTM!Quick test functions correctly updated to use
parse_with_options. The changes are consistent with the repo-wide migration pattern.Also applies to: 41-41, 113-113
crates/biome_js_analyze/tests/spec_tests.rs (2)
26-28: LGTM!Test discovery correctly expanded to include
.gjsand.gtsGlimmer template files. This enables comprehensive test coverage for the new Glimmer support.
8-8: LGTM!Parsing calls correctly migrated to
parse_with_options. The changes maintain existing test behaviour whilst enabling option-aware parsing.Also applies to: 179-179, 345-345
crates/biome_js_analyze/src/react/components.rs (1)
669-669: LGTM!Test helper
parse_jsxcorrectly updated to useparse_with_options, ensuring all React component tests use the new parsing API consistently.Also applies to: 673-673
crates/biome_js_analyze/src/services/semantic_class.rs (1)
927-927: LGTM!The import correctly reflects the parser API change to
parse_with_options.crates/biome_js_formatter/src/generated.rs (1)
2266-2310: Glimmer wiring looks spot‑onFormatRule/AsFormat plumbing mirrors existing patterns and the doctest fences are safely
rust,ignore. Nice one.
|
|
||
| fn parse_ts(code: &str) -> Parse<AnyJsRoot> { | ||
| let source = parse(code, JsFileSource::ts(), JsParserOptions::default()); | ||
| let source = parse_with_options(code, JsFileSource::ts(), JsParserOptions::default()); |
There was a problem hiding this comment.
🧩 Analysis chain
Function call updated correctly.
The test helper properly uses parse_with_options with the correct parameters. To ensure completeness, verify that no other files in this crate still reference the old parse function.
🏁 Script executed:
#!/bin/bash
# Verify no remaining usages of the old parse function from biome_js_parser
rg -n --type rust 'biome_js_parser::parse[^_]' crates/biome_js_analyze/
rg -n --type rust 'use biome_js_parser::\{[^}]*\bparse\b[^}]*\}' crates/biome_js_analyze/Length of output: 112
---
🏁 Script executed:
```shell
#!/bin/bash
# Find actual usages of the parse function in spec_tests.rs (not just the import)
rg -n --type rust '\bparse\s*\(' crates/biome_js_analyze/tests/spec_tests.rs
Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Check parse usage in the entire crate and semantic_class.rs specifically
rg -n --type rust '\bparse\b' crates/biome_js_analyze/src/services/semantic_class.rs | grep -v parse_with_options
rg -n --type rust 'parse[^_]' crates/biome_js_analyze/src/ -A2 -B2 | head -50
Length of output: 3398
Remove unused parse import from spec_tests.rs.
The migration to parse_with_options at line 987 is correct. However, the old parse function remains imported in crates/biome_js_analyze/tests/spec_tests.rs:8 but isn't actually used—only parse_with_options is in use across the crate.
Update the import to:
use biome_js_parser::{JsParserOptions, parse_with_options};🤖 Prompt for AI Agents
In crates/biome_js_analyze/src/services/semantic_class.rs around line 987 and in
crates/biome_js_analyze/tests/spec_tests.rs (import block at line 8), the code
migrated to parse_with_options but the old parse import is still present and
unused; remove the unused parse import and update the import line to import
JsParserOptions and parse_with_options (i.e., replace the current
biome_js_parser import with one that lists JsParserOptions and
parse_with_options) so the test file only imports what it actually uses.
3 participants
Disclaimer
This PR was primarily written with Claude Code. However, I reviewed the plan in-depth, modifying as appropriate. I have also reviewed all of the code personally.
Summary
This PR implements Phase 1 of Glimmer (Ember.js) 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:
Templates are now recognized in:
const Tpl = <template>...</template>;class C { <template>...</template> }The lexer emits helpful diagnostics for unclosed templates.
Discussion
https://discord.com/channels/1132231889290285117/1433896814917980202
Test Plan
There are extensive automated tests as part of this.
Docs
TODO