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.

Sign up for GitHub

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

implement hash concat for wasm pkg #2387

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

implement hash concat for wasm pkg #2387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions frb_dart/lib/src/cli/build_web/executor.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,11 @@ Future<void> executeBuildWeb(BuildWebArgs args) async {
final rustCrateName =
await _getRustCreateName(rustCrateDir: args.rustCrateDir);

await _executeWasmPack(args, rustCrateName: rustCrateName);
final contentHash =
await _getRustContentHash(rustCrateDir: args.rustCrateDir);

await _executeWasmPack(args,
rustCrateName: rustCrateName, contentHash: contentHash);

if (args.enableWasmBindgen) {
await _executeWasmBindgen(args, rustCrateName: rustCrateName);
Expand Down Expand Up @@ -149,8 +153,12 @@ Future<String> _getRustCreateName({required String rustCrateDir}) async {
return rustCrateName;
}

Future<int> _getRustContentHash({required String rustCrateDir}) async {
throw UnimplementedError();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fzyzcjy how would you fetch the rust content hash from the build-web command?

Copy link
Owner

@fzyzcjy fzyzcjy Nov 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable! More brainstorms based on that:

  • Instead of rust_lib-abcdef.js, we do rust_lib.js?dummy_query=abcdef (where abcdef is 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 compute hash(read_file(rust_lib.js) + read_file(rust_lib.wasm)), and write it to a field in frb_generated.dart.

I am still wondering whether there are more "native" solutions instead of hand crafting it...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

}

Future<void> _executeWasmPack(BuildWebArgs args,
{required String rustCrateName}) async {
{required String rustCrateName, required int contentHash}) async {
await runCommand('wasm-pack', [
'build',
'-t',
Expand All @@ -159,7 +167,7 @@ Future<void> _executeWasmPack(BuildWebArgs args,
args.outputWasm,
'--no-typescript',
'--out-name',
rustCrateName,
'${rustCrateName}${contentHash.toRadixString(16)}',
if (!args.release) '--dev',
args.rustCrateDir,
'--',
Expand Down
3 changes: 2 additions & 1 deletion frb_dart/lib/src/loader/_io.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@ import 'dart:async';
import 'dart:io';

import 'package:flutter_rust_bridge/src/loader/_common.dart';
import 'package:flutter_rust_bridge/src/main_components/entrypoint.dart';
import 'package:flutter_rust_bridge/src/platform_types/_io.dart';

/// Load the [ExternalLibrary], with the following cases in mind:
/// 1. When `flutter run`, or when a real app is bundled.
/// 2. When running Flutter widget tests.
/// 3. When `dart test`, `dart run`, `dart compile exe`, etc.
FutureOr<ExternalLibrary> loadExternalLibrary(
ExternalLibraryLoaderConfig config) async {
BaseEntrypoint _, ExternalLibraryLoaderConfig config) async {
final ioDirectory = config.ioDirectory;
return loadExternalLibraryRaw(
nativeLibDirWhenNonPackaged:
Expand Down
6 changes: 4 additions & 2 deletions frb_dart/lib/src/loader/_web.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import 'dart:async';

import 'package:flutter_rust_bridge/src/loader/_common.dart';
import 'package:flutter_rust_bridge/src/main_components/entrypoint.dart';
import 'package:flutter_rust_bridge/src/platform_types/_web.dart';
import 'package:flutter_rust_bridge/src/wasm_module/_web.dart';

/// See `loadExternalLibrary` in the counterpart `_io.dart` for doc
FutureOr<ExternalLibrary> loadExternalLibrary(
ExternalLibraryLoaderConfig config) async {
BaseEntrypoint entrypoint, ExternalLibraryLoaderConfig config) async {
var hex = entrypoint.rustContentHash.toRadixString(16);
return loadExternalLibraryRaw(
moduleRoot: '${config.webPrefix}${config.stem}');
moduleRoot: '${config.webPrefix}${config.stem}${hex}');
}

/// Please see `loadExternalLibrary` for details
Expand Down
3 changes: 1 addition & 2 deletions frb_dart/lib/src/main_components/entrypoint.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,15 +130,14 @@ abstract class BaseEntrypoint<A extends BaseApi, AI extends BaseApiImpl,
String get codegenVersion;

/// {@macro flutter_rust_bridge.only_for_generated_code}
@protected
int get rustContentHash;

/// {@macro flutter_rust_bridge.only_for_generated_code}
@protected
Future<void> executeRustInitializers();

Future<ExternalLibrary> _loadDefaultExternalLibrary() async =>
await loadExternalLibrary(defaultExternalLibraryLoaderConfig);
await loadExternalLibrary(this, defaultExternalLibraryLoaderConfig);

A _createDefaultApi(
BaseHandler handler,
Expand Down
Loading