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
Retain trailing comments in array expressions #10369
Conversation
This is a proposed fix for babel#10368 with a simple test.
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/11403/ |
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.
I see some opportunity for code reuse here, but wanted to keep the changes isolated.
If you think that it's worth it, please open a PR after that this one is merged!
packages/babel-parser/test/fixtures/comments/basic/array-expression-trailing-comma/input.js
Outdated
Show resolved
Hide resolved
}, | ||
{ | ||
"type": "CommentLine", | ||
"value": " Three", |
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.
🤔 I think Three
should not be the trailing comment of the string literal, instead it should be of the array expression.
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.
Ah good catch. I'll try fixing that.
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.
@banga Thanks.
I think that the CallExpression arguments may also be affected, but we could open a new PR to address that. And ideally we can refactor to have CallExpression and ArrayExpression shared the same routines.
fn(
a,
b,
/* comment */
)
/* outer */
Yeah, sounds good |
* Retain trailing comments in array expressions This is a proposed fix for babel#10368 with a simple test. * Move lastElement in the block where it's used * Test trailing comment after array expression * Don't move comments after the array expression * Retain trailing comment after the array expression
Thanks! Will do in a couple of days. |
Following up from babel#10369 - Unify the logic for adjusting trailing comments into a separate function - Use it for all three cases, which fixes a bug for ObjectExpressions and CallExpressions - Update tests to check for the fixed bug
* Refactor trailing comment adjustment Following up from #10369 - Unify the logic for adjusting trailing comments into a separate function - Use it for all three cases, which fixes a bug for ObjectExpressions and CallExpressions - Update tests to check for the fixed bug * Fix tests - Only modify trailingComments if necessary - Update snapshots of a couple of affected tests * Drop the underscore in adjustCommentsAfterTrailingComma_ * Handle ArrayPattern * Handle ObjectPattern * Handle import and export declarations These have to be handled a bit differently, because the node is visited after the and before the declaration. So we intercept when we are going from the last specifier to the source node. * Remove unnecessary check
This is a proposed fix for #10368
with a simple test.
This is based on the fix from #8488 but for array expressions. I see some opportunity for code reuse here, but wanted to keep the changes isolated.