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

misc/wasm: incorrectly checks for require("fs") validity #40446

Open
tharvik opened this issue Jul 28, 2020 · 5 comments
Open

misc/wasm: incorrectly checks for require("fs") validity #40446

tharvik opened this issue Jul 28, 2020 · 5 comments
Milestone

Comments

@tharvik
Copy link

@tharvik tharvik commented Jul 28, 2020

When loaded, wasm_exec.js tries to know on which platform it is run. "fs" is loaded if not already available and check if it is empty (when using webpack to provide an empty implementation) or not (when running inside node). The check is wrongly implemented as Object.keys returns an array of key, not a count, thus making that check always true, and unable to use with webpack.

The line in question

if (Object.keys(fs) !== 0) {

Which can be easily fixed by adding a .length after the Object.keys(fs).

@cagedmantis cagedmantis changed the title misc/wasm/wasm_exec.js incorrectly checks for `require("fs")` validity misc/wasm: incorrectly checks for require("fs") validity Jul 28, 2020
@cagedmantis cagedmantis added this to the Backlog milestone Jul 28, 2020
@agnivade
Copy link
Contributor

@agnivade agnivade commented Jul 29, 2020

That's right. There's a CL at https://go-review.googlesource.com/c/go/+/241119.

@tharvik
Copy link
Author

@tharvik tharvik commented Jul 29, 2020

Closing as duplicate then, thanks for the pointer.

@tharvik tharvik closed this Jul 29, 2020
@tharvik
Copy link
Author

@tharvik tharvik commented Jul 29, 2020

Oups, missread that, reopening.

@cagedmantis
Copy link
Contributor

@cagedmantis cagedmantis commented Jul 29, 2020

@neelance
Copy link
Member

@neelance neelance commented Jul 30, 2020

The CL will land in the next release cycle.

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

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.