Skip to content

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented May 13, 2025

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

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 import

In the 5th commit we simplified the import phase comment handling.

@JLHwung JLHwung added PR: Bug Fix 🐛 A type of pull request used for our changelog categories pkg: parser Spec: Source Phase Imports labels May 13, 2025
@babel-bot
Copy link
Collaborator

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

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a 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",

Copy link
Contributor Author

@JLHwung JLHwung May 13, 2025

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.

Copy link
Member

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?

Copy link
Contributor Author

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

// 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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we could try

@JLHwung JLHwung merged commit 3784ab7 into babel:main May 14, 2025
57 checks passed
@JLHwung JLHwung deleted the improve-import-phase-parsing branch May 14, 2025 17: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 Aug 14, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: Source Phase Imports
Projects
None yet
Development

Successfully merging this pull request may close these issues.

import.source() does not work with @babel/eslint-parser
4 participants