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

Lazy loading only required exports from dynamic import() #10964

Merged
merged 2 commits into from
Jun 16, 2020

Conversation

pushkar100
Copy link
Contributor

@pushkar100 pushkar100 commented May 28, 2020

PR for the feature request #10959 (resolves #10959)

Adding magic comments to lazy load only named exports from a dynamically imported module.
Introducing webpackExports: "<name>" or webpackExports: ["<name1>", "<name2>"]

At present, the entire module is exported. With this feature, we can reduce the code we import (i.e chunk files of smaller size are created)

What kind of change does this PR introduce?
feature

Did you add tests for your changes?
Yes (will add more when required)

Does this PR introduce a breaking change?
No

What needs to be documented once your changes are merged?

  • Addition to Webpack magic comments
  • Benefit of using webpackExports

@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)

lib/dependencies/ImportParserPlugin.js Outdated Show resolved Hide resolved
lib/dependencies/ImportParserPlugin.js Outdated Show resolved Hide resolved
@alexander-akait
Copy link
Member

I think supporting import { something } from /* webpackExports: "something" */ 'package'; should be good feature too

@webpack-bot
Copy link
Contributor

@pushkar100 Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@sokra Please review the new changes.

@alexander-akait
Copy link
Member

@pushkar100 What about if I want to export something as default - import(webpackExports: "customFunction").then((myFunction) => myFunction())?

@pushkar100
Copy link
Contributor Author

pushkar100 commented May 28, 2020

@evilebottnawi I did think about it. The import() proposal is supposed to return a promise for the module namespace object which contains all the exports including "default" (Please correct me if I am wrong). Won't we be creating an alternative pattern by resolving directly to the default export instead of the module object itself?
The idea I currently have for accessing default export is:

import(/* webpackExports: "default" */ "./module")
.then(module => {/* only module.default is available */})

@sokra
Copy link
Member

sokra commented May 28, 2020

The code should keep working even if the comment is ignored (e.g. in native esm). So even if you only request a single export please return a namespace object for compat with native esm.

@sokra
Copy link
Member

sokra commented May 28, 2020

One can use destructing for shorter syntax: import("module").then(({ default: theDefaultExport }) => doSomething(theDefaultExport)) or const { default: theDefaultExport } = await import("module")

@pushkar100
Copy link
Contributor Author

@sokra @evilebottnawi I have prevented mangling of exports when webpackExports is used.
Also, added basic tests for checking unmangled names and default exports.

Now we can use webpackExports: "default" to get only the default export.

Copy link
Member

@sokra sokra left a comment

Choose a reason for hiding this comment

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

A little bit refactoring to getReferencedExports is needed to allow to pass the mangling info.

lib/Dependency.js Outdated Show resolved Hide resolved
lib/dependencies/ImportWeakDependency.js Outdated Show resolved Hide resolved
test/cases/chunks/inline-options/index.js Outdated Show resolved Hide resolved
test/cases/chunks/inline-options/index.js Outdated Show resolved Hide resolved
lib/FlagDependencyUsagePlugin.js Outdated Show resolved Hide resolved
@pushkar100
Copy link
Contributor Author

pushkar100 commented May 31, 2020

@sokra I have refactored the code: using an object for referenced exports with canMangle option.
Fixed the basic test cases (weak mode & removal of "not" from title).

@pushkar100 pushkar100 marked this pull request as ready for review May 31, 2020 19:15
lib/Dependency.js Outdated Show resolved Hide resolved
lib/Dependency.js Outdated Show resolved Hide resolved
lib/FlagDependencyUsagePlugin.js Show resolved Hide resolved
lib/ModuleGraph.js Outdated Show resolved Hide resolved
lib/dependencies/ImportWeakDependency.js Outdated Show resolved Hide resolved
types.d.ts Outdated Show resolved Hide resolved
types.d.ts Outdated Show resolved Hide resolved
@pushkar100 pushkar100 force-pushed the feature/lazy-load-named-exports branch from 8bb314a to 31e3369 Compare June 5, 2020 18:51
@pushkar100
Copy link
Contributor Author

@sokra I've incorporated the type and refactoring changes.

An additional change in WasmFinalizeExportsPlugin.js was required so that jsIncompatibleExports[name] does not throw type related error on receiving an object

@webpack-bot
Copy link
Contributor

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

@sokra sokra merged commit c00fec3 into webpack:master Jun 16, 2020
@sokra
Copy link
Member

sokra commented Jun 16, 2020

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Lazy load only required exports from module
4 participants