Web support

[Desdaemon]

Closes #315. Continues from #386.

Additions:

• --wasm flag to emit WASM-specific files
  • Requires --dart-decl-output
• --inline-rust to inline platform modules into a single file
• WASM shims for allo_isolate (merge upstream?)

Changes:

• Migrate to clap v3

Internal changes:

• Functions that used to pluck values from Opts now receive a reference to Opts

Checklist

• [ ] New tests are added to ensure new features are working (probably by modifying the frb_example/pure_dart example).
• [ ] All existing and new tests are passing.
• [ ] If this PR adds/changes features, documentations (in book folder) are modified as well.

[fzyzcjy]

Awesome job! Looking forward to merging it! - Feel free to ping me (or click "request review" button) when it is ready, cannot wait for that :)

[fzyzcjy]

Btw I see some refactors in the PR, e.g. migrate to clap, use bail instead of assert, etc. What about making it a separate PR such that we can merge it firstly? That will make this main PR smaller and more focused on one problem (the web support problem).

[fzyzcjy]

WASM shims for allo_isolate (merge upstream?)

May I know what to merge? Not see PR in allo_isolate yet. Anyway just bump cargo.toml in this pr is sufficient

[Desdaemon]

WASM shims for allo_isolate (merge upstream?)

May I know what to merge? Not see PR in allo_isolate yet. Anyway just bump cargo.toml in this pr is sufficient

No, I'm still unsure if I should merge this code at all, it introduces a lot of dependencies to support just a single platform. Maybe if I could get some feedback later on the shims then I can consider merging the code.

[fzyzcjy]

it introduces a lot of dependencies to support just a single platform

Which dependencies? Maybe we can use cargo's conditional dependencies to make it optional

[Desdaemon]

it introduces a lot of dependencies to support just a single platform

Which dependencies? Maybe we can use cargo's conditional dependencies to make it optional

Definitely, they are already target-gated to WASM. They are in frb_rust/Cargo.toml if you want to double check.

[fzyzcjy]

So, maybe I do a code review on this draft even before it is mark as done? Because this is a very nontrivial PR, and early feedback instead of feedbacks after everything is done can save time and work needed.

[fzyzcjy]

Remark: No worries about the code review comments - I am just leaving them here to provide early feedback :)

[Desdaemon]

This PR is big work! I only reviewed part of it today and will look at it again later. As we all know code-review is critical to ensure code quality, so I will definitely review all, no worries ;) (And to avoid providing a ton of review to stress you...)

No I agree, it's a big one so I will also need to be more thorough than usual in my tests.

[fzyzcjy]

more thorough than usual in my tests.

Btw, we already have a ton of e2e tests (in the frb_dart .dart), so as long as we can run them in Web, we automatically get a lot of confidence :)

[fzyzcjy]

Btw I am thinking about posting the news (frb supports web) after this PR is merged in Rust & Dart subreddit, since this new feature strengthens the lib a lot :) What do you think？

[mz2]

Hopefully my comment is more helpful than unhelpful given the PR is still in the works and I'm aware that the documentation is not yet updated: I got curious and tried installing and running flutter_rust_bridge_codegen off the source branch of this PR (flutter_rust_bridge_codegen --wasm --rust-input frb_example/with_flutter/rust/src/api.rs --dart-output frb_example/with_flutter/lib/bridge_generated.dart --dart-decl-output frb_example/with_flutter/lib/bridge_definitions.dart), resulting in lib/bridge_definitions.dart, lib/bridge_generated.dart, lib/bridge_generated.web.dart that looked similar to what's on the branch already, with different formatting applied (presuming a Dart SDK version difference).

After this I successfully ran frb_example/with_flutter, on macOS and Linux (repeated the same steps on both)... but on both platforms, attempting cargo run -d chrome results in a bunch of errors:

lib/main.dart:2:8: Error: Not found: 'dart:ffi'
import 'dart:ffi';

lib/bridge_generated.dart:11:8: Error: Not found: 'dart:ffi'
import 'dart:ffi' as ffi;
       ^
lib/bridge_generated.dart:16:40: Error: Type 'ffi.DynamicLibrary' not found.
  factory FlutterRustBridgeExampleImpl(ffi.DynamicLibrary dylib) =>

... (goes on quite a while)...

lib/bridge_generated.dart:694:16: Error: Only JS interop members may be 'external'.
Try removing the 'external' keyword or adding a JS interop annotation.
  external int len;

Again, hopefully this is of use 😄

[fzyzcjy]

@mz2 Judging from the errors, seems that, when running the web platform, the example fail to use conditional import to completely avoid importing any files that uses native feature (i.e. dart:ffi).

[mz2]

(Continuing noting my observations, let me know if this is just too early / not even supposed to work and I should not bother yet!)

Judging from the errors, seems that, when running the web platform, the example fail to use conditional import to completely avoid importing any files that uses native feature (i.e. dart:ffi).

Yep, that was it, thanks. The example doesn't take care of conditional imports yet. I was able to get the example application to run with the following (not sure if there is a more idiomatic solution, or some import that would itself take care of this):

import 'package:flutter_rust_bridge_example/bridge_generated.dart'
    if (dart.library.html) 'package:flutter_rust_bridge_example/bridge_generated.web.dart';
import 'dart:ffi' if (dart.library.html) './dylib_stub.dart';

