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

Dependency to "env" makes triangle example not run with wasm_bindgen in browser #5480

Open
vizigr0u opened this issue Apr 3, 2024 · 11 comments
Labels
help required We need community help to make this happen. type: bug Something isn't working

Comments

@vizigr0u
Copy link

vizigr0u commented Apr 3, 2024

edit: tl;dr: wgpu's dependency to parking_lot 0.11 adds a dependency to instant without its wasm-bindgen feature flag, which causes wasm to have an incorrect dependency to env

Description

The Triangle example compiles with target wasm32-unknown-unknown but doesn't run in the browser using wasm_bindgen.

When adding wasm_bindgen(start) (or just wasm_bindgen) macro to the main in the triangle example,
the generated javascript contains import * as __wbg_star0 from 'env';

When ran, this results in Failed to resolve import "env" from "backend/pkg/backend.js". Does the file exist?

This looks like an issue other crates also had when something broke compatibility with wasm32-unknown-unknown.

Repro steps

The following minimal repro contains code that can be commented out to get the code to run :

https://github.com/vizigr0u/rust-wasm-gl/tree/wgpu-wasm-env-bug/backend

Expected vs observed behavior

When surface.configure(&device, &config); is in the code, the generated javascript starts with import * as __wbg_star0 from 'env'; which gives an error when running.

When this line is removed, the generated javascript doesn't have the line and the program runs fine.

Expected: in either case there shouldn't be a dependency to env.

Extra materials

image

Platform

compile command (found in project root package.json) : wasm-pack build --dev ./backend --target web

target wasm32-unknown-unknown
wgpu 0.19.3
see dependencies used in repro's cargo.toml

❯ cargo --version
cargo 1.77.1 (e52e36006 2024-03-26)
❯ rustc --version
rustc 1.77.1 (7cf61ebde 2024-03-27)

Windows 10 + WSL2

@vizigr0u vizigr0u changed the title Dependency to "env" makes wasm not run with wasm_bindgen in browser Dependency to "env" makes triangle example not run with wasm_bindgen in browser Apr 3, 2024
@daxpedda
Copy link
Contributor

daxpedda commented Apr 3, 2024

You should try this tool to find the guilty function.

@vizigr0u
Copy link
Author

vizigr0u commented Apr 3, 2024

Thanks for the suggestion, I forgot to mention that I tried this tool and for some reason it didn't return anything.
But sleeping over this I now realize I blindly called it with the parameters env and function following the example, whereas I should have first used wasm2wat to find the line responsible for adding end.

Now I explored the wasm using wasm2wat and I found (import "env" "now" (func $now (type $t28))) which the tool successfully points to libinstant

Looking at my dependency tree, this is something that comes from wgpu's dependency to parking_lot 0.11.2

├── wgpu v0.19.3
│   ├── arrayvec v0.7.4
│   ├── cfg-if v1.0.0
│   ├── log v0.4.21
│   ├── parking_lot v0.11.2
│   │   ├── instant v0.1.12
│   │   │   └── cfg-if v1.0.0
│   │   ├── lock_api v0.4.11
│   │   │   └── scopeguard v1.2.0
│   │   │   [build-dependencies]
│   │   │   └── autocfg v1.2.0
│   │   └── parking_lot_core v0.8.6
│   │       ├── cfg-if v1.0.0
│   │       ├── instant v0.1.12 (*)
│   │       ├── libc v0.2.153
│   │       └── smallvec v1.13.2

Is there any way to not depend on this?

@vizigr0u
Copy link
Author

vizigr0u commented Apr 3, 2024

I fixed it by adding instant = { version = "0.1.12", features = ["wasm-bindgen"] as an explicit dependency of my project.

Maybe wgpu could add a wasm-bindgen feature as well to match and use this feature? Or some documentation to suggest to explicity depend on this feature. It made it particularily hard to get started.

@Wumpf
Copy link
Member

Wumpf commented Apr 3, 2024

better still, wgpu should enable the wasm-bindgen feature on parking lot whenever targeting wasm32. After all wasm bindgen is a dependency under the same condition

@Wumpf
Copy link
Member

Wumpf commented Apr 3, 2024

turns out this was the case at some point. Probably got lost when moving to workspace dependencies?

would be good to do the detective work and check if there was a reason this got removed

@Wumpf Wumpf added type: bug Something isn't working area: infrastructure Testing, building, coordinating issues help required We need community help to make this happen. and removed area: infrastructure Testing, building, coordinating issues labels Apr 3, 2024
@vizigr0u
Copy link
Author

vizigr0u commented Apr 4, 2024

I don't think recent versions of parking_lot have this feature flag, maybe that's why it got removed

@grovesNL
Copy link
Collaborator

grovesNL commented Apr 4, 2024

Looks like it was removed in #2639 when moving from parking_lot 0.11 to 0.12, which was the right decision for based on this issue that mentions it shouldn't be required for wasm32-unknown-unknown in 0.12.

The problem is that we then started allowing parking_lot 0.11 again in #2660 but didn't re-enable the feature for 0.11. Is it possible to enable the feature for 0.11 only, or is this a cargo limitation?

@grovesNL
Copy link
Collaborator

grovesNL commented Apr 4, 2024

I think we can also remove parking_lot 0.11 after switching to windows-rs fully (#3207)

@cwfitzgerald
Copy link
Member

@grovesNL we can remove it now - the problem was that firefox couldn't accept windows-rs; but they now can. We should double check that parking_lot 0.12 is okay, then just fully upgrade

@cwfitzgerald
Copy link
Member

Yes, it's fine https://searchfox.org/mozilla-central/source/Cargo.lock#4334

@grovesNL
Copy link
Collaborator

grovesNL commented Apr 4, 2024

Oh ok awesome 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help required We need community help to make this happen. type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants