wasm-bindgen icon indicating copy to clipboard operation
wasm-bindgen copied to clipboard

Split up to `wasm-bindgen` and to a new `wasm-bindgen-core`

Open 05storm26 opened this issue 3 years ago • 3 comments

Motivation

Break the ahash dependency cycle:

error: cyclic package dependency: package ahash v0.7.4 depends on itself. Cycle:
package ahash v0.7.4
... which is depended on by hashbrown v0.11.2
... which is depended on by indexmap v1.7.0
... which is depended on by serde_json v1.0.64
... which is depended on by wasm-bindgen v0.2.74
... which is depended on by js-sys v0.3.51
... which is depended on by getrandom v0.2.3
... which is depended on by ahash v0.7.4

Currently getrandom uses wasm-bindgen just to be able to use:

The contents of:

Due to feature unification, several crates end up enabling features inside wasm-bindgen and its downstream dependencies that causes a dependency cycle that affects various parts of the rust community:

https://github.com/tkaitchuck/aHash/issues/95

Proposed Solution

This: https://github.com/tkaitchuck/aHash/issues/95#issuecomment-881242012

js-sys would then only depend on wasm-bindgen-core, the full wasm-bindgen would just reexport wasm-bindgen-core, and getrandom would just depend on wasm-bindgen-core and js-sys.

Alternatives

Break the ahash dependency cycle somewhere else.

But creating a minimal wasm crate that can be used by low-level dependencies like getrandom without the risk of pulling in too many dependencies due to feature unification seems to be the most elegant solution.

Additional Context

Crates like wasm-bindgen, getrandom are seemingly cornerstones to this ecosystem, it would be great if fundamental crates would compile.

05storm26 avatar Jan 16 '22 00:01 05storm26

Are there any workarounds in the meantime?

maxcountryman avatar Feb 13 '22 15:02 maxcountryman

@maxcountryman lock indexmap like this: indexmap = "~1.6.2"

andoriyu avatar Feb 16 '22 04:02 andoriyu

So I ended up here too. It looks like the only working solution right now is to pin inedxmap to 1.6.2 as described. Which is pretty annoying. I also found no way to prevent wasm-bindgen from importing serde_json. But I guess there are several ways this could triggered.

ctron avatar Aug 09 '22 14:08 ctron

js-sys would then only depend on wasm-bindgen-core, the full wasm-bindgen would just reexport wasm-bindgen-core, and getrandom would just depend on wasm-bindgen-core and js-sys.

Unfortunately, that won't work - from_serde/into_serde are inherent methods of JsValue, which can't be added on by wasm-bindgen because of the orphan rule. Also, wasm-bindgen is pretty much already a 'core' crate; there wouldn't be much to remove in wasm-bindgen-core, so having the distinction would be weird even if it did work.

I was hoping to try and replace serde_json with something else like serde-wasm-bindgen, but from_serde/into_serde both return serde_json::Result, so it's impossible to drop the serde_json dependency without it being a breaking change.

So, I think the best thing we can do is deprecate the serde-serialize feature. It's pretty easily replaced by serde-wasm-bindgen or using serde_json manually, so it should be easy for users of it to switch away, and then there hopefully won't be any crates that enable the feature and trigger this dependency cycle.

@alexcrichton What do you think?

Liamolucko avatar Aug 13 '22 02:08 Liamolucko

Yes I think deprecation is fine myself.

alexcrichton avatar Aug 15 '22 14:08 alexcrichton

It might make sense to create a test for this, just to ensure that this indeed works. Now and in the future.

ctron avatar Aug 16 '22 07:08 ctron

gloo-utils v0.1.5 has been released that provides an alternate (with identical behavior) to JsValue::{from_serde,into_serde}

Thanks to @andoriyu for their contributions to make this happen.

ranile avatar Aug 21 '22 14:08 ranile