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

Delete Buffer global when node integration is disabled #8605

Merged
merged 6 commits into from Feb 14, 2017

Conversation

Projects
None yet
2 participants
@kevinsawicki
Contributor

kevinsawicki commented Feb 6, 2017

This was originally done in #7114 but was pulled back because at the time there wasn't a known way to do this without breaking existing apps and libraries.

This pull request builds on the change done in #8539 for process and global by adding Buffer as an argument to the require wrapper function.

The new wrapper node patch is at electron/node@439dbac

The diff is:

diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js
index 65567de..085ea00 100644
--- a/lib/internal/bootstrap_node.js
+++ b/lib/internal/bootstrap_node.js
@@ -512,7 +512,7 @@
   };
 
   NativeModule.wrapper = [
-    '(function (exports, require, module, __filename, __dirname, process, global) { ' +
+    '(function (exports, require, module, __filename, __dirname, process, global, Buffer) { ' +
     'return function (exports, require, module, __filename, __dirname) { ',
     '\n}.call(this, exports, require, module, __filename, __dirname); });'
   ];
@@ -529,7 +529,7 @@
         lineOffset: 0,
         displayErrors: true
       });
-      fn(this.exports, NativeModule.require, this, this.filename, undefined, process, localGlobal);
+      fn(this.exports, NativeModule.require, this, this.filename, undefined, process, localGlobal, localGlobal.Buffer);
 
       this.loaded = true;
     } finally {
diff --git a/lib/module.js b/lib/module.js
index ff24970..1bc27c0 100644
--- a/lib/module.js
+++ b/lib/module.js
@@ -565,7 +565,7 @@ Module.prototype._compile = function(content, filename) {
   }
   var dirname = path.dirname(filename);
   var require = internalModule.makeRequireFunction.call(this);
-  var args = [this.exports, require, this, filename, dirname, process, global];
+  var args = [this.exports, require, this, filename, dirname, process, global, global.Buffer];
   var depth = internalModule.requireDepth;
   if (depth === 0) stat.cache = new Map();
   var result = compiledWrapper.apply(this.exports, args);

Closes #7081

/cc @ide

@ide

This comment has been minimized.

Show comment
Hide comment
@ide

ide Feb 6, 2017

Contributor

Nice!

Contributor

ide commented Feb 6, 2017

Nice!

@kevinsawicki kevinsawicki changed the title from [WIP] Delete Buffer global when node integration is disabled to Delete Buffer global when node integration is disabled Feb 13, 2017

@kevinsawicki kevinsawicki merged commit 624e44d into master Feb 14, 2017

7 of 9 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
electron-linux-arm Build #5509346 succeeded in 67s
Details
electron-linux-ia32 Build #5509347 succeeded in 62s
Details
electron-linux-x64 Build #5509348 succeeded in 134s
Details
electron-mas-x64 Build #3421 succeeded in 8 min 5 sec
Details
electron-osx-x64 Build #3428 succeeded in 8 min 14 sec
Details
electron-win-ia32 Build #2432 succeeded in 8 min 6 sec
Details
electron-win-x64 Build #2416 succeeded in 7 min 57 sec
Details

@kevinsawicki kevinsawicki deleted the no-more-global-buffer branch Feb 14, 2017

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