feat(js-formatter): implement delimiterSpacing for JSX#9720
Conversation
Add the `delimiterSpacing` formatter option. This option inserts spaces inside delimiters when content fits on a single line. Empty delimiters are not affected, and no space is added before the opening delimiter. The specific delimiters affected depend on the language. Includes the shared DelimiterSpacing type, global configuration, overrides support, Prettier migration, CLI flag, generated schema/bindings, and changeset.
🦋 Changeset detectedLatest commit: 3fb8118 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 |
Implement delimiterSpacing for JSX nodes: expression attribute values, spread attributes, expression children, and spread children.
32676cb to
855f84c
Compare
WalkthroughThis pull request introduces a new Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
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 (1)
crates/biome_js_formatter/src/jsx/auxiliary/expression_child.rs (1)
73-91:⚠️ Potential issue | 🟡 MinorDon’t pad empty JSX expression children.
When
expressionisNoneand there are no dangling comments, thedelimiter_spacingbranch still emits{ }. That breaks the new “empty delimiters stay untouched” contract and makes recovered JSX a bit noisier than necessary. Please only add the extra spaces when there is actual comment content to wrap.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_js_formatter/src/jsx/auxiliary/expression_child.rs` around lines 73 - 91, The current branch pads empty JSX expression children when delimiter_spacing is true even if there are no dangling comments; change the logic so you only add surrounding spaces when there are actual dangling comments to render. Compute a boolean (e.g., has_dangling_comment) using comments.leading_dangling_trailing_comments(node.syntax()).any(...) to detect any dangling comments, then replace the else if condition to require both delimiter_spacing and has_dangling_comment before writing [space(), format_dangling_comments(node.syntax()), space()]; keep the existing has_line_comment branch unchanged.
🧹 Nitpick comments (2)
crates/biome_js_formatter/src/js/objects/computed_member_name.rs (1)
18-40: Consider usingsoft_block_indent_with_maybe_spacefor consistency.This implementation uses explicit
space()tokens rather thansoft_block_indent_with_maybe_space, which other bracket-delimited constructs (arrays, array patterns) use. The current approach means:
obj[ veryLongExpressionHere ]won't get soft line breaks when content overflows- Behaviour diverges from similar constructs
If computed member names are intentionally kept short/inline, this is fine. Otherwise, consider aligning with the pattern used elsewhere:
♻️ Optional: align with other bracket patterns
- let should_insert_space = f.options().delimiter_spacing().value(); - - if should_insert_space { - write![ - f, - [ - l_brack_token.format(), - space(), - expression.format(), - space(), - r_brack_token.format(), - ] - ] - } else { - write![ - f, - [ - l_brack_token.format(), - expression.format(), - r_brack_token.format(), - ] - ] - } + let should_insert_space = f.options().delimiter_spacing().value(); + + write![ + f, + [ + l_brack_token.format(), + group(&soft_block_indent_with_maybe_space( + &expression.format(), + should_insert_space + )), + r_brack_token.format(), + ] + ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_js_formatter/src/js/objects/computed_member_name.rs` around lines 18 - 40, The current computed member formatting uses explicit space() tokens based on should_insert_space, which prevents soft line breaks; replace the explicit spacing around expression.format() with the same pattern used for other bracket-delimited constructs by using soft_block_indent_with_maybe_space (or the equivalent helper) together with l_brack_token.format(), r_brack_token.format(), and expression.format() so the formatter will insert a soft indent/space and allow line breaks when the expression is long; update the branch that checks should_insert_space (and the else branch if needed) to use soft_block_indent_with_maybe_space instead of direct space() calls while still honoring f.options().delimiter_spacing().value() and the existing tokens l_brack_token and r_brack_token.crates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/computed_members.js (1)
53-64: Minor duplication offoo[a]test case.Lines 53 and 63 both test
foo[a]. Since they're in different sections with different semantic purposes, this is fine, but you could consider removing one if consolidation is desired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/computed_members.js` around lines 53 - 64, The test file contains a duplicated test case for the computed member expression foo[a] in two different sections; to resolve, remove one of the duplicate occurrences (either the first foo[a] or the later one under the "Computed member expressions - dynamic expressions" section) or consolidate them into a single test entry so the suite only checks foo[a] once while preserving coverage for other cases like foo["a"] and foo[bar].
🤖 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/delimiter-spacing-js.md:
- Around line 5-15: Update the changeset text in
.changeset/delimiter-spacing-js.md to reflect that delimiterSpacing applies to
both JavaScript and JSX brace spacing: modify the title and body to mention JSX
braces in addition to JS, add a JSX example demonstrating spacing inside JSX
expression braces (e.g., <Component prop={ value }/> style), and ensure wording
clarifies the behavior for template literals, array/object/parenthesis
delimiters, logical NOT chains, and that empty delimiters/opening-space rules
are unchanged so the release note accurately covers both JS and JSX.
In
`@crates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/template_elements.js`:
- Around line 45-47: The test inputs for arr1/arr2/arr3 are inconsistent: arr1
uses an extra space in the template expression (`${ []}`) while arr2 and arr3
use `${[... ]}`; either remove the extra space in the arr1 template expression
to match the others (change the template to use `${[]}`) or, if the spacing is
intentional to test normalization, add a brief comment above the arr1
declaration explaining the purpose; locate the arr1, arr2, arr3 declarations in
the delimiter-spacing template_elements test and apply the chosen change.
In `@crates/biome_js_formatter/tests/specs/jsx/delimiter_spacing/options.json`:
- Around line 1-8: The $schema field value in this JSON spec is using one too
many "../" segments; update the "$schema" property so it uses six parent
directories instead of seven (change
"../../../../../../../packages/@biomejs/biome/configuration_schema.json" to
"../../../../../../packages/@biomejs/biome/configuration_schema.json") to
correct the schema path resolution.
---
Outside diff comments:
In `@crates/biome_js_formatter/src/jsx/auxiliary/expression_child.rs`:
- Around line 73-91: The current branch pads empty JSX expression children when
delimiter_spacing is true even if there are no dangling comments; change the
logic so you only add surrounding spaces when there are actual dangling comments
to render. Compute a boolean (e.g., has_dangling_comment) using
comments.leading_dangling_trailing_comments(node.syntax()).any(...) to detect
any dangling comments, then replace the else if condition to require both
delimiter_spacing and has_dangling_comment before writing [space(),
format_dangling_comments(node.syntax()), space()]; keep the existing
has_line_comment branch unchanged.
---
Nitpick comments:
In `@crates/biome_js_formatter/src/js/objects/computed_member_name.rs`:
- Around line 18-40: The current computed member formatting uses explicit
space() tokens based on should_insert_space, which prevents soft line breaks;
replace the explicit spacing around expression.format() with the same pattern
used for other bracket-delimited constructs by using
soft_block_indent_with_maybe_space (or the equivalent helper) together with
l_brack_token.format(), r_brack_token.format(), and expression.format() so the
formatter will insert a soft indent/space and allow line breaks when the
expression is long; update the branch that checks should_insert_space (and the
else branch if needed) to use soft_block_indent_with_maybe_space instead of
direct space() calls while still honoring
f.options().delimiter_spacing().value() and the existing tokens l_brack_token
and r_brack_token.
In
`@crates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/computed_members.js`:
- Around line 53-64: The test file contains a duplicated test case for the
computed member expression foo[a] in two different sections; to resolve, remove
one of the duplicate occurrences (either the first foo[a] or the later one under
the "Computed member expressions - dynamic expressions" section) or consolidate
them into a single test entry so the suite only checks foo[a] once while
preserving coverage for other cases like foo["a"] and foo[bar].
🪄 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: f1423606-ce38-40ad-89f2-07b096b22cb6
⛔ Files ignored due to path filters (23)
crates/biome_cli/tests/snapshots/main_cases_help/check_help.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_help/ci_help.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_help/format_help.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_format/applies_global_delimiter_spacing.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_format/language_overrides_global_delimiter_spacing.snapis excluded by!**/*.snapand included by**crates/biome_configuration/tests/invalid/formatter_extraneous_field.json.snapis excluded by!**/*.snapand included by**crates/biome_configuration/tests/invalid/formatter_quote_style.json.snapis excluded by!**/*.snapand included by**crates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/arrays.js.snapis excluded by!**/*.snapand included by**crates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/arrow_functions.js.snapis excluded by!**/*.snapand included by**crates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/call_arguments.js.snapis excluded by!**/*.snapand included by**crates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/computed_members.js.snapis excluded by!**/*.snapand included by**crates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/conditionals_misc.js.snapis excluded by!**/*.snapand included by**crates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/control_flow.js.snapis excluded by!**/*.snapand included by**crates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/loops.js.snapis excluded by!**/*.snapand included by**crates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/parameters_catch.js.snapis excluded by!**/*.snapand included by**crates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/parenthesized.js.snapis excluded by!**/*.snapand included by**crates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/setters.js.snapis excluded by!**/*.snapand included by**crates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/template_elements.js.snapis excluded by!**/*.snapand included by**crates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/unary.js.snapis excluded by!**/*.snapand included by**crates/biome_js_formatter/tests/specs/jsx/delimiter_spacing/jsx_expression_attrs.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_formatter/tests/specs/jsx/delimiter_spacing/jsx_expression_children.jsx.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (55)
.changeset/delimiter-spacing-js.md.changeset/delimiter-spacing-jsx.md.changeset/delimiter-spacing-option.mdcrates/biome_cli/src/execute/migrate/prettier.rscrates/biome_cli/tests/commands/format.rscrates/biome_configuration/src/formatter.rscrates/biome_configuration/src/javascript/formatter.rscrates/biome_configuration/src/overrides.rscrates/biome_formatter/src/lib.rscrates/biome_js_formatter/src/context.rscrates/biome_js_formatter/src/js/assignments/array_assignment_pattern.rscrates/biome_js_formatter/src/js/auxiliary/template_element.rscrates/biome_js_formatter/src/js/bindings/array_binding_pattern.rscrates/biome_js_formatter/src/js/bindings/parameters.rscrates/biome_js_formatter/src/js/classes/setter_class_member.rscrates/biome_js_formatter/src/js/declarations/catch_declaration.rscrates/biome_js_formatter/src/js/expressions/array_expression.rscrates/biome_js_formatter/src/js/expressions/arrow_function_expression.rscrates/biome_js_formatter/src/js/expressions/call_arguments.rscrates/biome_js_formatter/src/js/expressions/computed_member_expression.rscrates/biome_js_formatter/src/js/expressions/parenthesized_expression.rscrates/biome_js_formatter/src/js/expressions/unary_expression.rscrates/biome_js_formatter/src/js/objects/computed_member_name.rscrates/biome_js_formatter/src/js/objects/setter_object_member.rscrates/biome_js_formatter/src/js/statements/do_while_statement.rscrates/biome_js_formatter/src/js/statements/for_in_statement.rscrates/biome_js_formatter/src/js/statements/for_of_statement.rscrates/biome_js_formatter/src/js/statements/for_statement.rscrates/biome_js_formatter/src/js/statements/if_statement.rscrates/biome_js_formatter/src/js/statements/switch_statement.rscrates/biome_js_formatter/src/js/statements/while_statement.rscrates/biome_js_formatter/src/jsx/attribute/expression_attribute_value.rscrates/biome_js_formatter/src/jsx/attribute/spread_attribute.rscrates/biome_js_formatter/src/jsx/auxiliary/expression_child.rscrates/biome_js_formatter/src/jsx/auxiliary/spread_child.rscrates/biome_js_formatter/src/lib.rscrates/biome_js_formatter/src/utils/jsx.rscrates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/arrays.jscrates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/arrow_functions.jscrates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/call_arguments.jscrates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/computed_members.jscrates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/conditionals_misc.jscrates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/control_flow.jscrates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/loops.jscrates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/options.jsoncrates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/parameters_catch.jscrates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/parenthesized.jscrates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/setters.jscrates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/template_elements.jscrates/biome_js_formatter/tests/specs/js/module/delimiter-spacing/unary.jscrates/biome_js_formatter/tests/specs/jsx/delimiter_spacing/jsx_expression_attrs.jsxcrates/biome_js_formatter/tests/specs/jsx/delimiter_spacing/jsx_expression_children.jsxcrates/biome_js_formatter/tests/specs/jsx/delimiter_spacing/options.jsoncrates/biome_service/src/file_handlers/javascript.rscrates/biome_service/src/settings.rs
Merging this PR will not alter performance
Comparing Footnotes
|
ematipico
left a comment
There was a problem hiding this comment.
I'll take care of the changeset towards the end
|
Sorry, I haven't had time to review the rest of the PRs to get them up to date, but I think I can find some time to do it this afternoon or tomorrow. |
Summary
Implements
delimiterSpacingfor JSX, building on #9719 (JS).For JSX, affects:
attr={ value }){ ...props }){ children }){ ...items })AI usage
AI assisted in implementing the JSX formatter changes and writing tests.
Test Plan