[refactor] replace command macro

[JonasKruckenberg]

Describe the problem

I noticed a recurring theme among discord questions: Issues with commands.

Here is a short list of the most common ones:

• Async cmds must return Result (throws a lifetime error otherwise) see Compile error for async stateful commands that don't return a Result #2533.
• What parameters are available to commands?
• __cmd__<command name> is not defined / __cmd__my_<command name> must be defined only once
• macros are arcane magic, especially for rust beginners

Describe the solution you'd like

While most of this can be addressed through docs, I wanna propose a in-code solution:

rust pseudocode

struct Ctx {
   app: AppHandle,
   window: Window
}

type CmdFn = dyn Fn(InvokeMessage);

fn cmd_sync<F,A,R>(cmd_fn: F) -> Box<CmdFn>
where 
   F: Fn(Ctx, A) -> Result<R, Error> + 'static
   A: DeserializeOwned
   R: Serialize + 'static

fn cmd_async<F,A,R,RV>(cmd_fn: F) -> Box<CmdFn>
where
   F: Fn(Ctx, A) -> R + 'static,
   A: DeserializeOwned,
   R: Future<Output = Result<RV, Error>>,
   RV: Serialize + 'static

Usage example (using the plugin builder proposal #2959 ):

async fn foo(ctx: Ctx, arg: String) -> Result<(), Error> {
   println!("{}", arg);
   Ok(())
}

pub fn init() -> Plugin { // not the Plugin trait
   PluginBuilder::new("example")
      .commands([
         // This is where the above function comes into play
         cmd_async(foo)
      ])
}

This has several advantages over the current situation:

1. It enforces users to return Result, clearly mapping to frontend promise states.
2. Doesn't produce weird lifetime, or variable errors
3. Handlers are heap allocated, this paves the way for one time commands. (A variation of the above allowing FnOnce as arguments)
4. It's easier to both export functions (for rust users) and making them available through IPC.

Incremental Adoption

The command macro can trivially use this new system under the hood, so we don't break existing code.
We could then mark it as deprecated and remove it with v3.

Alternatives considered

Additional context

This heavily inspired by denos op functions: https://github.com/denoland/deno/blob/main/core/ops_json.rs

Related issues:
#3255
#2533
#3029

Features/Use cases to consider

• One time commands
• Abortable commands
• Proper namespacing (<module>.<command>)
• Commands callable from Rust
• Command Scoping (allowing only certain contexts to access certain commands)
• IPC middleware

[JonasKruckenberg]

@FabianLars Gonna ping you here, you have dealt with way more questions, maybe you wanna add something here!

[probablykasper]

Would you have to define commands with all those where arguments all the time?

One other thing that annoys me with the command macro is that rust-analyzer's intellisense seems to kinda just break in command functions

[JonasKruckenberg]

Would you have to define commands with all those where arguments all the time?

No that is just the function signature. I'll update the post with an actual usage example 👍🏻

[amrbashir]

I like enforcing Result but do users have to call cmd_sync() or cmd_async() around each function passed to tauri::generate_handler! macro? if so, it is a bit annoying.
Can't tauri::generate_handler! macro automatically detect whether a function is async or not and call that internally? I think we already have this sort of detection implemented.

[JonasKruckenberg]

I would like to get rid of generate_handler too, IMO it's way better to pass a IntoIterator.

I will refactor the IPC system after v1 anyway to enable removing event handler,1 one time commands and ipc middleware. so that will be easy to accommodate

[JonasKruckenberg]

I would like to get rid of generate_handler too, IMO it's way better to pass a IntoIterator.

As little magic as possible, that's important to me

[FabianLars]

@ amrbashir the idea is to get rid of generate_handler!, but thinking about it maybe it's enough to just drop one of them to fix some of the pain points? Anyway... Edit: Forgot to press enter so jonas already answered lol

I personally like enforcing Result too, but ideally i'd like to see a shortcut to use Option too. Both alternatives, converting Option to Result (lol, no) and Ok(Some(val)) are not that nice...

i don't rly like cmd_sync() + cmd_async(), either. idk, atm i kinda see it as a blocker for this, especially considering that not every tauri user is a rust pro and this would another (small) layer of friction :/
But i also can't rly imagine an alternative to it with the goal of dropping the macros.

Also, i'm not sure if a super complicated command signature is much better than what we got or whether we will just end up trading problems for (new) problems...

@FabianLars Gonna ping you here, you have dealt with way more questions

So far i heard ~3 kinds of questions related to this, all mentioned by you:

1. async State lifetime error
   • Which (hopefully) should be somehow fixable with our current system. Either by actually fixing it or by forcing a Result?
2. pub command in main.rs not workin
   • yeah, i don't reallyyy care about this cause they don't need to be pub anyway (wouldn't clippy also complain about this if it would actually compile?). Maybe this is fixable too? no idea about macros tbh
3. re-use commands as normal function
   • valid in theory, but a) it has an easy workaround by pulling the body out into another function and b) most of the users who asked for this used injected stuff (State,AppHandle,etc) in their command signature, so this change wouldn't be enough for them anyway.

