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: library module without export statement #18176

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hai-x
Copy link
Contributor

@hai-x hai-x commented Mar 11, 2024

fixes #18056

What kind of change does this PR introduce?

When library === module and providedExport === false we also flag exportsInfo by FlagDependencyExportsPlugin but only for the entry module. Not sure if this is the optimal solution, but I tested it and it is indeed feasible.

Did you add tests for your changes?
Yes

Does this PR introduce a breaking change?
No

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)

const { library } = output;
if (providedExports === false && library && library.type === "module") {
// only flag the entry module
let connection = Array.from(moduleGraph.getIncomingConnections(module));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there any other better way to determine whether it is an entry module during finishModules stage?

@alexander-akait
Copy link
Member

/cc @sokra

options.optimization.providedExports ||
(options.optimization.providedExports === false &&
options.output.library &&
options.output.library.type === "module")
Copy link
Member

Choose a reason for hiding this comment

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

I dont' think we should rely on such things, we need to improve logic inside the plugin itself

Copy link
Contributor Author

@hai-x hai-x Mar 21, 2024

Choose a reason for hiding this comment

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

Yes, i think this would also destroy the meaning of the config, but 'libraryType: module' does rely on the module's exportsInfo, maybe we need to simply record export by sth like 'exportsInfo' for 'output: module'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have any other suggestions? I'd be glad to read the related code and fix this issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, i think this would also destroy the meaning of the config, but 'output: module' does rely on the module's exportsInfo, maybe we need to simply record export by sth like 'exportsInfo' for 'output: module'.

And it seems that users rarely set 'providedExport' to false because of the performance issues of it.

Copy link
Member

Choose a reason for hiding this comment

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

I will look deeply later

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.

when setting optimization.providedExports to false, outputModule wouldn't generate export statement
3 participants