opt out of Send for wasm32

[insipx]

I'm working on adding a Wasm SQLite Backend to Diesel with the newly released wa-sqlite.

Lots of functions for wa-sqlite are async and cannot be made sync in a wasm environment through blocking (b/c web). This means i've turned to this crate to try and continue to get it working, but this crate requires Send for it's Connection/diesel traits. This PR relaxes that condition for wasm32 environment.

[insipx]

Haven't touched this part of the code but CI for rust beta doesn't pass w/o this. LMK if you want me to revert

[weiznich]

Thanks for opening this PR. Can you explain why you need to relax this bound on a technical level. So what exactly does not work if that bound is there? I can understand that it might be feel duplicated on a plain wasm32 target but:

• I would really like to not have different behavior on different platforms if somewhat possible
• Even the wasm32-unknown-unknow target now allows some sort of threading via web-workers, although that requires setting a few additional target feature flags while compiling.

[insipx]

@weiznich wasm-bindgen futures aren't Send, but async-trait requires Send by default. So, for instance if I try returning the future created with a wasm-bindgen JS Export in Connection::establish, it will return a future that's !Send and won't satisfy the requirements for Connection.

I understand the hesitation of having different behavior on different platforms, and I'd be OK leaving this as a duplicated/temp workaround until the diesel-wasm-sqlite backend becomes more full-featured. However, it would be nice to eventually have something like this. I think it's also common practice in other libraries, if that makes it any better.

Maybe it would be worth putting a warning in the readme that the wasm target is experimental?

[weiznich]

Thanks for providing that context. That's at least helpful to understand why you made this change. For me that raises a more fundamental question: Why do you believe that the AsyncConnection trait is the better solution there instead of using the Connection trait from diesel itself? I mean if you don't have threads at all there is no way to do something like spawn_blocking + sqlite only offers a blocking API at all. Given that I personally do see no advantage of trying to use an async interface there at all.

Maybe it would be worth putting a warning in the readme that the wasm target is experimental?

Platform support for diesel essentially boils down to: Officially supported is what we test in CI (in terms of target, platform, database system). Anything else is not supported. It might work, but we don't give any guarantee about it. I think that is quite similar to what most other projects do.

[insipx]

Thanks for providing that context. That's at least helpful to understand why you made this change. For me that raises a more fundamental question: Why do you believe that the AsyncConnection trait is the better solution there instead of using the Connection trait from diesel itself? I mean if you don't have threads at all there is no way to do something like spawn_blocking + sqlite only offers a blocking API at all. Given that I personally do see no advantage of trying to use an async interface there at all.

Yeah this is actually a very interesting problem. Sqlite does have only blocking calls, but the wa-sqlite has a largely Promise-based API that translates to Rust futures with Wasm Bindgen. For instance, consider the exec function of wa-sqlite, or open_v2 which both return Promises. Even the 'sync' wasm build uses Promise-based functions.

As for why that is, i'm not entirely sure. There's probably an answer somewhere in wa-sqlite discussions

According to this discussion, it may be something to do with browser APIS like OPFS which are strictly async

[weiznich]

So just to summarize everything: It's basically required as wa-sqlite bindings generate futures that are not Send, right?

[insipx]

So just to summarize everything: It's basically required as wa-sqlite bindings generate futures that are not Send, right?

Yes, pretty much :)

Although I would generalize this -- it's not that wa-sqlite alone generates !Send futures, but anything wasm-bindgen generates !Send futures.

There is a recent discussion that popped up on the topic of a sync wa-sqlite here. It looks out of scope for the current wa-sqlite creators, but it would definitely be interesting to watch the progress in the future

[weiznich]

I had some time thinking about this again. As previously mentioned I do not like the idea to couple this on the target. In the end it's something that depends on the underlying connection implementation, not on the target.

Given that I would like to propose a different solution: We move away from using #[async_trait] and instead have an explicit associated type per function returning a future. That associated type would then be only require Future, but it would allow implementations to opt into the + Send by using Box<dyn Future + Send>. I think for AsyncConnection that should be relatively straight forward. For traits building on top of these it will be a bit more complicated. I expect that this will either require to specify complex contaminator types there or having a custom future type so that the compiler can automatically infer if the returned type is Send or not. Being able to just use impl Future there would be great, but it seams like you cannot use it for associated types yet. For trait functions it doesn't "leak" the Send bound as it's the case for free standing functions, which in turn makes it not usable there.

[insipx]

That makes sense, and overall a more robust solution. Even with this PR, I had to end up replicating a bunch of the trait definitions to fit them into WASM.

Since I don't have the bandwidth to work on this myself right now, can close this and I opened #185 instead