Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unpin wasm-bindgen to improve compatibility with other crates #3186

Merged
merged 2 commits into from
Nov 4, 2024

Conversation

DanielleHuisman
Copy link
Contributor

Leptos can't be installed alongside another crate that uses a newer wasm-bindgen version. For example:

error: failed to select a version for `wasm-bindgen`.
    ... required by package `web-sys v0.3.72`
    ... which satisfies dependency `web-sys = "^0.3.72"` of package `crate-c v0.0.1`
    ... which satisfies dependency `crate-c` of package `crate-b v0.0.1`
versions that meet the requirements `^0.2.95` are: 0.2.95

all possible versions conflict with previously selected packages.

  previously selected package `wasm-bindgen v0.2.93`
    ... which satisfies dependency `wasm-bindgen = "=0.2.93"` of package `leptos v0.7.0-rc0`
    ... which satisfies dependency `leptos = "^0.7.0-rc0"` (locked to 0.7.0-rc0) of package `crate-a v0.0.1`

failed to select a version for `wasm-bindgen` which could resolve this conflict

Unpinning wasm-bindgen in Leptos should improve compatibility with other crates.

@DanielleHuisman
Copy link
Contributor Author

I'm aware wasm-bindgen doesn't adhere to semver, but pinning the version prevents users from even trying a newer version. There is a good chance newer versions of wasm-bindgen will work without modification. I tried my fix in a project and it works perfectly fine. In my opinion, the chance of it working with a newer version is preferable over refusing to even install. Additionally, it would allow early adopters to report bugs/incompatibilities with the newer version.

Pinning is required due to compatibility with cargo-leptos

Could you explain why this is required?

@sabify
Copy link
Contributor

sabify commented Nov 3, 2024

Pinning is required due to compatibility with cargo-leptos and also because breaking changes happen in wasm-bindgen in patch releases which do not follow semver semantics. The whole ecosystem should adapt to the new version at once, but pinning may be required here.

@sabify
Copy link
Contributor

sabify commented Nov 3, 2024

Due to wasm-bindgen restrictions, the versions used for generating the binary and the project dependency should be the same to prevent API usage violations.

You get the following error on version mismatch:

       it looks like the Rust project used to create this wasm file was linked against
       version of wasm-bindgen that uses a different bindgen format than this binary:

         rust wasm file schema version: 0.2.95 (94b266337)
            this binary schema version: 0.2.93 (94b266337)

       Currently the bindgen format is unstable enough that these two schema versions
       must exactly match. You can accomplish this by either updating this binary or
       the wasm-bindgen dependency in the Rust project.

@gbj
Copy link
Collaborator

gbj commented Nov 3, 2024

I'm happy to let CI run on this PR. Looking at the cargo-leptos repo it looks like it has been updated to 0.2.95 but that a release has not been made. So I assume this will fail on the examples, but can make a new cargo-leptos release later and that should work.

@gbj
Copy link
Collaborator

gbj commented Nov 4, 2024

Okay, so after publishing an updated cargo-leptos that uses 0.2.95 and letting CI run again, these (new! fun! exciting!) CI errors are unrelated. I'll merge this and rerelease as rc1.

For context and just FYI for the future, the reason I had pinned it was that, as described above, the wasm-bindgen-cli-support crate used by cargo-leptos won't allow us to compile anything with even a patch mismatch. There was a breaking change that removed needed APIs in wasm-bindgen-cli-support 0.2.94, making it impossible for us to update cargo-leptos and immediately release it, which meant every Leptos build using cargo-leptos was immediately broken unless the user manually pinned wasm-bindgen to =0.2.93. I took the expedient of pinning at the crate level.

This was a helpful nudge since all this needed was just a new c-l release from its current state.

@gbj gbj merged commit 865c6df into leptos-rs:main Nov 4, 2024
68 of 73 checks passed
@sabify
Copy link
Contributor

sabify commented Nov 4, 2024

@gbj assume that with the current state, wasm-bindgen releases a new version v0.2.96 and we suddenly try to run cargo update. Now we have mismatch versions, cargo-leptos uses v0.9.95 and the wasm-bindgen will not generate a binary. You get an error I mentioned in my previous comment. Due to restrictions of wasm-bindgen, we will get off the track without pinning this specific dependency easily and we should try to do the same in our codebase to pin wasm-bindgen until we have a new release from cargo-leptos.

IMHO, pinning is required here and we need to synchronize releases for this dependency on both leptos and cargo-leptos. I know that coupling may not be a good idea, but it is required here until wasm-bindgen works on mismatch versions.

The solution to outdated dependencies is to get notified by Dependabot by introducing Cargo.lock to act accordingly.

@DanielleHuisman
Copy link
Contributor Author

Thanks for merging this!

I looked into the cargo-leptos pinning issue a bit more. The only reason the version mismatch exists is because cargo-leptos is compiled with wasm-bindgen-cli-support.

I was using Trunk and didn't experience any issues. Trunk is not compiled with wasm-bindgen-cli, but instead finds the appropriate version to use from Trunk.toml/Cargo.lock/Cargo.toml. It then downloads the right binary to a cache and calls the binary to build the bindings.

Using a similar approach in cargo-leptos would solve the pinning issue. If this sounds like an interesting solution, I'd be happy to make a PR for cargo-leptos.

@DanielleHuisman DanielleHuisman deleted the fix/unpin-wasm-bindgen branch November 4, 2024 08:18
@gbj
Copy link
Collaborator

gbj commented Nov 4, 2024

I am aware of both the issues and the trade-offs you describe in your comments, above. Over the course of the last couple years, the least painful way for us to manage wasm-bindgen's policy of breaking semver has been to simply update cargo-leptos and re-release as needed.

@sabify You have proposed adding Cargo.lock in #2881 and I replied, saying it was probably a good idea. Feel free to make a PR to add it.

@DanielleHuisman If you want to make a PR to change this behavior in cargo-leptos, feel free to open an issue there to discuss, or to make a PR. I know Ben is in the process of rewriting in any case, so some discussion with him before doing the work of a PR probably makes sense.

In any case I don't think more of either discussion should happen here, as conversations in the comments of closed PRs are hard to keep track of.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants