implement hash concat for wasm pkg #2387
Conversation
|
Hi! Thanks for opening this pull request! 😄 |
| @@ -149,8 +153,12 @@ Future<String> _getRustCreateName({required String rustCrateDir}) async { | |||
| return rustCrateName; | |||
| } | |||
|
|
|||
| Future<int> _getRustContentHash({required String rustCrateDir}) async { | |||
| throw UnimplementedError(); | |||
There was a problem hiding this comment.
@fzyzcjy how would you fetch the rust content hash from the build-web command?
There was a problem hiding this comment.
One way may be parsing the generated code and find it.
But I am thinking about something more general: How does normal wasm-pack + js users (i.e. not wasm + dart users) handle this? I guess there may be some ways to add content hashing within wasm-pack ecosystem, and maybe we can utilize it.
EDIT: https://rustwasm.github.io/docs/wasm-pack/tutorials/hybrid-applications-with-webpack/using-your-library.html Hmm I guess maybe js users will just import it as a npm package or something like that, and rely on webpack to add hashing. But we do surely do not want to introduce webpack. Thus our current way seems reasonable.
Another question: Rust content hash only hashes about the bindings. But if the user changes other codes (e.g. some rust code), the hash will not change. Maybe we can find a better hash. Sorry for pointing for this hash yesterday - too tired then :(
Maybe we can create an issue at wasm-pack to discuss this. They officially support the no-module deployment, i.e. deploy without the need of webpack. So maybe we can ask what is the suggested way to handle such cache-invalidtion issues in this case.
There was a problem hiding this comment.
Maybe we can just build it like it was, and then at the final packing we hash the wasm file, append it to the filename and put it in the loader config too?
There was a problem hiding this comment.
Looks reasonable! More brainstorms based on that:
- Instead of
rust_lib-abcdef.js, we dorust_lib.js?dummy_query=abcdef(whereabcdefis the content hash). Then we do not need to rename anything to keep it simple. - Then, after
build-web, if this feature is enabled, we just computehash(read_file(rust_lib.js) + read_file(rust_lib.wasm)), and write it to a field infrb_generated.dart.
I am still wondering whether there are more "native" solutions instead of hand crafting it...
There was a problem hiding this comment.
You’re right, we could do that. Unfortunately the generated js file split the filename.js and add _bg.wasm as suffix. We would need to do a find and replace to add _bg.wasm?dummy=abcdef
so maybe having it in the directly would be better / easier.
There was a problem hiding this comment.
So, show we ask wasm-pack if they want to handle the hash on their side, or should I continue to work on a implementation here?
There was a problem hiding this comment.
I guess we can create an issue there to have a discussion with them (feel free to cc me if you like). Because if this feature is done there, then not only frb but also every other package that depend on wasm-pack will benefit from this!
Changes
Closes #2384
Checklist
./frb_internal precommit --mode slow(orfast) is run (it internal runs code generator, does auto formatting, etc)../websitefolder) are updated.Remark for PR creator
./frb_internal --helpshows utilities for development.