-
-
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): ensure comments before async
keyword are placed before it
#1500
Conversation
✅ Deploy Preview for biomejs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
async
keyword are placed before it
@@ -209,7 +209,8 @@ class Foo { | |||
@decorator /*before*/ | |||
/*after*/ method() {} | |||
@decorator.method(value) /*before*/ | |||
async /*after*/ method() {} | |||
/*after*/ |
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.
Original code
@decorator.method(value) /*before*/
/*after*/ async method() {}
The behavior is the same as Prettier
/*after*/ | ||
async method() {} |
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.
It guess it's a regression.
Prettier playground
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.
Here is the original code. It has a decorator above.
biome/crates/biome_js_formatter/tests/specs/js/module/decorators/class_members_call_decorator.js
Lines 70 to 71 in d45bc75
@decorator.method(value) /*before*/ | |
/*after*/ async method() {} |
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.
Sorry, you're right(:
CodSpeed Performance ReportMerging #1500 will degrade performances by 7.43%Comparing Summary
Benchmarks breakdown
|
async
keyword are placed before itasync
keyword are placed before it
@@ -1244,6 +1245,32 @@ fn handle_import_export_specifier_comment( | |||
} | |||
} | |||
|
|||
/// Ensure that comments before the `async`` keyword are placed just before it. |
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.
Could you please check if we need to do the same thing for generators?🙏
We can do it in the next PR.
https://biomejs.dev/playground/?code=YwBsAGEAcwBzACAARgBvAG8AIAB7AAoAIAAgACAAQABkAGUAYwBvAHIAYQB0AG8AcgAuAG0AZQB0AGgAbwBkACgAdgBhAGwAdQBlACkAIAAvACoAYgBlAGYAbwByAGUAKgAvACAACgAgACAAIAAvACoAYQBmAHQAZQByACoALwAgAGEAcwB5AG4AYwAgAG0AZQB0AGgAbwBkACgAKQAgAHsAfQAgAAoACgAgACAAIAAgACAAQABkAGUAYwBvAHIAYQB0AG8AcgAuAG0AZQB0AGgAbwBkACgAdgBhAGwAdQBlACkAIAAvACoAYgBlAGYAbwByAGUAKgAvACAACgAgACAAIAAvACoAYQBmAHQAZQByACoALwAgACoAIABtAGUAdABoAG8AZAAxACgAKQAgAHsAfQAgAAoACgAgACAAIAAgACAAIAAgAEAAZABlAGMAbwByAGEAdABvAHIALgBtAGUAdABoAG8AZAAoAHYAYQBsAHUAZQApACAALwAqAGIAZQBmAG8AcgBlACoALwAgAAoAIAAgACAALwAqAG0AaQBkAGQAbABlACoALwAgAGEAcwB5AG4AYwAgAC8AKgBhAGYAdABlAHIAKgAvACoAIABtAGUAdABoAG8AZAAyACgAKQAgAHsAfQAgAAoAfQA%3D
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.
Generators have the same issue. I will submit another PR to fix it.
Looks good to me! We have just released a patch version. Could you rebase your branch on main? |
Done |
Summary
Closes #1406
Ensure the comments before the
async
keyword are placed before it. If thefollowing_token
of the comment isasync
keyword then attach the comment to the preceding node