fix: exit process when build finishes #985
Conversation
✅ Deploy Preview for rolldown-rs canceled.
|
crates/rolldown_binding/src/options/plugin/binding_plugin_options.rs
Outdated
Show resolved
Hide resolved
crates/rolldown_binding/src/options/plugin/binding_plugin_options.rs
Outdated
Show resolved
Hide resolved
| fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
| f.debug_struct("BindingPluginOptions").field("name", &self.name).finish_non_exhaustive() | ||
| } | ||
| } | ||
|
|
||
| impl FromNapiValue for BindingPluginOptions<true> { |
There was a problem hiding this comment.
Do we still need customization if we already make these option with WEAK: true like you above code?
There was a problem hiding this comment.
Is there a way to make this work in napi_derive?
hmmm, I think I can support the const generic in napi_derive
There was a problem hiding this comment.
Do we still need customization if we already make these option with
WEAK: truelike you above code?
Yes, because we need BindingPluginOptions<true> for main thread plugins and BindingPluginOptions<false> for plugins in worker threads.
If pub struct BindingPluginOptions<const WEAK: bool = false> is flipped to pub struct BindingPluginOptions<const WEAK: bool = true>, then we need impl FromNapiValue for BindingPluginOptions<false> and call cb.refer(env) instead.
There was a problem hiding this comment.
Nah. It's ok we mark main thread plugins WEAK: true.
| @@ -65,18 +65,20 @@ use rolldown_utils::debug::pretty_type_name; | |||
| /// - Rust(Simplified): `MaybeAsyncJsCallback<(Option<String>, i32), Option<i32>>` | |||
| /// - Js: `(a: string | null | undefined, b: number) => Promise<number | null | undefined | void> | number | null | undefined | void` | |||
| /// - Js(Simplified): `(a: Nullable<string>, b: number) => MaybePromise<VoidNullable<number>>` | |||
| pub type JsCallback<Args, Ret> = ThreadsafeFunction<Args, Either<Ret, UnknownReturnValue>, false>; | |||
| pub type JsCallback<Args, Ret, const WEAK: bool = false> = | |||
There was a problem hiding this comment.
| pub type JsCallback<Args, Ret, const WEAK: bool = false> = | |
| pub type JsCallback<Args, Ret, const WEAK: bool = true> = |
I think we could make it as the default.
| let env = &napi::Env::from_raw(env); | ||
|
|
||
| if let Some(cb) = &mut false_options.build_start { | ||
| cb.unref(env)?; |
There was a problem hiding this comment.
no need if WEAK is true https://github.com/napi-rs/napi-rs/blob/main/crates/napi/src/threadsafe_function.rs#L296
|
So let's make |
Actually, this wasn't correct. If I call a WEAK=true threadsafe-function in worker threads, it throws "Rolldown internal error: Closing" error, even if I call |
|
I guess this happens because |
|
This is the code I wanted to write but reverted before I made this PR due to compile errors: 76f912b |
|
I looked into it. The current workaround would be use different struct for main and worker threads. Making NAPI-RS supporting generic could reduce the maintain overhead. @Brooooooklyn |
|
let's on hold this pr before #1003. |
<!-- Thank you for contributing! --> ### Description Workaround of #985. <!-- Please insert your description here and provide especially info about the "what" this PR is solving -->
Description
The cli didn't exit process immediately after the build finished. This PR fixes that.
But I'm not sure why it did exit the process after some seconds even if the thread safe functions are not "weak".
We can add a test by using
why-is-node-running. I didn't add it though.