Fix require handling with electron; fixes #11599#11603
Fix require handling with electron; fixes #11599#11603sokra merged 1 commit intowebpack:masterfrom MayaRainer:master
Conversation
|
|
|
For maintainers only:
|
|
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
| }); | ||
| F(output, "chunkFormat", () => { | ||
| if (tp) { | ||
| if (tp.document) return "array-push"; |
There was a problem hiding this comment.
What's the reason behind this change?
There was a problem hiding this comment.
and also fixes
electron-rendererto use the correct chunk format, as it is in a browser and not in a module context.
It's should be in both contexts. So require() should be available and we can use it to load chunk.
There was a problem hiding this comment.
Note that electron-renderer means a BrowserWindow with nodeIntegration: true.
For a BrowserWindow with nodeIntegration: false you should use target: "web".
There was a problem hiding this comment.
The primary reason behind this change was to match previous behaviour. As you can see here, electron-renderer used to use array-push and jsonp, and my changes restore that behaviour.
require() is available, yes; but chunkFormat=commonjs makes it generate a chunk in the format
exports.id = "common";
exports.ids = ["common"];
exports.modules = {
...which doesn't work because there is no module context in the browser and so we get a
common.js:1 Uncaught ReferenceError: exports is not defined
regardless of whether nodeIntegration is turned on (unless I've overlooked something else, of course).
|
Hi @MayaWolf. Just a little hint from a friendly bot about the best practice when submitting pull requests:
You don't have to change it for this PR, just make sure to follow this hint the next time you submit a PR. |
|
Thanks |
5 participants
This fixes electron/index.js being bundled as described in #11599, and also fixes
electron-rendererto use the correct chunk format, as it is in a browser and not in a module context.What kind of change does this PR introduce?
Bugfix.
Did you add tests for your changes?
Not yet
Does this PR introduce a breaking change?
No
What needs to be documented once your changes are merged?
Nothing