...where dylib_stub.dart is something I made just as a test just to see it build, with the stub fullfilling the relevant API of DynamicLibrary that is referenced in main.dart (the process, executable, open factory constructors -- stub just throws errors from each of these since it's just needed to make the compiler happy):

late final dylib = kIsWeb
    ? {} as WasmModule // just put this here to get it to build as a test, since I have not worked out how to generate the wasm module successfully for the example...
    : Platform.isIOS
        ? DynamicLibrary.process()
        : Platform.isMacOS
            ? DynamicLibrary.executable()
            : DynamicLibrary.open(path);
late final api = FlutterRustBridgeExampleImpl(dylib as dynamic);

Again, I am guessing there's an abstraction intended to solve this either already somewhere or coming up. Like this the code runs but of course doesn't work right yet, since I don't know how to get the wasm for the example built successfully. Is that meant to work? I tried the following, issued in frb_example/with_flutter/rust:

> wasm-pack build --target web
[INFO]: 🎯  Checking for the Wasm target...
[INFO]: 🌀  Compiling to Wasm...
   Compiling flutter_rust_bridge_example v0.1.0 (/Users/mz2/Developer/flutter_rust_bridge/frb_example/with_flutter/rust)
error[E0308]: mismatched types
   --> src/bridge_generated.web.rs:215:28
    |
215 |         let vec: Vec<u8> = self.wire2api();
    |                  -------   ^^^^^^^^^^^^^^^ expected struct `Vec`, found struct `String`
    |                  |
    |                  expected due to this
    |
    = note: expected struct `Vec<u8>`
               found struct `String`

error[E0308]: mismatched types
   --> src/bridge_generated.web.rs:233:51
    |
233 |             let wrap = support::box_from_leak_ptr(self);
    |                                                   ^^^^ expected *-ptr, found struct `Box`
    |
    = note: expected raw pointer `*mut _`
                    found struct `Box<[flutter_rust_bridge::JsValue]>`

error[E0609]: no field `ptr` on type `Box<_>`
   --> src/bridge_generated.web.rs:234:45
    |
234 |             support::vec_from_leak_ptr(wrap.ptr, wrap.len)
    |                                             ^^^ unknown field

...

error[E0609]: no field `ptr` on type `Box<_>`
   --> src/bridge_generated.web.rs:281:45
    |
281 |             support::vec_from_leak_ptr(wrap.ptr, wrap.len)
    |                                             ^^^ unknown field

[fzyzcjy]

if (dart.library.html)

That is what I expected

For other errors, I guess it is because the work is still WIP - the wire types have not been constructed fully yet. I am just guessing and @Desdaemon is the expert on this question, surely :)

[mz2]

Awesome, well thanks a lot for all your effort on this, I can't wait to give this a spin and I bet I'm not the only one :-)

[fzyzcjy]

I can't wait to give this a spin and I bet I'm not the only one :-)

You are right - I also can't wait to try it :)

[Desdaemon]

As of the last commit the with_flutter example works on web, but you will either need to wait for rust-lang/rust#99800 to land, or apply the changes to the stdlib locally first. You can use just serve -c rust when in the example directory. As everything in WASM-land is concerned, nightly is a must.

[fzyzcjy]

As of the last commit the with_flutter example works on web, but you will either need to wait for https://github.com/rust-lang/rust/pull/99800 to land, or apply the changes to the stdlib locally first. You can use just serve -c rust when in the example directory. As everything in WASM-land is concerned, nightly is a must.

That PR seems to be merged :)

[Desdaemon]

Some todos remain, but I might be able to wrap up this PR ~~this weekend~~ soon.

[fzyzcjy]

Great job! I will find time start reviewing this huge PR today

[Desdaemon]

Potential blocker: JS values are stored in a JS scope-local heap right now, making StreamSink not Send. This disallows the pattern of saving the sink to a static variable and later accessing it from a different thread, i.e. the sink cannot leave its declared function scope.

[fzyzcjy]

JS values are stored in a JS scope-local heap right now

Is it possible to make a global heap and allocate everything there, just like the "memory pool" design pattern?

This disallows the pattern of saving the sink to a static variable and later accessing it from a different thread, i.e. the sink cannot leave its declared function scope.

That does not sound like a very huge problem: Maybe we can let the scope be long-running, while having another queue sending data to it?

static ref GLOBAL_QUEUE = SomeQueueTypeThatIsTheadSafe(); // tons of lib for this

fn the_function_that_sink_cannot_leave(sink) {
  loop {
    let data = GLOBAL_QUEUE.pop();
    sink.add(data);
  }
}

fn another_function_that_wants_to_send_data_to_the_sink() {
  GLOBAL_QUEUE.push(data);
}

Just one little level of abstraction over the original case.

[Desdaemon]

That does not sound like a very huge problem: Maybe we can let the scope be long-running, while having another queue sending data to it?

I really like this solution, even though it looks a bit cumbersome at first. My only worry is that WASM code will not work out of the box with existing code.

[fzyzcjy]

My only worry is that WASM code will not work out of the box with existing code.

Well that is a problem. But maybe we can release this PR first, and then improve it later as a separate PR - smaller PRs are easier to review and re-read later by other people :)

[Desdaemon]

Web tests can now be run with just test-pure-web, though there isn't much to see but errors for now.

[fzyzcjy]

Good job! Btw seems that GitHub refuses to run CI b/c "This branch has conflicts that must be resolved"

[fzyzcjy]

Btw the design doc looks great :)

https://github.com/fzyzcjy/flutter_rust_bridge/blob/6b18afaa5f2a7303544b3e51ff738f1c6f0d2655/book/src/contributing/architecture.md#user-content-fnref-1-2-e19c48f74dd148f1e563271f80801252

[Desdaemon]

The last blocker is surprisingly the three ConcatenateWith tests which have never succeeded in CI and very flaky on local. It might have to do with the interactions between wasm_thread and our own worker pool.

[fzyzcjy]

The last blocker is surprisingly the three ConcatenateWith tests which have never succeeded in CI and very flaky on local. It might have to do with the interactions between wasm_thread and our own worker pool.

Weird... Anyway this is great b/c it helps us to find bugs of our implementations