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
Sort module export names #13788
Sort module export names #13788
Conversation
} else { | ||
// We generate init statements (`exports.a = exports.b = ... = void 0`) | ||
// for every 100 exported names. | ||
const chunkSize = 100; |
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.
The chunk behaviour has existed since 95e08b6#diff-f9b022d801ca0bab9bbb7057b9ac49aac0d8c2ab53f41f97e7e15bdd10f26069R272. I don't know if it is a workaround of browser quirks or it is for aesthetic reasons.
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 think it's to avoid a too deeply nested AST
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.
Ref - #14272
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/48889/ |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 3f6b64f:
|
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.
We still need to update the buildNamespaceInitStatements
implementation.
Turns out we don't need to update |
In this PR we sort the export initialization statements by the export names. The
initStatements
andexportsNames
are merged to an array of tuple<string, Statement>
so we can sort the init statements by their names.We didn't add new tests as current tests are sufficient to show that the initializations are ordered by names.
Note that under the
noIncompleteNsImportDetection
assumption, the issue can not be fixed without adding new initialization statements, which are exactly removed by definition of the assumption. We can add an extra note about the effects of this assumption.