[probablykasper]

Will you still be able to specify multiple arguments as before?

async fn foo(ctx: Ctx, arg1: bool, arg2: String) -> Result<(), Error> {
   println!("{} {}", arg1, arg2);
   Ok(())
}

Would it still work to pass a String as error?

async fn foo(ctx: Ctx) -> Result<(), String> {
   Err("User error message".to_string())
}

And accessing state would have to work like this, correct?

async fn foo(ctx: Ctx) -> Result<(), String> {
   let my_struct: State<MyStruct> = ctx.app.state();
   Ok(())
}

[JonasKruckenberg]

Will you still be able to specify multiple arguments as before?

async fn foo(ctx: Ctx, arg1: bool, arg2: String) -> Result<(), Error> {

   println!("{} {}", arg1, arg2);

   Ok(())

}

No, not like that anyway. Taking multiple arguments would look like this:

// we can use tuples here to accept more arguments than one. 
// all types in the tuple need to implement Serialize of course.
// this is exactly how projects like wasmtime or deno handle typed functions
async fn foo(ctx: Ctx, args: (bool, String)) -> Result<(), Error> {

   println!("{} {}", arg1, arg2);

   Ok(())

}

This will make commands more predictable, so when calling this from the frontend you would need to pass an array of two elements.

invoke("foo", [true, "hello world"])

If users would like a different representation they can use a struct explicitly:

#[derive(Serialize)]
struct Payload {
   arg1: bool,
   #[serde(rename = "bar")]
   arg2: String
}

async fn foo(ctx: Ctx, args: Payload) -> Result<(), Error> {

   println!("{} {}", arg1, arg2);

   Ok(())

}

invoke("foo", {
   arg1: true,
   bar: "hello world"
})

Would it still work to pass a String as error?

async fn foo(ctx: Ctx) -> Result<(), String> {

   Err("User error message".to_string())

}

Yeah! Serialize is implemented for String by default. This will continue to work as-is.

And accessing state would have to work like this, correct?

async fn foo(ctx: Ctx) -> Result<(), String> {

   let my_struct: State<MyStruct> = ctx.app.state();

   Ok(())

}

I don't particularly like that, so I'm hoping to get some feedback from y'all.
Yes the first parameter would be a request context object, but maybe we could make accessing state and such a bit more ergonomic

[oscartbeaumont]

I was sent this issue and wanted to leave some thoughts. I'm sure you have come across many of these limtations already but I wanted to document them all together for anyone coming across this issue. I have been solving a lot of similar problems to this while working on rspc as it's macroless.

Concerns going macroless

Automatic command names

If the Tauri command system were going to go fully macroless it would be impossible to determine the name and the module path (for command namespacing) of the command function.

Eg.

fn demo() {}

fn cmd(func: impl Fn()) {
   // TODO: How do determine the name of function ("demo") here?
}

Alternative Solutions:

• User manually specifies a name when passing the function into the cmd function. Eg. cmd(demo, "demo")

Vardic Arguments Into Object

It was shown in the code from the original issue but not fully explained. It would not be possible for vardic arguments to be converted into an object with named fields. This is because the macro is currently able to read the key of the fields from the TokenStream but Rust doesn't provide a way to do this at runtime.

Eg.

fn demo (some_important_name: String) {}

pub trait Command {}

// This isn't valid Rust because these will conflict but this can be worked around using "generic markers"
impl<T: Fn(TArg1), TArg1:  serde::Serialize> Command for T {}
impl<T: Fn(TArg1, TArg2), TArg1:  serde::Serialize, TArg2:  serde::Serialize> Command for T {}

fn cmd(func: impl Command) {
   // TODO: How to determine the key of the argument/s ("some_important_name") here or from within the `Command` impls?
}

