-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
worker_threads.Worker missing argv
and execArgv
option
#4130
Comments
Hi @paperdave, |
it has not been resolved. feel free to go for it. |
@paperdave Just recently started learning zig, but i wanna go for it. Bun is a huge project could you point me in a direction of where this would be best implemented? |
@sp90 already working on this |
@umrkhn if you go for it may i request to support |
@sp90 i don't know on how to implement |
also i was unable to fix this issue so you can take over from here if you want to |
@sp90 , will you give it a go? |
@birkskyum i finally got bun dev env running but have minimal experience using zig and have never worked with C nor C++ so i feel blind going in, do i wanna try yes - can you expect it to be done any time soon, no promises :) So if you are anyone else have a fix, just throw it on here i'll read the code to get a better understanding |
@paperdave , this is labelled as "Good first issue", but it's blocking a significant number of tickets, and has already proven to be hard for first-time contributors to resolve - so if you consider it trivial, I think it would be worthwhile to just get it out of the way. |
Agree, @paperdave and @birkskyum, also tried to look into it, but unable to go anywhere. Its the one thing holding us up using bun across all our system, since it makes cloudflare wrangler fail, e.g. when trying to run tests const { unstable_dev } = require("wrangler");
describe("Worker", () => {
let worker;
beforeAll(async () => {
// error: [bun] Warning: worker_threads.Worker option "execArgv" is not implemented.
worker = await unstable_dev("src/index.js", {
experimental: { disableExperimentalWarning: true },
});
});
afterAll(async () => {
await worker.stop();
});
it("should return Hello World", async () => {
const resp = await worker.fetch();
const text = await resp.text();
expect(text).toMatchInlineSnapshot(`"Hello World!"`);
});
}); |
Adding a case: compiling Angular with the new application builder enabled (esbuild/Vite) requires these as well |
@muuvmuuv , interesting. I believe Analog support is stuck in the same place |
Yes, you are right, Analag also uses this builder under the hood with Vite so I guess this applies to all frameworks with builders that use Vite? |
@muuvmuuv , pretty much yet - I look very much forward to this one, because even though it's not the biggest implementation effort, it touches a wide variety of the FE ecosystem. |
Is one already actively working on this, or a PR that needs review? If it isn't that much of an effort, I wonder how we can push this further to make Bun “useable” to more people, and so on, let more people consider Bun as a Node/npm replacement in their pipeline. |
@muuvmuuv , I've not seen any indication of work on this since the "good first issue" label triggered some attempts that unfortunately didn't make it all the way. |
I found the relevant API file: https://github.com/oven-sh/bun/blob/main/src/js/node/worker_threads.ts#L195 Looking at the I am still looking for the relevant Bun/Zig part that adapts the Node Worker API but could only find this cpp file: https://github.com/oven-sh/bun/blob/main/src/bun.js/bindings/webcore/JSWorker.cpp |
Ok, found this web_worker.zig file but can't tell if it's the relevant file: https://github.com/oven-sh/bun/blob/main/src/bun.js/web_worker.zig Just some hints for someone who wants to work on it, I never programmed in Zig or have exp in building Bun apis. But would love to :D |
@muuvmuuv , there is a contribution guide here that helped me build it locally - https://bun.sh/docs/project/contributing |
Big thanks to @otgerrogla !! 🎉 |
https://nodejs.org/api/worker_threads.html#new-workerfilename-options
argv
modifies the innerprocess.argv
. we can copy howenv
is handled but slightly different semantics. should be an easy issue.execArgv
has more semantics in node.js, since those options can control the runtime. but we dont implement these flags. what we will do for this issue is simply make it visible inprocess.execArgv
, so basically the same asprocess.argv
but i think this is an override instead of an append.these two keys can be added to the global worker, and then the warnings from
src/js/node/worker_threads.ts
can be removed.The text was updated successfully, but these errors were encountered: