-
-
Notifications
You must be signed in to change notification settings - Fork 724
feat: indentation for vue and svelte files #7330
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: dc1316c 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 |
WalkthroughThe PR updates Svelte and Vue formatting to use biome_js_formatter::format_node with JsLanguage and prints with indent 1. The printer adjusts pending indentation initialisation and moves line-suffix flushing inside the main loop. New indentation-aware tests are added. CLI tests for Svelte/Vue remove strict file-content assertions, relying on snapshots instead. Service handlers for Svelte and Vue switch from javascript::format to the new formatting path with error logging. A changeset documents the formatting behaviour change for script blocks. No public API signatures are modified. Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
baseUrl in tsconfig.json (#7263)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.changeset/dull-drinks-switch.md (1)
5-12: Tighten wording and example; end sentences with full stops.
- Per the repo’s changeset guidelines, end every sentence with a full stop. The first sentence currently ends with a colon.
- The diff example shows three leading spaces. Prettier’s default indent width is two spaces; consider using two spaces or rephrase to “one indent level” to avoid implying a specific width.
Apply this minimal tweak:
-Updated the formatting of `.svelte` and `.vue` files. Now the indentation of the JavaScript blocks matches Prettier's: +Updated the formatting of `.svelte` and `.vue` files. The indentation of the JavaScript blocks now matches Prettier. + ```diff <script> -- import Component from "./Component" -+ import Component from "./Component" +- import Component from "./Component" ++ import Component from "./Component" </script></blockquote></details> <details> <summary>crates/biome_service/src/file_handlers/svelte.rs (1)</summary><blockquote> `1-1`: **Imports look good; minor style nit.** You mixed method and field access for options in this file and related code (e.g., elsewhere it’s options.indent_style() vs options.indent_style). For consistency, prefer the accessor helpers throughout. No need to change now if the project tolerates both. Also applies to: 12-12, 14-14, 19-19 </blockquote></details> <details> <summary>crates/biome_formatter/src/printer/mod.rs (2)</summary><blockquote> `1824-1843`: **Make the test resilient: set indent style explicitly.** The assertion expects tabs. If PrinterOptions::default ever changes, this test may become flaky. Pin the indent style. ```diff - let options = PrinterOptions { - print_width: PrintWidth::new(10), - ..PrinterOptions::default() - }; + let options = PrinterOptions { + indent_style: IndentStyle::Tab, + print_width: PrintWidth::new(10), + ..PrinterOptions::default() + };
1844-1902: Same here: pin indent style in the fill test.Avoid reliance on defaults for the expected “\t” output.
- let printed = Printer::new(PrinterOptions::default().with_print_width(PrintWidth::new(10))) + let printed = Printer::new( + PrinterOptions { + indent_style: IndentStyle::Tab, + ..PrinterOptions::default() + }.with_print_width(PrintWidth::new(10)) + )
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (28)
crates/biome_cli/tests/snapshots/main_cases_handle_astro_files/format_astro_files.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_astro_files/format_astro_files_write.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_astro_files/format_stdin_write_successfully.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_svelte_files/check_stdin_write_successfully.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_svelte_files/check_stdin_write_unsafe_successfully.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_svelte_files/format_stdin_successfully.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_svelte_files/format_stdin_write_successfully.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_svelte_files/format_svelte_carriage_return_line_feed_files.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_svelte_files/format_svelte_ts_context_module_files.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_svelte_files/format_svelte_ts_context_module_files_write.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_vue_files/check_stdin_write_successfully.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_vue_files/check_stdin_write_unsafe_successfully.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_vue_files/format_stdin_successfully.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_vue_files/format_stdin_write_successfully.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_vue_files/format_vue_carriage_return_line_feed_files.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_vue_files/format_vue_explicit_js_files.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_vue_files/format_vue_explicit_js_files_write.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_vue_files/format_vue_generic_component_files.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_vue_files/format_vue_implicit_js_files.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_vue_files/format_vue_implicit_js_files_write.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_vue_files/format_vue_ts_files.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_vue_files/format_vue_ts_files_write.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_format/format_svelte_explicit_js_files.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_format/format_svelte_explicit_js_files_write.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_format/format_svelte_implicit_js_files.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_format/format_svelte_implicit_js_files_write.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_format/format_svelte_ts_files.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_format/format_svelte_ts_files_write.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (7)
.changeset/dull-drinks-switch.md(1 hunks)crates/biome_cli/tests/cases/handle_svelte_files.rs(0 hunks)crates/biome_cli/tests/cases/handle_vue_files.rs(0 hunks)crates/biome_cli/tests/commands/format.rs(0 hunks)crates/biome_formatter/src/printer/mod.rs(3 hunks)crates/biome_service/src/file_handlers/svelte.rs(3 hunks)crates/biome_service/src/file_handlers/vue.rs(3 hunks)
💤 Files with no reviewable changes (3)
- crates/biome_cli/tests/cases/handle_vue_files.rs
- crates/biome_cli/tests/commands/format.rs
- crates/biome_cli/tests/cases/handle_svelte_files.rs
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_service/src/file_handlers/svelte.rscrates/biome_formatter/src/printer/mod.rscrates/biome_service/src/file_handlers/vue.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_service/src/file_handlers/svelte.rscrates/biome_formatter/src/printer/mod.rscrates/biome_service/src/file_handlers/vue.rs
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md: Create changesets withjust new-changeset; store them in.changeset/with correct frontmatter (package keys and change type).
In changeset descriptions, follow content conventions: user-facing changes only; past tense for what you did; present tense for current behavior; link issues for fixes; link rules/assists; include representative code blocks; end every sentence with a period.
When adding headers in a changeset, only use #### or ##### levels.
Files:
.changeset/dull-drinks-switch.md
🧠 Learnings (11)
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import the FormatNode trait and implement it for your Node
Applied to files:
crates/biome_service/src/file_handlers/svelte.rscrates/biome_service/src/file_handlers/vue.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Use the generic Format trait and FormatNode for AST nodes when implementing the formatter
Applied to files:
crates/biome_service/src/file_handlers/svelte.rscrates/biome_service/src/file_handlers/vue.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Build an intermediate IR via the provided helper APIs when formatting
Applied to files:
crates/biome_service/src/file_handlers/svelte.rscrates/biome_service/src/file_handlers/vue.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/Cargo.toml : Add biome_js_formatter as a path dependency in Cargo.toml: biome_js_formatter = { version = "0.0.1", path = "../biome_js_formatter" }
Applied to files:
crates/biome_service/src/file_handlers/svelte.rscrates/biome_service/src/file_handlers/vue.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Do not attempt to fix code: if a mandatory token/node is missing, return None instead
Applied to files:
crates/biome_service/src/file_handlers/svelte.rs
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/src/lib.rs : Place the plumbing traits and impls (AsFormat, IntoFormat, FormattedIterExt, and their Iterator adapters) in the biome_html_formatter crate’s lib.rs
Applied to files:
crates/biome_service/src/file_handlers/svelte.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Use dbg_write! to debug and inspect the written IR elements during formatter development
Applied to files:
crates/biome_service/src/file_handlers/svelte.rs
📚 Learning: 2025-08-17T08:55:30.118Z
Learnt from: CR
PR: biomejs/biome#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T08:55:30.118Z
Learning: Applies to crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/** : Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Applied to files:
crates/biome_service/src/file_handlers/svelte.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : When a token is mandatory and present in the AST, use the AST-provided token (e.g., node.l_paren_token().format()) instead of hardcoded tokens
Applied to files:
crates/biome_service/src/file_handlers/svelte.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : For non-mandatory tokens, use the formatter helper functions (e.g., token, space_token) instead of hardcoding
Applied to files:
crates/biome_service/src/file_handlers/svelte.rs
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/src/lib.rs : Expose a documented public function format_node(options: HtmlFormatOptions, root: &HtmlSyntaxNode) -> FormatResult<Formatted<HtmlFormatContext>> delegating to biome_formatter::format_node
Applied to files:
crates/biome_service/src/file_handlers/svelte.rscrates/biome_formatter/src/printer/mod.rscrates/biome_service/src/file_handlers/vue.rs
🧬 Code graph analysis (3)
crates/biome_service/src/file_handlers/svelte.rs (3)
crates/biome_service/src/file_handlers/mod.rs (1)
parse_lang_from_script_opening_tag(770-814)crates/biome_formatter/src/separated.rs (1)
format_node(17-17)crates/biome_js_parser/src/parse.rs (1)
parse_js_with_cache(272-283)
crates/biome_formatter/src/printer/mod.rs (2)
crates/biome_formatter/src/lib.rs (16)
new(807-809)new(882-884)new(948-960)new(1185-1187)new(1235-1237)new(1925-1933)new(2052-2054)document(892-894)format(1363-1379)from(391-393)from(397-399)from(403-405)from(485-487)from(567-572)from(598-600)from(684-686)crates/biome_formatter/src/builders.rs (2)
soft_line_break(67-69)soft_line_break_or_space(188-190)
crates/biome_service/src/file_handlers/vue.rs (3)
crates/biome_service/src/file_handlers/mod.rs (1)
parse_lang_from_script_opening_tag(770-814)crates/biome_formatter/src/separated.rs (1)
format_node(17-17)crates/biome_js_parser/src/parse.rs (1)
parse_js_with_cache(272-283)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: Documentation
- GitHub Check: Check Dependencies
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: autofix
- GitHub Check: Bench (biome_json_analyze)
🔇 Additional comments (6)
crates/biome_service/src/file_handlers/vue.rs (2)
1-1: Imports and wiring are tidy.Nothing alarming; mirrors the Svelte changes neatly.
Also applies to: 12-12, 14-14, 19-19
144-153: Great, digging into the JS handler to check indent support. Let me know the output, and we’ll confirm whether range/on-type need an explicit indent parameter.crates/biome_formatter/src/printer/mod.rs (4)
61-65: Initialising pending indent fixes the “indent 0 at start” bug.Setting pending_indent from the stack before the loop is the right call and explains the observed improvement for embedded script blocks.
68-71: Flushing line suffixes when the queue empties is sensible.This prevents dangling suffixes at EOF. The new tests cover the case well; no objections.
73-79: Printed construction: confirm no range metadata regression.You now always return Printed with range=None from print_with_indent. That’s expected, but double-check nothing relied on a non-None range here (range-based APIs should still go through format_range paths).
1422-1433: Nice test helper for indentation.Keeps tests concise and focused.
CodSpeed Performance ReportMerging #7330 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Would it make sense to also implement the indentScriptAndStyle option this way? I can send a follow up PR for that.
Yes! That's how we would do it :) |
Summary
The function
print_with_indentwas already available in our printer, but it was broken. While I was working at the embedded languages, I found out the issue and fixed it.We can ship this as a minor in the meantime, I bet the users will like this improvement :)
Test Plan
Added new tests, updated existing snapshots.
Docs
Will need to open a PR to update the language support