Implement draft RPC API. #95

tmpfs · 2021-03-03T07:16:13Z

Remove old Callback mechanism.

Remove obsolete Callback API
Remove FuncCall and RPC
Update README
Rename set_rpc_handler() to set_handler()
Use shared rpc_proxy() function for platform consistency
Improve handling of promise cleanup
Update README with RPC API info.
Panic if webview handler is set after window creation.
Improve rpc_proxy() logic, try to ensure any corresponding promise is always removed.
Remove FuncCall wrapper.

Remove old Callback mechanism. * Remove obsolete Callback API * Remove FuncCall and RPC * Update README * Rename set_rpc_handler() to set_handler() * Use shared rpc_proxy() function for platform consistency * Improve handling of promise cleanup * Update README with RPC API info. * Panic if webview handler is set after window creation. * Improve rpc_proxy() logic, try to ensure any corresponding promise is always removed. * Remove FuncCall wrapper.

tmpfs · 2021-03-03T07:22:06Z

@wusyong, this version has a tidy history. Would appreciate it if we could get this merged 👍

I tried and failed to fix the MacOS issue and hope you have more success. I tried using OnceCell to get an &'static reference to the RpcHandler but could not as it is not thread safe. It appears that ultimately it boils down to:

= help: within `[closure@src/application/general.rs:370:52: 380:10]`, the trait `Sync` is not implemented for `*mut winit::platform_impl::platform::observer::CFRunLoopSource`

Which is all the stuff inside the WindowProxy -> ApplicationProxy chain which I know very little about. Any idea how we can get the RpcHandler into did_receive() safely?

wusyong · 2021-03-03T07:25:59Z

We probably need to declare another objc class and it's full of hacks. I can take another day to work it. But for now that's just make it work on linux.

tmpfs · 2021-03-03T07:26:43Z

Thanks @wusyong, it works on Linux do you have a Windows machine to test on? It might work there too!

wusyong · 2021-03-03T07:33:54Z

Sure. Do you mind I cherry pick this commit to feat/rpc? I wish I could work on it right away.

tmpfs · 2021-03-03T07:38:11Z

Sure thing, let's put it in feat/rpc and merge it back into the main branch once we have all three platforms 👍 Let me know if there is anything else I can do to help. Maybe a tracking issue for progress on this feature?

wusyong · 2021-03-03T07:40:49Z

We can keep the discussion on #66.
Currently I am thinking about making Callback in the application module work with rpc.
I will list more in the issue later.

tmpfs · 2021-03-03T07:49:33Z

Currently I am thinking about making Callback in the application module work with rpc.

Oh no, we may have misunderstood each other. I have removed Callback as I don't see the benefit it brings to have a separate Rust closure for each Javascript function. The new model is a single Rust closure which tests on the method of RpcRequest.

The benefit of the old callback model was that you could just call a vanilla Javascript function as I understood it. This can now be achieved like this:

function someCallback() {
  return window.rpc.call('someRpcMethod', Array.prototype.slice.call(arguments, 0));
}

someCallback("param").then((res) => console.log(res));

What is still not done is a function that would set these callback aliases automatically. That would be the set_rpc_aliases() function I mentioned before.

wusyong · 2021-03-03T08:10:01Z

I see. I do agree with this and it looks a lot cleaner.
Btw it turns out it's hell of chore to cherry pick 😅. Could you let me just add them manually?

tmpfs · 2021-03-03T08:20:49Z

Hi @wusyong, sorry I didn't realize you have been working in the feat/rpc branch. I took a look at the commits and there are lots of conflicts with that I have done here. Particularly with regard to CALLBACKS and Callback.

What I attempted to do was be fully backwards compatible with the first PR that you merged and then when you said that you liked the idea of cleaning up FuncCall and Callback this PR was created that completely removes any concept of callbacks with the exception of Javascript-defined callback functions that wrap the RPC API.

Is it possible for me to take any important changes from your feat/rpc branch and merge them with this PR? Not sure which changes are important. Please advise.

Edit: would you like me to merge the feat/rpc branch and resolve the conflicts?

wusyong · 2021-03-03T09:07:57Z

Sorry I deleted the previous comment thinking it's not good for you to develop further.
I think let's just keep working on this branch and finish all the changes you would like to address (ie. RPC alias).
Forget about feat/rpc, I'll update them afterward once this PR is merged.

tmpfs · 2021-03-03T09:38:39Z

Sure @wusyong, I'll do the callback aliases stuff and hope that it works out.

I looked at the tauri code that configures the Callback and all they would need to do is change it to RpcHandler and call set_handler() when the application is set up. But without knowing the code well it is hard to say how much effort it would be.

Maybe we can get some people working on the main Tauri code to comment on how much this change will affect them?

wusyong · 2021-03-03T09:50:30Z

Yeah we'll have a discussion about this.
Another thing I would like to address is maybe moves the handler to parameters of Applicaton::create_window.
I'm not a fan of requiring the correct order of calling methods (which we suffered a lot from original webview)

tmpfs · 2021-03-03T10:15:07Z

I'm not a fan of requiring the correct order of calling methods

Agreed @wusyong . I did add a panic! for out of order calling but it's still a bit rubbish.

So then we remove set_handler() and RpcHandler is passed as the second argument to add_window_with_configs()? Replacing the old Vec<Callback>.

wusyong · 2021-03-03T10:22:42Z

yeah, if that's possible.

Use second argument to add_window_with_configs() to set an RpcHandler.

tmpfs · 2021-03-03T10:43:33Z

Sure thing @wusyong, that's done now and it feels more idiomatic 👍

