tauri icon indicating copy to clipboard operation
tauri copied to clipboard

[feat] Expose async runtime's sleep() function

Open Qix- opened this issue 2 years ago • 9 comments

Describe the problem

Most of the common items you'd need from the async runtime (e.g. Tokio) are adequately exposed from the async_runtime module, and I understand the goal to keep it lightweight. However, one common but missing items is an async sleep(), useful for e.g. debouncing events.

Describe the solution you'd like

It would be great if sleep() is re-exported. Most runtimes have such a function, usually taking a standard Duration (which could also be exported, if need be).

Alternatives considered

Importing the entirety of tokio, which in this case would be very overkill.

Additional context

No response

Qix- avatar Dec 22 '23 17:12 Qix-

Thanks for the request :)

What would be the major advantage over tokio::sleep considering it's the very same thing as a re-export would be? I'd personally prefer to keep our API surface as small as possible even if we're talking about re-exports but idk how the rest of the team thinks.

FabianLars avatar Dec 22 '23 18:12 FabianLars

Well the point of async_runtime is to expose only the most common of things, right? Having to import tokio just for sleep seems excessive given that it's such a common function to use in an async context - so much so that I was surprised it wasn't already there

Qix- avatar Dec 22 '23 20:12 Qix-

Ran into the same problem, would love to have this

Shawak avatar Jan 20 '24 01:01 Shawak

Well the point of async_runtime is to expose only the most common of things, right?

In my opinion no. I'm not a fan of simple re-exports, especially if it means that we have to enable feature flags of the dependency just for tha re-export.

From my understanding this was another instance of "Hey we use something internally, let's export this" which we're reverting in many places in v2.

For example the whole http api on the rust side was written for the js side and quite suboptimal for the rust side. In this case it was replaced with a reqwest re-export which indeed contradicts my pov from above. I'll propose removing that re-export too though mainly because it's still 0.x which means we couldn't upgrade reqwest without a major release.

tl;dr: In my opinion we should remove the re-exports (or at least keep them generally to a minimum), and only keep the few things that actually do something special, so basically just spawn and block_on.

But this is not my decision which is why this issue is still open.

FabianLars avatar Jan 20 '24 14:01 FabianLars

Well the sort of alternative here is that you're forcing people to include tokio and keep the version synchronized with Tauri's, since there's no way to magically include any other async runtime. So by using tauri I'm beholden to the async runtime tauri is using, anyway.

Would it be possible simply to have tauri re-export the whole tokio runtime it uses such that we don't have to version-match? Then we can be assured that our tokio-specific code is going to jive well with whatever Tauri uses underneath. I know this is "exposing the internals" a bit but it's not unheard of in the Rust ecosystem for this exact reason.

It would also end any bikeshedding about what does or does not deserve a re-export. It's just a pub use tokio line.

Qix- avatar Jan 20 '24 15:01 Qix-

Exposing tokio as a whole would mean we'd have to enable all of its features, right? (genuine question). Because that's something i'd like to avoid, tauri's compile times are already a pain and every second counts at this point imo.

About versioning, in case of tokio we're not seeing many major releases so relying on cargo to deduplicate the minor version should be fairly reliable. In tauri we only specify the major version so that endusers can choose whatever version they want. Since tokio is part of the public api either way, we can only change the version in major tauri versions or via feature flags.

It would also end any bikeshedding about what does or does not deserve a re-export. It's just a pub use tokio line.

True but then maybe the bikeshedding about which other crates should be exposed in the same way (first ones i'd think about are the webview bindings since they are a bit more annoying to sync) would begin idk.

Sorry for being difficult about it 😅

FabianLars avatar Jan 20 '24 17:01 FabianLars

Nope, if someone needs extra features you don't already have enabled then that is when they would include tokio (with the same version) and manually enable the extra features Tauri hasn't enabled. Features are additive and cumulative over all transient dependencies.

Assuming the features Tauri needs are enough for the user, then they wouldn't have to add Tokio as a dependency themselves.

EDIT: and no worries, it's not being difficult. These are important decisions and it's the fact the Rust community tends to discuss these things a bit more critically that make Rust great IMO. :)

Qix- avatar Jan 22 '24 08:01 Qix-

Right, i know how it works on the technical side, but i'm not sure what's best for the user i guess. Or maybe differently: How should we decide which features we should enable? Because leaving only those enabled that tauri itself needs is apparently not the right answer too considering this issue exists.

More generally, I think i slightly prefer this flow: "Oh, i need tokio, let's add it to my project with the features i need" over "Oh, i need tokio, let's check if tauri has what i want. Damn, it doesn't have it :("

wow, i'm really bad at explaining what i mean here, hope you still got the gist of it 😂

FabianLars avatar Jan 23 '24 14:01 FabianLars

This issue is for API exposure, not enabled Tokio features. If you re-exported Tokio (pub use ::tokio) and it was missing a Cargo feature then that's on me, the consumer. Enabling all features isn't what I'd personally expect, and I'd work around that in the case Tauri wasn't using one (ideally Cargo would allow us to override sub-dependency features but that's a separate issue with its own caveats).

It's just inconsistent IMO that Tauri re-exports the underlying runtime in part, as there's no clear boundary of what should be exported and what not (contributing in part to this issue 😅), which is totally understandable.

Especially for core utility stuff, we haven't needed much else from Tokio ourselves as most async primitives we need are available without any extra features.

In my personal, fully subjective opinion, Tauri should either decide to expose tokio as a re-export, or not expose it at all and leave it up to the end user to include Tokio If they need it. That being said, re-exporting dependency crates is fairly common for this exact reason - it's free, avoids version mismatching, and is generally sufficient for the majority of usecases. Just use the features Tauri needs, re-export, and people can enable additional features if they really need them. Given Tauri's scope, it'd be corner case scenarios where something isn't already available.

And no worries, it's a good discussion to have :)

Qix- avatar Jan 23 '24 15:01 Qix-