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

feat: root module is less prone to be wrapped in IIFE #18348

Merged
merged 1 commit into from Apr 24, 2024

Conversation

fi3ework
Copy link
Contributor

What kind of change does this PR introduce?

This is the prerequisite dependency for https://github.com/webppack/webpack/pull/18272.

Issue to address: We hope that when building an ESM format library, the concatenated module is placed at the top level, not wrapped in an IIFE so that it can be tree-shaken. However, there are some conditions that will cause the concatenated module wrapped in an IIFE when it is an inline module, which could be resolved though. This PR is dedicated to resolving one of the conditions: "it need to be isolated against other modules in the chunk."

This not only prepares for the ability to build a tree-shakable library but also, removing the IIFE will enhance the runtime performance of the artifact. I created a gist to demonstrate the change with the affected test case https://gist.github.com/fi3ework/5006ab48c4b6338fbd91f433f35e5ff6/revisions.

How: The solution is quite straightforward, this PR resolves the conflict between a single inlinedModule and chunkModules. As long as we prevent the scope of the inlinedModule from being perceived by chunkModules, we can take away the inlinedModules from the IIFE. This approach is very similar to concatenateModules, and I have also moved part of the method from concatenateModules to lib/util/mergeScope.js to facilitate codes sharing. This PR may affect some performance as doing the scope scan, and if necessary, the feature of this PR can be marked as an experimental attribute or other methods to selectively enable.

Plan: There is still a condition like inlinedModules.size > 2 where the IIFE wrapper can be eliminated, we can take it step by step and implement afterwards.

Did you add tests for your changes?

Yes, and the existing related test case is test/cases/entry-inline.

Does this PR introduce a breaking change?

I don't think so, although we changed the result of the code generation that eliminating the IIFE wrapper of inlinedModule.

What needs to be documented once your changes are merged?

No.

@webpack-bot
Copy link
Contributor

For maintainers only:

  • This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
  • This needs to be backported to webpack 4 (issue will be created when merged)

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Great job

@alexander-akait alexander-akait merged commit c586c7b into webpack:main Apr 24, 2024
52 of 56 checks passed
@fi3ework fi3ework deleted the no-iife branch April 24, 2024 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Need's Release
Development

Successfully merging this pull request may close these issues.

None yet

3 participants