Skip to content

Fix printing of comments before =>#15160

Merged
nicolo-ribaudo merged 2 commits intobabel:mainfrom
nicolo-ribaudo:hotfix
Nov 8, 2022
Merged

Fix printing of comments before =>#15160
nicolo-ribaudo merged 2 commits intobabel:mainfrom
nicolo-ribaudo:hotfix

Conversation

@nicolo-ribaudo
Copy link
Copy Markdown
Member

@nicolo-ribaudo nicolo-ribaudo commented Nov 8, 2022

Q                       A
Fixed Issues? Fixes #15161
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

This fix conflicts with the fix proposed in #15159, however I believe that it solves the problem in a better way: we must keep the correct _noLineTerminator to make sure to disallow any multiline comment right before =>.

All these cases with multi-line comments before => cannot happen just by parsing+printing the source code, but can happen if plugins modify the AST. It's better to miss printing the comment (or to report an error, as in #15158), but we should never generate invalid code.

@babel-bot
Copy link
Copy Markdown
Collaborator

babel-bot commented Nov 8, 2022

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/53392/

@liuxingbaoyu
Copy link
Copy Markdown
Member

This PR looks good to me, can we keep https://github.com/babel/babel/pull/15159/files#diff-6c32c344bfe1f4e77de24cdd3e427e84c4ac0a871380ff3720a7351cac63cb74R197-R198?
In my opinion this is better than being trailing Commens.

@liuxingbaoyu

This comment was marked as outdated.

@nicolo-ribaudo
Copy link
Copy Markdown
Member Author

Well, we could still keep the new this.printInnerComments(); call

@liuxingbaoyu
Copy link
Copy Markdown
Member

The current this._noLineTerminator is automatically set to false, which I fear does not completely prevent all line breaks.

Also the output in #15161 is weird, it seems like we are wrapping comments for non-existent, it might be worth me to open a PR to optimize.

@nicolo-ribaudo
Copy link
Copy Markdown
Member Author

The current this._noLineTerminator is automatically set to false, which I fear does not completely prevent all line breaks.

It's automatically set to false after printing a token/word. [no LineTerminator here] is only between two tokens, so after printing the second one is always safe to set _noLineTerminator to false.

@nicolo-ribaudo nicolo-ribaudo merged commit 4285d5f into babel:main Nov 8, 2022
@nicolo-ribaudo nicolo-ribaudo deleted the hotfix branch November 8, 2022 16:45
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Feb 9, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area: comments i: regression outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: generator PR: Bug Fix 🐛 A type of pull request used for our changelog categories PR: Fixes failing main

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: With comments: false, babel inserts an extra newline before the => of an arrow function

4 participants