-
-
Notifications
You must be signed in to change notification settings - Fork 476
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
fix(js_formatter): placements of line comments between function parentheses and body #1485
fix(js_formatter): placements of line comments between function parentheses and body #1485
Conversation
✅ Deploy Preview for biomejs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
94666d3
to
df3c813
Compare
CodSpeed Performance ReportMerging #1485 will degrade performances by 8.85%Falling back to comparing Summary
Benchmarks breakdown
|
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.
Just a tiny comment to address, not a blocker
// ```javascript | ||
// function test() /* comment */ { | ||
// function test() // comment { |
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.
The JS snippet didn't need to change, this is now incorrect because the curly bracket {
is part of the comment, unless you meant to move the {
to a new line too,
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.
oh yeah, you are right.
250d1c1
to
8828e1b
Compare
We should only do this for `Line` comments, it seems like that is what prettier does.
c2e31e3
to
6feeb93
Compare
6feeb93
to
bd2c08b
Compare
Summary
To match prettier we need to attach a line comment that sits between function parentheses and function body to first statement inside the body.
Before we only did this for function declarations and we attached all types of comments to the body but to match prettier we should only do that for line comments. We should also not exclude empty statements.
I found another non-idempotent formatting issue but we can tackle this in another pr. For this case prettier also has a non-idempotent formatting.
Closes: #1172
Test Plan
Added new tests and uncommented prittier diff