anp / moxie

lightweight platform-agnostic tools for declarative UI
https://moxie.rs
Apache License 2.0
828 stars 27 forks source link

Error resolving module specifier “env” #218

Closed MartinKavik closed 3 years ago

MartinKavik commented 3 years ago

Hi, a Seed app that indirectly depends on topo started to fail in runtime after some dependency updates.

The error in the console log: Uncaught TypeError: Error resolving module specifier “env”. Relative module specifiers must start with “./”, “../” or “/”.

After some hours of debugging I was able to track it to the parking_lot. This crate doesn't support Wasm on stable Rust and I wasn't able to compile it on nightly to prove that it works - see my effort in the parking_lot issue.

It looks like it works with parking_lot <= 0.10, but the latest 0.11.x fails. Some cfg_ifs for wasm32 have been introduced in the latest parking_lot versions.

I was also looking at wasm-bindgen and something like more explicit JS module handling has been introduced in recent versions according its changelog.

So I guess that wasm-bindgen tries to link a required wasm module from parking_lot, but the module is "hidden" under some compilation flags in parking_lot and linking silently fails with some mess in the initialization JS script. Then the script is invoked on the app start and immediately outputs that cryptic error to the console log and dies. The error is basically a show-stopper for most people - see the issue in wasm-bindgen.

Possible next steps:

Thank you very much! Martin

zetanumbers commented 3 years ago

The parking_lot does actually support Wasm on stable Rust just like std::sync (by panicking on block). This is a moxie bug actually, because parking_lot dependency didn't specify any web required features like stdweb or wasm_bindgen. However right now code does panicked at 'already borrowed: BorrowMutError', D:\projects\moxie\dom\raf\src\lib.rs:62:32, while simple_mutex solution worked perfectly (which is based on std::sync::Mutex). Right now I think this happens bc of async schedule or smth. Oh my mistake, that's it!

anp commented 3 years ago

Thanks @ZetaNumbers for investigating and finding a fix! I've merged https://github.com/anp/moxie/pull/226/commits/24ab304302eace284b50e764d730b6b1ba0e5ab9 and am publishing patch releases of the affected crates.

@MartinKavik can you give that a try? I'll close this for now but if this issue still isn't resolved please reply back and/or reopen the issue.

MartinKavik commented 3 years ago

can you give that a try? I'll close this for now but if this issue still isn't resolved please reply back and/or reopen the issue.

I tried it with the new topo version (0.13.1) and it doesn't fix the issue. The flag wasm-bindgen only set the same feature flag in the instant crate - I can imagine it would resolve some other issues but not this one. The problem is that parking_lot uses experimental API. The API requires nightly Rust and a compiler flag, see more info in this comment.

zetanumbers commented 3 years ago

@MartinKavik no. The parking_lot uses experimental wait/notify API only if you specify atomics target feature for wasm32. Without it there is no other option of blocking API on wasm32-unknown-unknown currently, so implementations of std::sync and parking_lot just panics if would block. The actual issue is that the instant crate relies on extern functions if none of stdweb or wasm-bindgen features are present, meaning it is trying to pull and link to them from this "env" thing.

EDIT: my advice to you would be to not use atomics feature and try to replicate this issue on the current main branch.

MartinKavik commented 3 years ago

my advice to you would be to not use atomics feature and try to replicate this issue on the current main branch.

I've just tried it with the moxie/topo's latest main and with topo 0.13.1. No luck for both cases, the same problem. I don't use atomics feature

My idea was that parking_lot fallbacks to a non-complete WASM support on stable Rust or without atomics feature and wasm-bindgen isn't able to process it correctly and fails in runtime because of that env error. But yeah I might be wrong, I wasn't deep enough it this rabbit hole to find all necessary data to prove my assumption.

zetanumbers commented 3 years ago

Ah sorry I forgot. Use feature wasm-bindgen for topo. @MartinKavik