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

[Proposal]: Plugin Builder #2959

Closed
JonasKruckenberg opened this issue Nov 25, 2021 · 11 comments
Closed

[Proposal]: Plugin Builder #2959

JonasKruckenberg opened this issue Nov 25, 2021 · 11 comments

Comments

@JonasKruckenberg
Copy link
Contributor

JonasKruckenberg commented Nov 25, 2021

This proposal is not finished yet! Please participate by reviewing the proposed API.

Is your feature request related to a problem? Please describe.

There are several observations I made about the current Plugin trait that I would like to address:

  1. Defining a custom struct for a plugin has little realworld use as plugin state is better stored using the app.manage and app.state methods. (This state is also accessible in commands while plugin struct fields are not)
  2. the Plugin method names are not super self explanatory and naming is somewhat inconsistent (created is an event callback but is not named on_created while on_event and on_page_load are)
  3. The practice of having an invoke_handler field and a extend_api to register commands is not obvious
  4. In "real-world" usage (the official plugins) the config property taken from the tauri.conf.json file had little use and makes plugins more confusing rather than easier to use. It has shown that exporting a builder and having configuration "in-code" is better.

In general I think the current Plugin api leads to unnecessarily verbose and complex plugins, so I would like to propose an improved builder based API:

Describe the solution you'd like
Introduce a PluginBuilder struct to streamline the plugin creation:

/// Mock command
#[command]
async fn execute<R: Runtime>(_app: AppHandle<R>) -> Result<String> {
  Ok("success".to_string())
}

pub fn init() -> Plugin { // not the Plugin trait
   PluginBuilder::new("example")
      .commands(vec![execute])
      .setup(|app| {
         app.manage(MyState::default());
         Ok(())
      })
}

Instead of the current api:

/// Mock command
#[command]
async fn execute<R: Runtime>(_app: AppHandle<R>) -> Result<String> {
  Ok("success".to_string())
}

pub struct ExamplePlugin<R: Runtime> {
  invoke_handler: Box<dyn Fn(Invoke<R>) + Send + Sync>,
}

impl<R: Runtime> Default for ExamplePlugin<R> {
  fn default() -> Self {
    Self {
      invoke_handler: Box::new(tauri::generate_handler![execute]),
    }
  }
}

impl<R: Runtime> Plugin<R> for ExamplePlugin<R> {
  fn name(&self) -> &'static str {
    "example"
  }

  fn initialize(&mut self, app: &AppHandle<R>, _config: JsonValue) -> tauri::plugin::Result<()> {
    app.manage(MyState::default());
    Ok(())
  }

  fn extend_api(&mut self, message: Invoke<R>) {
    (self.invoke_handler)(message)
  }
}

The proposed builder methods closely resemble the traits methods, with a few differences:

  • rename methods to better communicate their purpose
    js instead of initiailization_script, commands instead of extend_api and setup instead of initialize.
  • name event callbacks consistently
    with the prefix on_
