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(core): support generics (especially Param) in #[command] #1622

Merged
merged 13 commits into from May 5, 2021

Conversation

chippers
Copy link
Member

@chippers chippers commented Apr 26, 2021

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • New Binding Issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes. Issue #___
  • No

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix: #xxx[,#xxx], where "xxx" is the issue number)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:
In #[command(with_window)] commands, you could not specify a type for Event or Label, making any function that uses those associated types unusable from within that command function. Without those associated types specified, the developer will run into errors like "Expected type Params::Event but received struct String" when using things like window.emit(...). Previously, the following code could not work:

#[command(with_window)]
pub fn import_csv(
  window: tauri::Window<impl tauri::Params<Event = String>>,
  event: String,
  payload: Option<String>,
) -> Result<(), String> {
  window
    .emit(&"output".into(), Some("test".to_owned()))
    .expect("failed to emit");

  return Ok(());
}

This is because the import_csv_wrapper function generated by #[command] would always use <P: ::tauri::Params> as the generic.

With this first commit in this draft, the above code works because the following is now true:

  • The generic parameter is taken from the generic parameter used inside the first argument's type.
    • It expects the first parameter to be a Type::Path (like a struct). (unlikely to change)
    • That Type::Path must have exactly 1 generic parameter. (unlikely to change)
    • That 1 generic parameter MUST be an impl Trait of impl tauri::Params. This will change before the PR is merged. This is a side effect of testing the core changes before I flesh out support for all ways of specifying generics. The following will be supported:
      • fn my_command<P: Params<Event = String>>(window: Window<P>)
      • fn my_command<P>(window: Window<P>) where P: Params<Event = String>
      • fn my_command(window: Window<impl Params<Event = String>>)
      • any missing?

Note: The code is pretty shaky and only accepts a happy path. This was done to test the proof of concept and to make sure it actually solves what it set out to do. At this point, I am relatively confident that this approach will work and the code will be refactored


Tasks to do before the PR is merged:

  1. Refactor the error handling of the tauri-macros crate to allow better readability with full error handling
  2. Accept all ways of specifying a trait and its associated types (or at least the 3 above).
  3. use static_assertions and impls crate for compile-time detection of types/traits, so that we can spit out more specific errors if the wrong struct or trait is input into window: Window<P>

@chippers
Copy link
Member Author

chippers commented May 4, 2021

This is still marked as a draft because there is still 1 example command that doesn't work with type inference without changes. Ideally I want to support it, so I will look into ways to help inference it which may or may not involve small changes to CommandArg.

command that doesn't work:

#[tauri::command]
fn close_splashscreen<P: Params>(
splashscreen: State<'_, SplashscreenWindow<P>>,
main: State<'_, MainWindow<P>>,
) {
// Close splashscreen
splashscreen.0.lock().unwrap().close().unwrap();
// Show main window
main.0.lock().unwrap().show().unwrap();
}

error[E0282]: type annotations needed
  --> examples/splashscreen/src-tauri/src/main.rs:54:3
   |
54 |   #[tauri::command]
   |   ^^^^^^^^^^^^^^^^^ cannot infer type for type parameter `P` declared on the function `close_splashscreen`
...
77 |       .invoke_handler(tauri::generate_handler![close_splashscreen])
   |                       -------------------------------------------- in this macro invocation
   |
   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

It's not able to tell that the P in State is the same as the P in CommandItem. Ideally this is something I can fix on the impl CommandArg for State but might need to tweak the trait slightly if not.

Note: that if you add a type that can successfully inference, then the error goes away e.g. _window: tauri::Window<P>.

I'm also curious if we could implement CommandArg for Params which just returns a () to make it an easy hack to add inference. _: P

@chippers chippers changed the title wip: param argument proof of concept for #[command] wip: support generics (especially Param) in #[command] May 4, 2021
@chippers
Copy link
Member Author

chippers commented May 4, 2021

I solved the type inference by making a StateP which adds a P: Params generic parameter. Not sure if this should replace State instead and always require the P: Params so I've kept this marked as a draft until a decision is made.

edit: It probably shouldn't be replaced, so the draft question is if we should keep the StateP helper

because we recommend `_: Window<P>` for type inference, the following
function types are now supported:
* Pat::Wild (arg: "_")
* Pat::Struct (arg: final path segment)
* Pat::TupleStruct (arg: final path segment)
@chippers chippers marked this pull request as ready for review May 4, 2021 23:18
@chippers chippers requested review from a team May 4, 2021 23:18
@chippers
Copy link
Member Author

chippers commented May 4, 2021

StateP has been removed.

The following command function arguments now work:

  • Pat::Wild (e.g. _: Window<P>)
  • Pat::Struct (e.g. Person { name, age }: Person where the key is the last path segment "Person")
  • Pat::TupleStruct (e.g. InlinePerson(name, age): InlinePerson where the key is the last path segment "InlinePerson")

@lucasfernog lucasfernog changed the title wip: support generics (especially Param) in #[command] feat(core): support generics (especially Param) in #[command] May 5, 2021
Copy link
Member

@lucasfernog lucasfernog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanna kiss this code; can you just add the change file?

@lucasfernog
Copy link
Member

Also, we'll need to work on documentation (e.g. the _: Window<P> trick).

@chippers chippers requested a review from a team as a code owner May 5, 2021 15:57
@lucasfernog lucasfernog merged commit 1453d4b into dev May 5, 2021
@lucasfernog lucasfernog deleted the fix/command-macro-generics branch May 5, 2021 17:32
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

Successfully merging this pull request may close these issues.

None yet

2 participants