fix: duplicated comments #588
Conversation
|
Small note: I couldn't get ESLint to work locally, as it's installing v9 and the eslintrc is still in the old format. Installing ESLint v8 manually results in a circular dependency in the config with the vitest plugin. Hopefully it works in the pipelines |
src/traverseAndModifyAst.ts
Outdated
| if (node.leadingComments) { | ||
| delete node.leadingComments; | ||
| } | ||
| if (node.trailingComments) { | ||
| delete node.trailingComments; | ||
| } |
There was a problem hiding this comment.
[Question] Any reason for the ifs? No tests fail if I simplify them out:
| if (node.leadingComments) { | |
| delete node.leadingComments; | |
| } | |
| if (node.trailingComments) { | |
| delete node.trailingComments; | |
| } | |
| delete node.leadingComments; | |
| delete node.trailingComments; |
There was a problem hiding this comment.
Good catch :D I forgot delete doesn't throw if the property doesn't exist on the object, so the if statements are unnecessary. I've removed them
JoshuaKGoldberg
added
the
status: waiting for author
src/preprocess.test.ts
Outdated
There was a problem hiding this comment.
Perhaps someone else could verify these changes, as my company's codebase has mostly if statements that are being rewritten by this plugin, and not any of the other examples in the tests like while and for statements.
👍 yeah agreement from me, I can't find any issues. If anything pops up in the future then great, it should be represented here as a test case.
|
If you merge from the latest |
github-actions
bot
removed
the
status: waiting for author
|
@all-contributors please add @RobinHeidenis for code.
|
|
I've put up a pull request to add @RobinHeidenis! 🎉 |
PR Checklist
status: accepting prsOverview
When adding braces to (if) statements that have comments above or below them they would be reprinted, resulting in duplicated comments. This PR aims to fix that by deleting the
leadingCommentsandtrailingCommentsproperties from the AST nodes parsed by the plugin.Removing these can cause extra newlines being printed (in place of the comments), so they are being filtered out by
trim()ing the generated code.I've ran this modified version of the plugin on my company's internal project, and I haven't seen anything weird or wrong being outputted. I've also added two tests to verify the behavior.
I'm not sure if deleting the
leadingCommentsandtrailingCommentsis the best way to do this, but it does seem to work properly. Perhaps someone else could verify these changes, as my company's codebase has mostly if statements that are being rewritten by this plugin, and not any of the other examples in the tests like while and for statements.🚀