trait PluginBuilder {
   // creates the builder using the plugin `name`
   fn new(name: &'static str) -> Self;
   // Renamed `initialization_script` fn to better communicate it takes a JavaScript string
   fn js(&mut self, js_code: String) -> &mut self;
   // Register an array of tauri commands. This hides all the ugly Boxing and macro usage
   fn commands(&mut self, commands: Vec<?>) -> &mut self;
   // Renamed `initialize` fn to highlight the similarity to the `AppBuilder`s setup fn
   fn setup(setup: Fn(&AppHandle<R>) -> Result<(), Box<dyn Error + Send>> + Send + 'static);
   // set the `on_event ` method
   fn on_event(cb: Fn(&AppHandle<R>, &Event)) -> &mut self;
   // set the `on_page_load` method
   fn on_page_load(cb: Fn(Window<R>, payload: PageLoadPayload)) -> &mut self;
   // set the `created` method
   fn on_webview_ready(cb: Fn(Window<R>)) -> &mut self;
   // builds the plugin
   fn build(self) -> Plugin; // not the Plugin trait
}

The PluginBuilder struct could just be wrapper returning Plugin trait implementations, so this proposal would have a clear implementation path.

Notes

  1. Having a builder struct allows us to improve and extend the plugin API in the future with less hassle
  2. I propose plugins exporting a creation function called init (as shown in the example above). This function is free to receive any arguments, so plugin authors may use this to provide configuration options for end users.
  3. I'm considering to remove the js/initialization_script methods all together. Plugins should expose corresponding js packages and not rely on extending the global object. The overwhelming majority of tauri projects also uses bundlers which make providing api packages is a no-brainer. For the few projects that don't, plugin can provide prebuilt browser bundles. See tauri-plugin-store for an example of this.

Describe alternatives you've considered
Leave the trait as is.

Additional context
The builder pattern is inspired by deno_cores ExtensionBuilder.
The initfunction is inspired by denos extensions and the convention among rollup plugins to export a single creation function.

@amrbashir
Copy link
Member

amrbashir commented Nov 25, 2021

I think this is a good approach overall but the naming of initialization_scripts to js will be more ambiguous IMO and it will break the consistency with same method in Wry and Tauri.

@JonasKruckenberg
Copy link
Contributor Author

JonasKruckenberg commented Nov 25, 2021

I think this is a good approach overall but the naming of initialization_scripts to js will be morr ambiguous imo and it will break the consistency with same method in Wry and Tauri.

Yeah I'm not happy with that name either. What do you think about this note:

I'm considering to remove the js/initialization_script methods all together. Plugins should expose corresponding js packages and not rely on extending the global object.

@amrbashir
Copy link
Member

I must have missed that note.

I think initialization_scripts should not be removed, it is essential IMO. it can be useful in so many different ways other than just providing APIs in the global scope. An example of this endless possibility is what if Tauri doesn't provide data-tauri-drag-reigon ? someone can make a plugin and add a click event listener in initialization_scripts to make his custom data-tauri-drag-reigon.

@lucasfernog
Copy link
Member

Recently I wrote a plugin for sentry and custom titlebar and they need the initialization script.

Also I think commands should be renamed to invoke_handler so it looks like the tauri::Builder API.

@JonasKruckenberg
Copy link
Contributor Author

JonasKruckenberg commented Nov 25, 2021

I must have missed that note.

I think initialization_scripts should not be removed, it is essential IMO. it can be useful in so many different ways other than just providing APIs in the global scope. An example of this endless possibility is what if Tauri doesn't provide data-tauri-drag-reigon ? someone can make a plugin and add a click event listener in initialization_scripts to make his custom data-tauri-drag-reigon.

I guess there are situations like this where accessing the global is nice, you're right.

Also I think commands should be renamed to invoke_handler so it looks like the tauri::Builder API.

I have been struggling with making the api like sketched out above work. (The macro does not work with a vec as input) so this makes sense yes.

I have one question though: We now export these 3 things (and others) from the plugin module Plugin (the trait), Builder (the plugin builder) and TauriPlugin (the struct that the builder returns, that implements Plugin)
But TauriPlugin is a horrible name. Any ideas?

This was referenced Nov 25, 2021
@lucasfernog
Copy link
Member

I like the idea of the plugin builder returning a trait impl btw. I think we can improve the trait usage in the future, allowing the plugin's self instance to be accessed.

@JonasKruckenberg
Copy link
Contributor Author

JonasKruckenberg commented Dec 1, 2021

I don't like the naming right now though, plugin::Builder is fine, but plugin::TauriPlugin is awful.

use tauri::plugin::{Builder, TauriPlugin};

pub fn init() -> TauriPlugin {
   Builder::new("foo").build()
}

just feels weird, I'd prefer:

use tauri::plugin::{Builder, Plugin};

pub fn init() -> Plugin {
   Builder::new("foo").build()
}

accessing the builder could even be a static method on the plugin struct or something:

use tauri::plugin::Plugin;

pub fn init() -> Plugin {
   Plugin::new("foo").build()
}

This would mean renaming the trait though, so a breaking change. Wdyt @lucasfernog ?

Edit: I updated the plugin documentation accordingly, so once we resolve this naming question the branch can be merged!

@probablykasper
Copy link
Member

I think renaming initialization_script to js is still pretty unclear, I'd suggest js_init_script.

Maybe the trait could be plugin::PluginInterface

@amrbashir
Copy link
Member

How about keeping Plugin name for the trait and GenericPlugin for the plugin type returned by the builder?

@JonasKruckenberg
Copy link
Contributor Author

I like GenericPlugin it leaves room for other types of plugins in the future!
I have no strong preference about js_init_script vs initialization_script, the former is easier to type imo.

@JonasKruckenberg
Copy link
Contributor Author

Let's move the discussion to the PR everyone!

lucasfernog added a commit that referenced this issue Feb 7, 2022
Co-authored-by: Amr Bashir <48618675+amrbashir@users.noreply.github.com>
Co-authored-by: Lucas Nogueira <lucas@tauri.studio>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants