re-export worker_kv::KvStore type about workers-rs HOT 10 CLOSED

Oops, great catch! Thanks for bringing this up. I'll get a patch out ASAP.

from workers-rs.

I agree with this! I had to do the same for a project i am creating using workers-rs!

from workers-rs.

@nilslice is worker_kv actually a cloudflare library?

from workers-rs.

@omarabid - it's not, but, I like the crate.

I'm grateful for the work @zebp has done on it, and think there may be some collaboration we can do to bring some new functionality to it and possibly bring it directly into the workers-rs project. For now though, we'll re-export it in a new release along with some pending PRs once they land.

Do you have any suggestions on how to improve the status quo here? Happy to take input and make things as useful as they can be.

from workers-rs.

@nilslice I think the status quo is broken? It's good that you have tagged this as work-in-progress because having read the blog post first, I got the idea that this a launched/ready product.

Beyond the fact that worker-kv should be part of the workers library, I think there should be some consideration into making types Send + Sync. It's true that this is not required for WASM as it is single-threaded but most Rust libraries, especially the ones working with async are working with Send + Sync futures, and this breaks the chain of async.

I think (I'm not sure) it is "safe" to unsafely implement Send + Sync to the Promise functions bindings; as I'm not aware of a solution to "safely" implement these traits.

Also, the bindings in the workers-rs depends on the context, so my understanding is that I have to be passing the context "around" to be able to re-create the KvStore. This can be tricky as in my case I was working on GraphQl and thus would have to attach these contexts to the schemas that I'm building.

There is another alternative, however. In the previous project I was working on, the only JavaScript code that I used was a request dispatcher; and the rest was all Rust code. In that regard, this crate would reduce to just that: a dispatcher. The developer would then pick a router, kv-utility, etc... It would help if these parts are standardized in some sort; in that regard, you can deploy these workers to several CDN providers (I know this a Cloudflare thing for now :) ) and the only variable is the dispatcher.

from workers-rs.

@omarabid - thanks for the feedback, but I'm not sure I follow what exactly you're referring to and how anything is broken (beyond what we'd consider "in-progress").

Can you elaborate a bit more about where Sync + Send would be effective? Are you using the types from these crates in other projects?

Also, I wouldn't count on there being much effort put in to get Workers (in Rust or otherwise) running outside the platform. Not for any intentional restrictions, but rather there are many custom features of the Workers platform that I would imagine to not be implemented the same elsewhere.

from workers-rs.

Can you elaborate a bit more about where Sync + Send would be effective? Are you using the types from these crates in other projects?

The KvStore return is !Send/!Sync. Since it's an async (future) return, it pollutes the chain of async. This is a problem if you are using other libraries that uses async and instead return Send/Sync futures; as these are not compatible. Implementing (unsafely) Send/Sync is not difficult, see how I do it in this fork of worker-kv (https://github.com/omarabid/worker-kv-2/blob/42571345cf9a5a9957accbcff6193b51bc49587e/src/lib.rs#L40) and (https://github.com/omarabid/worker-kv-2/blob/42571345cf9a5a9957accbcff6193b51bc49587e/src/builder.rs#L12). Alternatively, one can keep boxing/pining these futures in his own program but it gets annoying after a while. I'm not sure of the security implications of such code but my understanding is since WASM is running in a single thread, implementing Send that way should be safe?

Also, I wouldn't count on there being much effort put in to get Workers (in Rust or otherwise) running outside the platform.

I think that's not sustainable for many developers (platform lockin?) at least if one is building more advanced applications; but I might be wrong. I'd like to have the ability to port the code to other platforms with minimum hassle especially that WASM is universal. Currently, I'm only using the KV feature, the routing being abstracted behind GraphQl. So it shouldn't be too hard to teleport the code to any place that runs WASM :)

Again this is just my opinion and how I'd like to do things :)

from workers-rs.

The KvStore return is !Send/!Sync. Since it's an async (future) return, it pollutes the chain of async. This is a problem if you are using other libraries that uses async and instead return Send/Sync futures; as these are not compatible. Implementing (unsafely) Send/Sync is not difficult, see how I do it in this fork of worker-kv (https://github.com/omarabid/worker-kv-2/blob/42571345cf9a5a9957accbcff6193b51bc49587e/src/lib.rs#L40) and (https://github.com/omarabid/worker-kv-2/blob/42571345cf9a5a9957accbcff6193b51bc49587e/src/builder.rs#L12). Alternatively, one can keep boxing/pining these futures in his own program but it gets annoying after a while. I'm not sure of the security implications of such code but my understanding is since WASM is running in a single thread, implementing Send that way should be safe?

This seems more like an issue of my library https://github.com/zebp/worker-kv than of this library. If you could make an issue giving some examples of where the lack of Send and Sync are causing problems I'd gladly accept a PR to add them.

from workers-rs.

The KvStore return is !Send/!Sync. Since it's an async (future) return, it pollutes the chain of async. This is a problem if you are using other libraries that uses async and instead return Send/Sync futures; as these are not compatible. Implementing (unsafely) Send/Sync is not difficult, see how I do it in this fork of worker-kv (https://github.com/omarabid/worker-kv-2/blob/42571345cf9a5a9957accbcff6193b51bc49587e/src/lib.rs#L40) and (https://github.com/omarabid/worker-kv-2/blob/42571345cf9a5a9957accbcff6193b51bc49587e/src/builder.rs#L12). Alternatively, one can keep boxing/pining these futures in his own program but it gets annoying after a while. I'm not sure of the security implications of such code but my understanding is since WASM is running in a single thread, implementing Send that way should be safe?

This seems more like an issue of my library https://github.com/zebp/worker-kv than of this library. If you could make an issue giving some examples of where the lack of Send and Sync are causing problems I'd gladly accept a PR to add them.

Yes, that's true. I'll make an example repo and post it here.

from workers-rs.

These types have been re-exported under kv module within the worker crate. I'm going to close this here, but please feel free to re-open in worker_kv repo.

from workers-rs.