However it does mean that for the aliases functionality that they would need to be passed in using the same mechanism so we probably want a wrapper type:

pub struct RpcConfig {
  handler: RpcHandler,
  callbacks: Vec<String>
}

Then we pass Option<RpcConfig> to add_window_with_configs() instead of Option<RpcHandler>; is that ok with you?

wusyong · 2021-03-03T10:59:19Z

Current Option<RpcHandler> is cleaner imho. Let me check with other devs if we can actually replace callbacks first.

wusyong

Great news! We could get rid of callbacks entirely! Just fix the following nits and I think it's good to go!

wusyong · 2021-03-03T14:08:28Z

README.md

@@ -46,6 +46,46 @@ cargo run --example multiwindow

 For more information, please read the documentation below.

+## Rust <-> Javascript


I left a comment in #39 that wish we could add descriptions to examples. Maybe you could start a README.md in examples/

examples/rpc.rs

src/application/mod.rs

src/platform/linux.rs

src/webview.rs

tmpfs · 2021-03-04T02:23:12Z

Thanks @wusyong, excited to see this land! I have made those small changes you mentioned but I have a couple more points I would like to make:

We throw out the idea of callback aliases and just add documentation on how to do it in Javascript
I read the Tauri code some more and they do async stuff in the existing Callback code and we need to support that use case.

I have a good idea how we can support async flows it it just a small bit of code and some documentation. I will do it now and ask you to take a look, thanks 👍

So that rust rpc handlers can perform asynchronous tasks and defer promise evaluation until later. If an rpc handler returns None then the handler takes responsibility for ensuring the corresponding promise is resolved or rejected. If the request contains an `id` then the handler *must* ensure it evaluates either `RpcResponse::into_result_script()` or `RpcResponse::into_error_script()`.

Update multiwindow example so it is slightly more illustrative of a real-world use case. Now it launches a window when a button is clicked in the main webview. Multiple windows can be launched and the URL for the new window is passed from the Javascript code.

tmpfs · 2021-03-04T03:39:10Z

Hi @wusyong, I think this can be merged if we can answer the following questions:

Does it work on Windows?
Do we have a clear path to get RpcHandler into did_receive() for MacOS support?

Hoping you are able to work out the MacOS issues! Cheers 👍

examples/multiwindow.rs

tmpfs · 2021-03-04T05:01:57Z

Thanks for all your help @wusyong, shall we get this merged and then over to you for the platform-specific stuff?

wusyong

Sure, really appreciate the help!

Remove set_handler().

180caa6

Use second argument to add_window_with_configs() to set an RpcHandler.

Fix windows type signature.

d88cd8e

wusyong requested changes Mar 3, 2021

View reviewed changes

Tidy obsolete comments and code.

46e1b03

tmpfs added 2 commits March 4, 2021 10:47

tmpfs requested a review from wusyong March 4, 2021 03:30

wusyong reviewed Mar 4, 2021

View reviewed changes

examples/multiwindow.rs Outdated Show resolved Hide resolved

Remove urlencoding from examples.

1dfce3a

tmpfs requested a review from wusyong March 4, 2021 05:04

wusyong approved these changes Mar 4, 2021

View reviewed changes

wusyong merged commit e215157 into tauri-apps:master Mar 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement draft RPC API. #95

Implement draft RPC API. #95

tmpfs commented Mar 3, 2021

tmpfs commented Mar 3, 2021

wusyong commented Mar 3, 2021

tmpfs commented Mar 3, 2021

wusyong commented Mar 3, 2021 •

edited

tmpfs commented Mar 3, 2021

wusyong commented Mar 3, 2021

tmpfs commented Mar 3, 2021

wusyong commented Mar 3, 2021

tmpfs commented Mar 3, 2021 •

edited

wusyong commented Mar 3, 2021

tmpfs commented Mar 3, 2021 •

edited

wusyong commented Mar 3, 2021

tmpfs commented Mar 3, 2021

wusyong commented Mar 3, 2021

tmpfs commented Mar 3, 2021

wusyong commented Mar 3, 2021

wusyong left a comment

wusyong Mar 3, 2021

tmpfs commented Mar 4, 2021

tmpfs commented Mar 4, 2021

tmpfs commented Mar 4, 2021

wusyong left a comment

		@@ -46,6 +46,46 @@ cargo run --example multiwindow

		For more information, please read the documentation below.

		## Rust <-> Javascript

Implement draft RPC API. #95

Implement draft RPC API. #95

Conversation

tmpfs commented Mar 3, 2021

tmpfs commented Mar 3, 2021

wusyong commented Mar 3, 2021

tmpfs commented Mar 3, 2021

wusyong commented Mar 3, 2021 • edited

tmpfs commented Mar 3, 2021

wusyong commented Mar 3, 2021

tmpfs commented Mar 3, 2021

wusyong commented Mar 3, 2021

tmpfs commented Mar 3, 2021 • edited

wusyong commented Mar 3, 2021

tmpfs commented Mar 3, 2021 • edited

wusyong commented Mar 3, 2021

tmpfs commented Mar 3, 2021

wusyong commented Mar 3, 2021

tmpfs commented Mar 3, 2021

wusyong commented Mar 3, 2021

wusyong left a comment

Choose a reason for hiding this comment

wusyong Mar 3, 2021

Choose a reason for hiding this comment

tmpfs commented Mar 4, 2021

tmpfs commented Mar 4, 2021

tmpfs commented Mar 4, 2021

wusyong left a comment

Choose a reason for hiding this comment

wusyong commented Mar 3, 2021 •

edited

tmpfs commented Mar 3, 2021 •

edited

tmpfs commented Mar 3, 2021 •

edited