fix(core): parsing of svelte snippets#10075
Conversation
🦋 Changeset detectedLatest commit: e2634b1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Parser conformance results onjs/262
jsx/babel
markdown/commonmark
symbols/microsoft
ts/babel
ts/microsoft
|
WalkthroughThis PR implements parser support for Svelte Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/sharp-jeans-reply.md:
- Line 5: Update the user-facing changeset sentence "Fixed
[`#9983`](https://github.com/biomejs/biome/issues/9983): now Biome parsers
functions declared inside the Svelte `#snippet` block withour throwing errors."
to a concise, user-facing 1–3 sentence phrasing that fixes the typos and grammar
(replace "parsers" and "withour"), e.g. "Fixed `#9983`: Biome parser functions
declared inside Svelte `#snippet` blocks no longer throw errors." Ensure the
edited text replaces the exact sentence in the .changeset entry.
In `@crates/biome_js_parser/src/syntax/program.rs`:
- Around line 117-124: The parameter list for Svelte snippet headers is being
parsed optionally (parse_parameter_list(...).ok()), which suppresses the
missing-`()` diagnostic and can leave JS_SNIPPET_SIGNATURE_TEMPLATE_ROOT without
its parameters child; change this to a required parse by replacing the trailing
.ok() with .or_add_diagnostic(p, <missing-parens-diagnostic>) so
parse_parameter_list(p, ParameterContext::Declaration, TypeContext::default(),
SignatureFlags::empty()).or_add_diagnostic(p, <missing-parens-diagnostic>) is
used, ensuring the parser emits the missing-`()` error and the
JS_SNIPPET_SIGNATURE_TEMPLATE_ROOT always gets a parameters node.
In `@crates/biome_service/src/file_handlers/html/parse_embedded_nodes.tests.rs`:
- Around line 52-69: Add a new test alongside snippet_svelte_ts_parsing that
covers a malformed snippet signature (e.g. missing a comma in "{`#snippet` add(a:
any b: float)}") by creating a new test function (e.g.
snippet_svelte_ts_malformed_parsing) with FILE_PATH/FILE_CONTENT similar to the
existing test but containing the bad signature, and replace the assertion
assert_no_diagnostics with an assertion that expects diagnostics (use the
existing project helper that asserts errors are emitted — e.g.
assert_has_diagnostics or the equivalent test helper in this test module) so the
parser's recovery path is exercised and the test fails if no diagnostic is
produced.
In `@crates/biome_service/src/workspace/document/services/embedded_bindings.rs`:
- Around line 104-134: Add a test that actually parses a Svelte snippet header
so the visitor path that constructs JsSnippetSignatureTemplateRoot is exercised:
modify or add a unit/integration test that uses the snippet-header helper but
ensures the input is parsed as EmbeddingKind::Svelte { is_function_signature:
true } (instead of using JsFileSource::ts()), then assert
visit_svelte_snippet_declaration (or the surrounding registration behavior)
registers the expected bindings; target the JsSnippetSignatureTemplateRoot and
visit_svelte_snippet_declaration code paths so typed snippet parameters and the
Svelte snippet header parsing are exercised and the regression is prevented.
In `@crates/biome_service/src/workspace/server.rs`:
- Around line 616-637: The helper currently treats missing or failed parse
(doc.syntax == None or Err(FileTooLarge)) as an empty diagnostics list; instead
call and mirror get_parse(path) to propagate the same errors (not_found /
file_ignored / file-too-large) rather than returning Ok([]). In
get_parse_diagnostics use self.get_parse(path) (or the same matching logic
get_parse uses) to obtain the parsed AST (propagating its WorkspaceError on
None/Err), then collect parse.diagnostics() plus embedded_snippets diagnostics
and return them; do not collapse None/Err into an empty Vec.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 115ae08b-8de7-42e3-a579-363b964a62a1
⛔ Files ignored due to path filters (6)
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_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**
📒 Files selected for processing (22)
.changeset/sharp-jeans-reply.mdcrates/biome_grit_patterns/src/grit_target_language/js_target_language/generated_mappings.rscrates/biome_js_formatter/src/comments.rscrates/biome_js_formatter/src/generated.rscrates/biome_js_formatter/src/js/any/root.rscrates/biome_js_formatter/src/js/auxiliary/mod.rscrates/biome_js_formatter/src/js/auxiliary/snippet_signature_template_root.rscrates/biome_js_parser/src/syntax/program.rscrates/biome_js_parser/tests/spec_test.rscrates/biome_js_semantic/src/events.rscrates/biome_js_semantic/src/semantic_model/builder.rscrates/biome_js_syntax/src/file_source.rscrates/biome_service/src/embed/registry.rscrates/biome_service/src/embed/types.rscrates/biome_service/src/file_handlers/html/parse_embedded_nodes.rscrates/biome_service/src/file_handlers/html/parse_embedded_nodes.tests.rscrates/biome_service/src/file_handlers/svelte.rscrates/biome_service/src/workspace/document/mod.rscrates/biome_service/src/workspace/document/services/embedded_bindings.rscrates/biome_service/src/workspace/server.rsxtask/codegen/js.ungramxtask/codegen/src/js_kinds_src.rs
Merging this PR will not alter performance
Comparing Footnotes
|
dyc3
left a comment
There was a problem hiding this comment.
On mobile right now, so I haven't gotten to test it, but looks good
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/biome_js_parser/src/syntax/program.rs (1)
117-117: Tiny clarity nit: this comment contradicts the behaviour.“These are not mandatory” clashes with the
or_add_diagnostic(...)call right below. Worth updating so future readers don’t have to squint.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_js_parser/src/syntax/program.rs` at line 117, The comment "These are not mandatory" is misleading because the parser immediately calls or_add_diagnostic(...) when they're missing; update the comment near the or_add_diagnostic(...) invocation to clarify that the fields are optional but the parser will emit a diagnostic/error when absent (e.g., "These fields are optional in the grammar, but a diagnostic is added if they are missing"). Locate the comment adjacent to the or_add_diagnostic(...) call in program.rs and replace it with a clear note that reflects the actual behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_service/src/workspace/document/services/embedded_bindings.rs`:
- Around line 121-129: The current loop only extracts bindings from
JsFormalParameter via param.as_any_js_formal_parameter(), so rest parameters and
other parameter variants that also carry a binding are missed; update the loop
over root.parameters().items() to attempt extracting the binding from every
parameter variant that exposes one (e.g., check
param.as_any_js_formal_parameter() and param.as_any_js_rest_parameter() /
corresponding as_js_rest_parameter() accessors or any other parameter variants
in this AST that have a binding method), and for each successful extraction call
self.visit_any_js_binding_pattern(&binding) so all parameter bindings (including
rest params) are registered.
---
Nitpick comments:
In `@crates/biome_js_parser/src/syntax/program.rs`:
- Line 117: The comment "These are not mandatory" is misleading because the
parser immediately calls or_add_diagnostic(...) when they're missing; update the
comment near the or_add_diagnostic(...) invocation to clarify that the fields
are optional but the parser will emit a diagnostic/error when absent (e.g.,
"These fields are optional in the grammar, but a diagnostic is added if they are
missing"). Locate the comment adjacent to the or_add_diagnostic(...) call in
program.rs and replace it with a clear note that reflects the actual behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d4f804f3-0288-465f-8108-63ab502474a3
⛔ Files ignored due to path filters (7)
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_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 (14)
.changeset/sharp-jeans-reply.mdcrates/biome_grit_patterns/src/grit_target_language/js_target_language/generated_mappings.rscrates/biome_js_formatter/src/comments.rscrates/biome_js_formatter/src/generated.rscrates/biome_js_formatter/src/js/any/root.rscrates/biome_js_formatter/src/js/auxiliary/mod.rscrates/biome_js_formatter/src/js/auxiliary/svelte_snippet_root.rscrates/biome_js_parser/src/syntax/program.rscrates/biome_js_semantic/src/events.rscrates/biome_js_semantic/src/semantic_model/builder.rscrates/biome_service/src/file_handlers/html/parse_embedded_nodes.tests.rscrates/biome_service/src/workspace/document/services/embedded_bindings.rsxtask/codegen/js.ungramxtask/codegen/src/js_kinds_src.rs
✅ Files skipped from review due to trivial changes (4)
- .changeset/sharp-jeans-reply.md
- crates/biome_js_formatter/src/js/any/root.rs
- crates/biome_js_formatter/src/js/auxiliary/mod.rs
- xtask/codegen/src/js_kinds_src.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- crates/biome_js_formatter/src/comments.rs
- crates/biome_grit_patterns/src/grit_target_language/js_target_language/generated_mappings.rs
- xtask/codegen/js.ungram
- crates/biome_js_semantic/src/events.rs
- crates/biome_js_formatter/src/generated.rs
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [@biomejs/biome](https://biomejs.dev) ([source](https://github.com/biomejs/biome/tree/HEAD/packages/@biomejs/biome)) | patch | `2.4.12` → `2.4.13` | --- ### Release Notes <details> <summary>biomejs/biome (@​biomejs/biome)</summary> ### [`v2.4.13`](https://github.com/biomejs/biome/blob/HEAD/packages/@​biomejs/biome/CHANGELOG.md#2413) [Compare Source](https://github.com/biomejs/biome/compare/@biomejs/biome@2.4.12...@biomejs/biome@2.4.13) ##### Patch Changes - [#​9969](biomejs/biome#9969) [`c5eb92b`](biomejs/biome@c5eb92b) Thanks [@​officialasishkumar](https://github.com/officialasishkumar)! - Added the nursery rule [`noUnnecessaryTemplateExpression`](https://biomejs.dev/linter/rules/no-unnecessary-template-expression/), which disallows template literals that only contain string literal expressions. These can be replaced with a simpler string literal. For example, the following code triggers the rule: ```js const a = `${"hello"}`; // can be 'hello' const b = `${"prefix"}_suffix`; // can be 'prefix_suffix' const c = `${"a"}${"b"}`; // can be 'ab' ``` - [#​10037](biomejs/biome#10037) [`f785e8c`](biomejs/biome@f785e8c) Thanks [@​minseong0324](https://github.com/minseong0324)! - Fixed [#​9810](biomejs/biome#9810): [`noMisleadingReturnType`](https://biomejs.dev/linter/rules/no-misleading-return-type/) no longer reports false positives on a getter with a matching setter in the same namespace. ```ts class Store { get status(): string { if (Math.random() > 0.5) return "loading"; return "idle"; } set status(v: string) {} } ``` - [#​10084](biomejs/biome#10084) [`5e2f90c`](biomejs/biome@5e2f90c) Thanks [@​jiwon79](https://github.com/jiwon79)! - Fixed [#​10034](biomejs/biome#10034): [`noUselessEscapeInRegex`](https://biomejs.dev/linter/rules/no-useless-escape-in-regex/) no longer flags escapes of `ClassSetReservedPunctuator` characters (`&`, `!`, `#`, `%`, `,`, `:`, `;`, `<`, `=`, `>`, `@`, `` ` ``, `~`) inside `v`-flag character classes as useless. These characters are reserved as individual code points in `v`-mode, so the escape is required. The following pattern is now considered valid: ```js /[a-z\&]/v; ``` - [#​10063](biomejs/biome#10063) [`c9ffa16`](biomejs/biome@c9ffa16) Thanks [@​Netail](https://github.com/Netail)! - Added extra rule sources from ESLint CSS. `biome migrate eslint` should do a bit better detecting rules in your eslint configurations. - [#​10035](biomejs/biome#10035) [`946b50e`](biomejs/biome@946b50e) Thanks [@​Netail](https://github.com/Netail)! - Fixed [#​10032](biomejs/biome#10032): [useIframeSandbox](https://biomejs.dev/linter/rules/use-iframe-sandbox/) now flags if there's no initializer value. - [#​9865](biomejs/biome#9865) [`68fb8d4`](biomejs/biome@68fb8d4) Thanks [@​dyc3](https://github.com/dyc3)! - Added the new nursery rule [`useDomNodeTextContent`](https://biomejs.dev/linter/rules/use-dom-node-text-content/), which prefers `textContent` over `innerText` for DOM node text access and destructuring. For example, the following snippet triggers the rule: ```js const foo = node.innerText; ``` - [#​10023](biomejs/biome#10023) [`bd1e74f`](biomejs/biome@bd1e74f) Thanks [@​ematipico](https://github.com/ematipico)! - Added a new nursery rule [`noReactNativeDeepImports`](https://biomejs.dev/linter/rules/no-react-native-deep-imports/) that disallows deep imports from the `react-native` package. Internal paths like `react-native/Libraries/...` are not part of the public API and may change between versions. For example, the following code triggers the rule: ```js import View from "react-native/Libraries/Components/View/View"; ``` - [#​9885](biomejs/biome#9885) [`3dce737`](biomejs/biome@3dce737) Thanks [@​dyc3](https://github.com/dyc3)! - Added a new nursery rule [`useDomQuerySelector`](https://biomejs.dev/linter/rules/use-dom-query-selector/) that prefers `querySelector()` and `querySelectorAll()` over older DOM query methods such as `getElementById()` and `getElementsByClassName()`. - [#​9995](biomejs/biome#9995) [`4da9caf`](biomejs/biome@4da9caf) Thanks [@​siketyan](https://github.com/siketyan)! - Fixed [#​9994](biomejs/biome#9994): Biome now parses nested CSS rules correctly when declarations follow them inside embedded snippets. - [#​10009](biomejs/biome#10009) [`b41cc5a`](biomejs/biome@b41cc5a) Thanks [@​Jayllyz](https://github.com/Jayllyz)! - Fixed [#​10004](biomejs/biome#10004): [`noComponentHookFactories`](https://biomejs.dev/linter/rules/no-component-hook-factories/) no longer reports false positives for object methods and class methods. - [#​9988](biomejs/biome#9988) [`eabf54a`](biomejs/biome@eabf54a) Thanks [@​Netail](https://github.com/Netail)! - Tweaked the diagnostics range for [useAltText](https://biomejs.dev/linter/rules/use-alt-text), [useButtonType](https://biomejs.dev/linter/rules/use-button-type), [useHtmlLang](https://biomejs.dev/linter/rules/use-html-lang), [useIframeTitle](https://biomejs.dev/linter/rules/use-iframe-title), [useValidAriaRole](https://biomejs.dev/linter/rules/use-valid-aria-role) & [useIfameSandbox](https://biomejs.dev/linter/rules/use-iframe-sandbox) to report on the opening tag instead of the full tag. - [#​10043](biomejs/biome#10043) [`fc65902`](biomejs/biome@fc65902) Thanks [@​mujpao](https://github.com/mujpao)! - Fixed [#​10003](biomejs/biome#10003): Biome no longer panics when parsing Svelte files containing `{#}`. - [#​9815](biomejs/biome#9815) [`5cc83b1`](biomejs/biome@5cc83b1) Thanks [@​dyc3](https://github.com/dyc3)! - Added the new nursery rule [`noLoopFunc`](https://biomejs.dev/linter/rules/no-loop-func/). When enabled, it warns when a function declared inside a loop captures outer variables that can change across iterations. - [#​9702](biomejs/biome#9702) [`ef470ba`](biomejs/biome@ef470ba) Thanks [@​ryan-m-walker](https://github.com/ryan-m-walker)! - Added the nursery rule [`useRegexpTest`](https://biomejs.dev/linter/rules/use-regexp-test/) that enforces `RegExp.prototype.test()` over `String.prototype.match()` and `RegExp.prototype.exec()` in boolean contexts. `test()` returns a boolean directly, avoiding unnecessary computation of match results. **Invalid** ```js if ("hello world".match(/hello/)) { } ``` **Valid** ```js if (/hello/.test("hello world")) { } ``` - [#​9743](biomejs/biome#9743) [`245307d`](biomejs/biome@245307d) Thanks [@​leetdavid](https://github.com/leetdavid)! - Fixed [#​2245](biomejs/biome#2245): Svelte `<script>` tag language detection when the `generics` attribute contains `>` characters (e.g., `<script lang="ts" generics="T extends Record<string, unknown>">`). Biome now correctly recognizes TypeScript in such script blocks. - [#​10046](biomejs/biome#10046) [`0707de7`](biomejs/biome@0707de7) Thanks [@​Conaclos](https://github.com/Conaclos)! - Fixed [#​10038](biomejs/biome#10038): [`organizeImports`](https://biomejs.dev/assist/actions/organize-imports/) now sorts imports in TypeScript modules and declaration files. ```diff declare module "mymodule" { - import type { B } from "b"; import type { A } from "a"; + import type { B } from "b"; } ``` - [#​10012](biomejs/biome#10012) [`94ccca9`](biomejs/biome@94ccca9) Thanks [@​ematipico](https://github.com/ematipico)! - Added the nursery rule [`noReactNativeLiteralColors`](https://biomejs.dev/linter/rules/no-react-native-literal-colors/), which disallows color literals inside React Native styles. The rule belongs to the `reactNative` domain. It reports properties whose name contains `color` and whose value is a string literal when they appear inside a `StyleSheet.create(...)` call or inside a JSX attribute whose name contains `style`. ```jsx // Invalid const Hello = () => <Text style={{ backgroundColor: "#FFFFFF" }}>hi</Text>; const styles = StyleSheet.create({ text: { color: "red" }, }); ``` ```jsx // Valid const red = "#f00"; const styles = StyleSheet.create({ text: { color: red }, }); ``` - [#​10005](biomejs/biome#10005) [`131019e`](biomejs/biome@131019e) Thanks [@​ematipico](https://github.com/ematipico)! - Added the nursery rule [`noReactNativeRawText`](https://biomejs.dev/linter/rules/no-react-native-raw-text/), which disallows raw text outside of `<Text>` components in React Native. The rule belongs to the new `reactNative` domain. ```jsx // Invalid <View>some text</View> <View>{'some text'}</View> ``` ```jsx // Valid <View> <Text>some text</Text> </View> ``` Additional components can be allowlisted through the `skip` option: ```json { "options": { "skip": ["Title"] } } ``` - [#​9911](biomejs/biome#9911) [`1603f78`](biomejs/biome@1603f78) Thanks [@​Netail](https://github.com/Netail)! - Added the nursery rule [`noJsxLeakedDollar`](https://biomejs.dev/linter/rules/no-jsx-leaked-dollar), which flags text nodes with a trailing `$` if the next sibling node is a JSX expression. This could be an unintentional mistake, resulting in a '$' being rendered as text in the output. **Invalid**: ```jsx function MyComponent({ user }) { return <div>Hello ${user.name}</div>; } ``` - [#​9999](biomejs/biome#9999) [`f42405f`](biomejs/biome@f42405f) Thanks [@​minseong0324](https://github.com/minseong0324)! - Fixed `noMisleadingReturnType` incorrectly flagging functions with reassigned `let` variables. - [#​10075](biomejs/biome#10075) [`295f97f`](biomejs/biome@295f97f) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed [`#9983`](biomejs/biome#9983): Biome now parses functions declared inside Svelte `#snippet` blocks without throwing errors. - [#​10006](biomejs/biome#10006) [`cf4c1c9`](biomejs/biome@cf4c1c9) Thanks [@​minseong0324](https://github.com/minseong0324)! - Fixed [#​9810](biomejs/biome#9810): `noMisleadingReturnType` incorrectly flagging nested object literals with widened properties. - [#​10033](biomejs/biome#10033) [`11ddc05`](biomejs/biome@11ddc05) Thanks [@​ematipico](https://github.com/ematipico)! - Added the nursery rule [`useReactNativePlatformComponents`](https://biomejs.dev/linter/rules/use-react-native-platform-components/) that ensures platform-specific React Native components (e.g. `ProgressBarAndroid`, `ActivityIndicatorIOS`) are only imported in files with a matching platform suffix. It also reports when Android and iOS components are mixed in the same file. The following code triggers the rule when the file does not have an `.android.js` suffix: ```js // file.js import { ProgressBarAndroid } from "react-native"; ``` </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMzkuNiIsInVwZGF0ZWRJblZlciI6IjQzLjEzOS42IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Reviewed-on: https://git.oirnoir.dev/OIRNOIR/YouTube-Helper-Server/pulls/1
Summary
Closes #9983
There were two bugs:
<script lang='ts'>imposes that all guest snippets be parsed with TS.Test Plan
I added a new test as a regression, and prepared a small infra so that we can add more cases like this.
Docs