-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Improve import phase parsing #17308
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
Improve import phase parsing #17308
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/59323 |
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.
👍
// Todo: support trailing comments spanned across trailing comma | ||
"import-phases/source-expression-comments/input.js", | ||
"import-phases/source-expression-options-comments/input.js", | ||
|
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.
Note: this is a newly discovered issue and should be fixed in another PR.
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 was wondering why they don't just work by default, aren't those comments marked as inner comments and thus printed automatically between ,
and )
, where we should be checking for inner comments?
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 they are marked as trailing comments now due to
babel/packages/babel-parser/src/parser/comments.ts
Lines 174 to 176 in 205e1cf
// If a commentWhitespace follows a comma and the containingNode allows | |
// list structures with trailing comma, merge it to the trailingComment | |
// of the last non-null list element |
If the experimental_preserveFormat
works well, maybe we can get rid of this part in Babel 8?
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.
Yes we could try
In the first commit we removed the
createImportExpressions
requirements for the import phase proposal.In the 2rd and 3rd commits we added more comments handling test cases and fixed a new bug surfaced from the new tests
In the 4th commit we add trailing comma handling to the ImportExpression such that
experimental_preserveFormat
can preserve the trailing comma in dynamic importIn the 5th commit we simplified the import phase comment handling.