Alternative Solutions:

• Require using either tuples or dedicated types
• Commands with vardic args could be automatically be converted into a tuple instead but this will be confusing.

Combining cmd_sync and cmd_async

It is technically possible to combine them without a macro (proof). However the downside of that is the error messages are not going to be as good because there is multiple valid types for the argument and Rust doesn't know which one the user is attempting to match.

For example in rspc (which uses this style of system) you will get an error like the trait IntoLayerResult<_> is not implemented for type if you forget to implement either serde::Serialize or specta::Type on the result of the function. Given Tauri only needs serde::Serialize the error will be a bit easier to document but it's still a potential concern.

Using separate async and sync functions you will get a more helpful error like help: the trait "Serialize" is not implemented for "MyCustomTypeIForgotToAddSerdeTo" because the Rust compiler knows what the users intentions are.

[JonasKruckenberg]

Thanks for the additional thoughts 👍 I'll put all this together in a different issue properly labeled as an RFC tonight.

Just wanted to add the approximate timeline for the macros:

• v2: Switch to a Router based implementation internally to allow for middleware & one-time-commands. This will be the majority of the work
• some minor after v2: Deprecate the current tauri::command and tauri::generate_handler macros and encourage switching to the router API
• v3 (maybe): remove the deprecated macros

Edit: I much much prefer people setting their own name for commands i.e. cmd("name", || {}) since we've seen time and time again that inferring the name like we do currently isn't that great for DX. People tend to be confused more than anything

[probablykasper]

I much much prefer people setting their own name for commands i.e. cmd("name", || {}) since we've seen time and time again that inferring the name like we do currently isn't that great for DX.

With tauri-specta you get the function exported directly anyway though, so with proper bindings this isn't an issue.

Overall I'm skeptical about deprecating the command macros if it means losing named arguments, requiring Result and more code to get them exported

[JonasKruckenberg]

With tauri-specta you get the function exported directly anyway though, so with proper bindings this isn't an issue.

Yeah that's true but the IPC system has to serve many more users elegantly. Even those that are not using any type generation utility.

Overall I'm skeptical about deprecating the command macros if it means losing named arguments, requiring Result and more code to get them exported

Let me get the proper proposal done and we can feel out the DX. I'm honestly not a big fan on the named arguments as they exist right now (though I appreciate the general idea) as it tends to get really verbose, especially calling commands from Rustwasm. But again we can feel all that out. Same goes for enforcing result.

[probablykasper]

Sure. Don't know how Rustwasm works, but the reason I like the named arguments is that it's less verbose, and imo clearer, than this:

fn command(args: (String, String, bool)) {
  let name = args.0;
  let description = args.1;
  let enabled = args.2;
}

[JonasKruckenberg]

Sure. Don't know how Rustwasm works, but the reason I like the named arguments is that it's less verbose, and imo clearer, than this:

fn command(args: (String, String, bool)) {

  let name = args.0;

  let description = args.1;

  let enabled = args.2;

}

Oh no don't worry, on the Rust side we'll definitely keep functions with more than one argument (implemented through a macro) for up to 16 or so parameters. After that you'd have to group them using tuples or newtypes.

Examples:

fn valid_command() {

}

fn also_valid_command(a: u8, b: String, c: HashMap<String, String>) {

}

async fn valid_too() {}

fn invalid_bc_too_many_args(
   a1: u8,
   a2: u8,
   a3: u8,
   a4: u8,
   a5: u8,
   a6: u8,
   a7: u8,
   a8: u8,
   a9: u8,
   a10: u8,
   a11: u8,
   a12: u8,
   a13: u8,
   a14: u8,
   a15: u8,
   a16: u8,
   a17: u8
) {}

It's a bit sad that we can't implement the trait required for this to work for all functions, but honestly 16 parameters is plenty and real code should not approach that limit anyway (we can maybe have a nice warning for users that let's them know that these are too many parameters. just too many)

[Brendonovich]

With tauri-specta you get the function exported directly anyway though, so with proper bindings this isn't an issue.

@probablykasper It's worth noting that with this new system tauri-specta would not be able to use the name of the function, since the command name is specified at runtime. I've detailed more thoughts over in this issue.

[drernie]

I'll put all this together in a different issue properly labeled as an RFC tonight.

@JonasKruckenberg Is there a link to the relevant RFC somewhere, that I missed?