Skip to content

fix(js_analyze): move comments with useSortedAttributes action#9298

Merged
Netail merged 2 commits intobiomejs:mainfrom
Netail:fix/9296-move-comments
Mar 2, 2026
Merged

fix(js_analyze): move comments with useSortedAttributes action#9298
Netail merged 2 commits intobiomejs:mainfrom
Netail:fix/9296-move-comments

Conversation

@Netail
Copy link
Member

@Netail Netail commented Mar 2, 2026 •
edited
Loading

Summary

Comments are not moved when sorting attributes

Fixes #9296

Test Plan

Added a unit test

Docs

@changeset-bot
Copy link

changeset-bot bot commented Mar 2, 2026 •
edited
Loading

🦋 Changeset detected

Latest commit: b91900c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
@biomejs/biome Patch
@biomejs/cli-win32-x64 Patch
@biomejs/cli-win32-arm64 Patch
@biomejs/cli-darwin-x64 Patch
@biomejs/cli-darwin-arm64 Patch
@biomejs/cli-linux-x64 Patch
@biomejs/cli-linux-arm64 Patch
@biomejs/cli-linux-x64-musl Patch
@biomejs/cli-linux-arm64-musl Patch
@biomejs/wasm-web Patch
@biomejs/wasm-bundler Patch
@biomejs/wasm-nodejs Patch
@biomejs/backend-jsonrpc Patch

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 github-actions bot added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels Mar 2, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 2, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ac2ce11 and b91900c.

⛔ Files ignored due to path filters (1)
  • crates/biome_js_analyze/tests/specs/source/useSortedAttributes/with-comments.jsx.snap is excluded by !**/*.snap and included by **
📒 Files selected for processing (1)
  • crates/biome_js_analyze/tests/specs/source/useSortedAttributes/with-comments.jsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/biome_js_analyze/tests/specs/source/useSortedAttributes/with-comments.jsx

Walkthrough

The assist action for useSortedAttributes was modified to use replace_node_discard_trivia instead of replace_node when replacing JSX props; this changes how trivia (comments/whitespace) is handled during attribute sorting. A new test case with inline and line comments was added to exercise sorting with comments. No public API signatures were changed.

Possibly related PRs

Suggested reviewers

  • ematipico
  • dyc3
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main fix: comments are now moved along with attributes when applying the useSortedAttributes action.
Description check ✅ Passed The description is related to the changeset and clearly references issue #9296, explaining the problem and test plan.
Linked Issues check ✅ Passed The PR successfully addresses issue #9296 by replacing replace_node with replace_node_discard_trivia to preserve comments during attribute sorting, with supporting test case added.
Out of Scope Changes check ✅ Passed All changes are scoped to the useSortedAttributes fix: the implementation change, changelog entry, and new test case—nothing extraneous.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 2, 2026 •
edited
Loading

Merging this PR will not alter performance

✅ 58 untouched benchmarks
⏩ 156 skipped benchmarks1

Comparing Netail:fix/9296-move-comments (b91900c) with main (6b01778)

Open in CodSpeed

Footnotes

  1. 156 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

dyc3
dyc3 reviewed Mar 2, 2026
View reviewed changes
crates/biome_js_analyze/tests/specs/source/useSortedAttributes/with-comments.jsx
Comment on lines +2 to +7
// Simple comment
tel={5555555}
address="NY"
{...this.props}
// Simple comment
lastName="Smith"
Copy link
Contributor

@dyc3 dyc3 Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make these comments different so its more obvious to see if the comments stay with the attributes or not.

Copy link
Member Author

@Netail Netail Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aye true

@Netail Netail merged commit 1003229 into biomejs:main Mar 2, 2026
18 checks passed
@Netail Netail deleted the fix/9296-move-comments branch March 2, 2026 20:40
@github-actions github-actions bot mentioned this pull request Mar 2, 2026
Merged
@coderabbitai coderabbitai bot mentioned this pull request Mar 19, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dyc3 dyc3 dyc3 left review comments

@ematipico ematipico ematipico approved these changes

Assignees

No one assigned

Labels

A-Linter Area: linter L-JavaScript Language: JavaScript and super languages

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

💅 useSortedAttributes comments

3 participants

@Netail @ematipico @dyc3