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

feat: support output options plugin #408

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

underfin
Copy link
Contributor

@underfin underfin commented Jan 12, 2024

Description

OutputPlugin binding

The pr intend to find a solution to support OutputPlugin, but here has some issue need to resolve.

The write/generate is async function, it isn't allow has PluginOptions argument, because the JsFunction isn't Send, here has some solution to resolve it.

  • We can using ThreadsafeFunction to replace JsFunction for PluginOptions, see this issue fix: future cannot be sent between threads safely napi-rs/napi-rs#1644, but the ThreadsafeFunction isn't ToNapiValue, we only can be using it directly as argument at the async function, it is ugly for many plugin hook.
  • Replace write/generate to sync function, but it will block node main thread work. Here I also try using AsyncTask to rewrite write/generate function, but it also has issue with the JsFunction isn't Send.

So here is a solution to using a sync function set_output_plugins to pass PluginOptions to rust side before call write/generate. Here use a map to store plugin using the sync function and take it to the write/generate call.

Rust side OutputPluginDriver and OutputPlugin

Here not create new a struct/trait to do that, but can be do for rust API clear. I'm free to implement it if anyone think it should do.

Test Plan

Not need at now.


@underfin
Copy link
Contributor Author

underfin commented Jan 12, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @underfin and the rest of your teammates on Graphite Graphite

milesj
milesj previously approved these changes Jan 12, 2024
@hyf0 hyf0 added the on-hold label Jan 22, 2024
@underfin underfin force-pushed the 01-10-refactor_binding_separate_plugin branch from 8f3440f to c82598f Compare January 31, 2024 06:37
Base automatically changed from 01-10-refactor_binding_separate_plugin to main January 31, 2024 06:39
@underfin underfin dismissed milesj’s stale review March 8, 2024 08:22

The merge-base changed after approval.

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

Successfully merging this pull request may close these issues.

None yet

3 participants