Modify module wrapper so that "process", "global" are shadowable #21
Conversation
In upstream Node the module wrapper doesn't define "process" and "global" so you can write code like `const process = require('process')` without an error due to redefining the variable. Short of updating this entire vendored copy of Node, a simple way I could think of that provides similar semantics is to modify the module wrapper by adding an extra IIFE that omits "process" and "global" from its parameter list.
Afaik that's exactly what we do, this repo tracks upstream node every time we make a major release of Electron |
|
@paulcbetts I did some more digging -- it looks like this Electron-specific commit forked the module wrapper from Node: 8238770 So I guess rebasing on top of upstream wouldn't fix the issue at hand (see electron/electron#8358), there still needs to be a patch like this. |
| '(function (exports, require, module, __filename, __dirname, process, global) { ', | ||
| '\n});' | ||
| '(function (exports, require, module, __filename, __dirname, process, global) { ' + | ||
| 'return function (exports, require, module, __filename, __dirname) { ', |
There was a problem hiding this comment.
@ide is the return statement needed here?
Looks like return value for this function is unused when it is called on line 506
|
@ide apologies for the delay merging this, we were in the middle of upgrading to node 7.4 from node 6.5. This upgrade is complete now, would you mind rebasing/retargeting this change against the |
|
We try to maintain the fewer number of commit patches to node so I've merged this change into the original patch to minimize conflicts going forward. So this was merged directly into commit f691373 Thanks 👍 |
|
Sweet, thanks, keeping the fork commits small and simple totally makes sense. |
In upstream Node the module wrapper doesn't define "process" and "global" so you can write code like
const process = require('process')without an error due to redefining the variable. Short of updating this entire vendored copy of Node, a simple way I could think of that provides similar semantics is to modify the module wrapper by adding an extra IIFE that omits "process" and "global" from its parameter list.Test Plan: Tested this end-to-end inside of Electron itself by building it from source and ensuring the test app loads.