Skip to content
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

Fix the duplicate export initialization (#14272) #14276

Closed
wants to merge 1 commit into from

Conversation

JasinYip
Copy link
Contributor

@JasinYip JasinYip commented Feb 16, 2022

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

The array uninitializedExportNames haven't reset to [] so the array would contains the previous chunks. This will make lots of duplicate initialization statements while the amount of exports more than 100.

For more detail: #14272

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.

Thanks! Could you add a test like the one I suggested in #14272 (comment)? It can go in a new packages/babel-plugin-transform-modules-commonjs/test/fixtures/regression/many-exports-chunked/ folder. You only have to create the options.json and input.mjs files, then yarn jest modules will automatically generate the output.js file.

@JasinYip
Copy link
Contributor Author

@nicolo-ribaudo No problem, I will add some tests.

Copy link
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

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

For reviewers: The rationale for this PR is that the uninitializedExportNames should be reset after they have been transformed to init statements. We have reset them when we found non-empty initStatements, but failed to do so at the chunk end.

@nicolo-ribaudo
Copy link
Member

Ping @JasinYip, we just need a test for the fixed behavior and then this PR can be merged 🙂

@nicolo-ribaudo
Copy link
Member

Merged at #14313 / 2671c98, thanks!

@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 May 29, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: modules outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: initializing ES Module exports might leads JSC to "Maximum call stack size exceeded"
4 participants