-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Fix the WASM Worker cannot start in node.js environment #20188
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -736,6 +736,18 @@ function runIfMainThread(text) { | |
} | ||
} | ||
|
||
function runIfWorkerThread(text) { | ||
if (WASM_WORKERS && PTHREADS) { | ||
return `if (ENVIRONMENT_IS_WASM_WORKER || ENVIRONMENT_IS_PTHREAD) { ${text} }`; | ||
} else if (WASM_WORKERS) { | ||
return `if (ENVIRONMENT_IS_WASM_WORKER) { ${text} }`; | ||
} else if (PTHREADS) { | ||
return `if (ENVIRONMENT_IS_PTHREAD) { ${text} }`; | ||
} else { | ||
return ''; | ||
} | ||
} | ||
|
||
function expectToReceiveOnModule(name) { | ||
return INCOMING_MODULE_JS_API.has(name); | ||
} | ||
|
@@ -1031,3 +1043,7 @@ function getPerformanceNow() { | |
return 'performance.now'; | ||
} | ||
} | ||
|
||
function implicitSelf() { | ||
return ENVIRONMENT.includes('node') ? 'self.' : ''; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if the environment includes both node and web and we run the result on the web? i.e. isn't choosing to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the web environment, it can run with or without the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I see. It looks like we have the same pattern in I wonder if we could find a way to avoid the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this issue is the same as the issue below, it can create a separate PR after merging this to deal with these two issues together. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,51 @@ | ||
// N.B. The contents of this file are duplicated in src/library_wasm_worker.js | ||
// in variable "_wasmWorkerBlobUrl" (where the contents are pre-minified) If | ||
// doing any changes to this file, be sure to update the contents there too. | ||
onmessage = function(d) { | ||
|
||
'use strict'; | ||
|
||
#if ENVIRONMENT_MAY_BE_NODE | ||
// Node.js support | ||
var ENVIRONMENT_IS_NODE = typeof process == 'object' && typeof process.versions == 'object' && typeof process.versions.node == 'string'; | ||
if (ENVIRONMENT_IS_NODE) { | ||
// Create as web-worker-like an environment as we can. | ||
|
||
var nodeWorkerThreads = require('worker_threads'); | ||
|
||
var parentPort = nodeWorkerThreads.parentPort; | ||
|
||
parentPort.on('message', (data) => typeof onmessage === "function" && onmessage({ data: data })); | ||
|
||
var fs = require('fs'); | ||
|
||
Object.assign(global, { | ||
self: global, | ||
require, | ||
location: { | ||
href: __filename | ||
}, | ||
Worker: nodeWorkerThreads.Worker, | ||
importScripts: (f) => (0, eval)(fs.readFileSync(f, 'utf8') + '//# sourceURL=' + f), | ||
postMessage: (msg) => parentPort.postMessage(msg), | ||
performance: global.performance || { now: Date.now }, | ||
addEventListener: (name, handler) => parentPort.on(name, handler), | ||
removeEventListener: (name, handler) => parentPort.off(name, handler), | ||
}); | ||
} | ||
#endif // ENVIRONMENT_MAY_BE_NODE | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like duplication of the code in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this issue is the same as the issue above, can create a separate PR after merging this to deal with these two issues together. |
||
|
||
{{{ implicitSelf() }}}onmessage = function(d) { | ||
// The first message sent to the Worker is always the bootstrap message. | ||
// Drop this message listener, it served its purpose of bootstrapping | ||
// the Wasm Module load, and is no longer needed. Let user code register | ||
// any desired message handlers from now on. | ||
onmessage = null; | ||
{{{ implicitSelf() }}}onmessage = null; | ||
d = d.data; | ||
#if !MODULARIZE | ||
self.{{{ EXPORT_NAME }}} = d; | ||
#endif | ||
#if !MINIMAL_RUNTIME | ||
d['instantiateWasm'] = (info, receiveInstance) => { var instance = new WebAssembly.Instance(d['wasm'], info); receiveInstance(instance, d['wasm']); return instance.exports; } | ||
d['instantiateWasm'] = (info, receiveInstance) => { var instance = new WebAssembly.Instance(d['wasm'], info); return receiveInstance(instance, d['wasm']); } | ||
#endif | ||
importScripts(d.js); | ||
#if MODULARIZE | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Hello from wasm worker! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Hello from wasm worker! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this the case, though? If it arrives from a Node.js or Web API then that means this workaround is necessary, but if not, then if it arrives from our own code, can we send data in the same format?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, reading that again, I think you're saying it's the first? We send data the same way, but Node and the Web end up providing it differently? If so then lgtm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, you are right! 👍