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

Add Buffer to args array for Electron require #10

Merged
merged 1 commit into from
Aug 4, 2019
Merged

Add Buffer to args array for Electron require #10

merged 1 commit into from
Aug 4, 2019

Conversation

ronwang01
Copy link
Contributor

Electron's module wrapper require an addtional global Buffer.

This was the PR where Electron added the additonal Buffer to the node module loader
electron/electron#8605

https://github.com/electron/node/blob/18aeda2077e7fe1ef9d7557692dda2e4d6a61860/lib/internal/modules/cjs/loader.js#L136

@tommoor
Copy link

tommoor commented May 14, 2019

Any chance of this getting merged? The module can't be used in Electron without it

@xxczaki
Copy link

xxczaki commented Jul 30, 2019

ping @zertosh

@xxczaki
Copy link

xxczaki commented Jul 31, 2019

To @tommoor and everyone waiting for this PR to get merged:

I created a copy of this package, which includes changes made in this Pull Request 😄

https://www.npmjs.com/package/@akepinski/v8-compile-cache

Note: This is just a temporary solution.

@zertosh
Copy link
Owner

zertosh commented Jul 31, 2019

Sorry, I take a look in the next day or two. I need to understand what the consequence of this is for non-Electron environments.

@xxczaki
Copy link

xxczaki commented Jul 31, 2019

@zertosh Thanks!

@zertosh
Copy link
Owner

zertosh commented Aug 4, 2019

Electron 2.x's Node fork, includes Buffer in the module wrapper (link), and also calls the compiled wrapper with Buffer (link). Electron 4.x's Node fork does the same (wrapper, call). But, Electron 6.x's Node fork no longer includes Buffer in the wrapper args (link), or process or global for that matter. This is consistent with regular Node 12.x behavior (link). However, there seems to be an oversight in Electron, where the wrapper is still called with process, global and Buffer (link). This doesn't matter for this change, just documenting my observation. AFAICT, there's no harm in calling the wrapper with extra arguments. JavaScript just ignores them, plus, arguments isn't accessed anywhere.

@zertosh zertosh merged commit 1a851b2 into zertosh:master Aug 4, 2019
@zertosh
Copy link
Owner

zertosh commented Aug 4, 2019

Published as v8-compile-cache@2.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants