Skip to content

Conversation

fi3ework
Copy link
Contributor

What kind of change does this PR introduce?

Adding fallback external type for "module-import" type when "module" and "import" are neither applicable.

In #18620, implement "module-import", but the premise is that we're using import or import() for external request, However, for example, if we use require('...'), "module-import" will failed to know which externals type to adopt ("import", "module", or even "node-commonjs"), and it will crash.

The possible options we may want to fall back to:

  1. node-commonjs: to let require('path') code could run in Node.js + ESM (this is widely used).
  2. import: possibly for some specific reason.
  3. module: possibly for some specific reason.
  4. commonjs: level the require there, to be bundled by a bundler.

In this PR, using externalsPresets to indicate which external to use as the fallback:

  1. when the exteranlsPresets is node.js related (electrons, node, nwjs): use node-commonjs
  2. when the exteranlsPresets is web: use module
  3. when the exteranlsPresets is webAsync: use import
  4. when exteranlsPresets is not assigned or derived, use commonjs

Since the implementation coupled the fallback with externalsPresets, when users want to set all ESM import/import() to module-import and leave other CJS require to node-commonjs, they need to override the node.js built-in externals type to module-import defined by externalsPresets.

There's a left TODO is when user setting multiple externalsPresets category, e.g. { node: true, web: true }. Which one should we choose to use as fallback. Currently using a simple prioritization module > import > node-commonjs.

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?

Update webpack/webpack.js.org#7345 after merged.

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

@fi3ework fi3ework force-pushed the module-import-fallback branch from 689c2cb to 3b52d6d Compare August 11, 2024 10:28
@fi3ework fi3ework force-pushed the module-import-fallback branch from 3b52d6d to c951c98 Compare August 13, 2024 04:10
@alexander-akait
Copy link
Member

Currently using a simple prioritization module > import > node-commonjs.

I am fine with it

@alexander-akait
Copy link
Member

Thank you

@fi3ework
Copy link
Contributor Author

Thank you, I'll update the doc tonight.

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.

3 participants