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

Improve ThreadsafeFunction API #1307

Open
devongovett opened this issue Sep 11, 2022 · 2 comments
Open

Improve ThreadsafeFunction API #1307

devongovett opened this issue Sep 11, 2022 · 2 comments

Comments

@devongovett
Copy link
Contributor

The current ThreadsafeFunction API has a major limitation: due to the way it's designed you cannot access the return value of the JS function. That's because the callback API only returns the parameters for the function, and calls the JS function internally, rather than allowing the Rust program to call the JS function directly.

Rather than returning a Vec of arguments to call the function with, I'd like the ThreadSafeCallContext to include the function itself, so that the callback can call the function directly. This would allow the return value to be retrieved, and sent back to the calling thread (e.g. via a channel).

Pseudo code:

let tsfn = callback.create_threadsafe_function(0, |ctx| {
  let result = ctx.callback.call(None, &[...]);
  // do something
})

For Lightning CSS, I forked threadsafe_function.rs into a version that supports this. You can see the result here. I'd love to upstream this back into napi-rs itself if you are open to the change, but I'm not sure how to do so without breaking changes. I guess it could either be a new API, a mode, a feature flag, or something else. Let me know if any of these would be ok, and I'm happy to send a PR!

@alcuadrado
Copy link

Hey, @devongovett! We are hitting the same limitation on our project. Is your threadsafe_function.rs module used in the production version of Lightning CSS? We are interested in reusing it.

@Wodann
Copy link
Contributor

Wodann commented Oct 30, 2022

We are using the referenced changes, but adapted to accept any return type - using generics.

The Rust source file can be found here: https://github.com/NomicFoundation/hardhat/pull/3182/files#diff-48f3f1c571f251c917a28b7488a53a7347070eff553d9da0a74ea740f79ae333

PR NomicFoundation/hardhat#3182

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

No branches or pull requests

3 participants