Web support #589

Desdaemon · 2022-07-24T21:28:27Z

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.

frb_rust/src/ffi.rs

fzyzcjy · 2022-07-24T22:17:27Z

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 · 2022-07-24T22:19:38Z

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 · 2022-07-24T22:30:59Z

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 · 2022-07-25T03:06:40Z

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 · 2022-07-25T03:14:50Z

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 · 2022-07-25T03:17:21Z

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 · 2022-07-25T08:44:31Z

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

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...)

frb_codegen/src/commands.rs

frb_codegen/src/config.rs

frb_codegen/src/lib.rs

frb_rust/src/ffi.rs

fzyzcjy · 2022-07-25T13:00:50Z

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

fzyzcjy

Great job! Looked at it a bit deeper, and only find some minor points that may need a bit of discussions

frb_codegen/src/generator/dart/mod.rs

frb_codegen/src/generator/dart/ty_optional.rs

frb_codegen/src/ir/ty_enum.rs

Desdaemon · 2022-07-25T20:00:46Z

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 · 2022-07-25T22:36:00Z

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

A little bit more review

frb_example/pure_dart/dart/lib/bridge_generated.web.dart

frb_example/pure_dart/dart/lib/main.dart

frb_example/with_flutter/rust/src/bridge_generated.web.rs

frb_rust/src/handler.rs

frb_codegen/src/ir/ty.rs

frb_codegen/src/ir/ty_boxed.rs

frb_codegen/src/ir/ty_delegate.rs

frb_rust/src/pool.rs

fzyzcjy · 2022-07-26T11:31:53Z

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 · 2022-07-27T22:21:11Z

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 · 2022-07-27T22:32:31Z

@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 · 2022-07-28T13:43:50Z

(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 · 2022-07-28T13:48:35Z

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 · 2022-07-28T14:05:20Z

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 · 2022-07-28T22:58:27Z

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 :)

tools/dartsdk/fetch.sh

frb_dart/lib/src/typed_data/io.dart

…ust_bridge into wasm_workers

Desdaemon · 2022-09-06T13:21:06Z

Should be the last batch of changes to be reviewed by @fzyzcjy before merging.

fzyzcjy · 2022-09-06T13:30:15Z

Excellent job! Now we are very close to the exciting moment of adding this big feature :) I will try to have as little issues as possible, and defer to future backlog as well.

Look at all (resolved) comments again since may miss some of them
Review the code briefly again

fzyzcjy

I believe in you, so only roughly look at the code and do not check the details :)

frb_dart/test/io_typed_data.dart

frb_rust/src/lib.rs

fzyzcjy

All done!

Only 4 conversations (2 new, 2 reopened), with no more than file or function renaming or removing a comment line etc.

I will sleep now and check inbox after ~8 hours :)

- Rename `wasm2api_body` to `wire2api_jsvalue` - Run library unit tests in CI - Update Scoop manifest

fzyzcjy · 2022-09-07T03:19:04Z

Ready to merge! (After #678 since otherwise cannot release a new version with CI passing)

fzyzcjy · 2022-09-07T03:32:28Z

Ah if you are resolving conflicts then I do not do it

Anyway the code is OK and will be merged once master turns green

fzyzcjy · 2022-09-07T06:40:05Z

🎉

Desdaemon added 4 commits July 23, 2022 12:31

Migrate to clap v3

b63857d

Web FFI runtime support

b0994ba

First draft of WASM codegen

00d4b1b

Generate Dart structs, enums, boxes

8b6f935

Desdaemon changed the title ~~Wasm workers~~ Web support Jul 24, 2022

Desdaemon commented Jul 24, 2022

View reviewed changes

frb_rust/src/ffi.rs Outdated Show resolved Hide resolved

fzyzcjy mentioned this pull request Jul 24, 2022

Code cleanup: dart/ty.rs is accidentically modified #590

Closed

fzyzcjy reviewed Jul 25, 2022

View reviewed changes

fzyzcjy reviewed Jul 26, 2022

View reviewed changes

frb_example/pure_dart/dart/lib/bridge_generated.web.dart Outdated Show resolved Hide resolved

frb_example/pure_dart/dart/lib/main.dart Outdated Show resolved Hide resolved

frb_example/with_flutter/rust/src/bridge_generated.web.rs Outdated Show resolved Hide resolved

Desdaemon commented Jul 26, 2022

View reviewed changes

frb_rust/src/handler.rs Outdated Show resolved Hide resolved

fzyzcjy reviewed Jul 26, 2022

View reviewed changes

frb_rust/src/handler.rs Outdated Show resolved Hide resolved

frb_rust/src/handler.rs Outdated Show resolved Hide resolved

fzyzcjy reviewed Jul 26, 2022

View reviewed changes

fzyzcjy reviewed Sep 5, 2022

View reviewed changes

tools/dartsdk/fetch.sh Show resolved Hide resolved

fzyzcjy reviewed Sep 5, 2022

View reviewed changes

frb_dart/lib/src/typed_data/io.dart Outdated Show resolved Hide resolved

Desdaemon added 2 commits September 4, 2022 22:50

Cleanup

c7cbc6a

Refactor

4644ef6

Desdaemon force-pushed the wasm_workers branch from e4e84c6 to 4644ef6 Compare September 5, 2022 03:58

Desdaemon added 5 commits September 5, 2022 00:18

Merge branch 'master' into wasm_workers

3a88ec3

Pass tests after merge

d0286c4

Downgrade to flutter 3.0.5

38a781f

Merge branch 'wasm_workers' of https://github.com/Desdaemon/flutter_r…

9cdc62e

…ust_bridge into wasm_workers

Merge remote-tracking branch 'origin/master' into wasm_workers

43c8b22

Regen files; forbid help files mismatch

f02f84a

Desdaemon force-pushed the wasm_workers branch from 4d05038 to f02f84a Compare September 6, 2022 14:27

fzyzcjy reviewed Sep 6, 2022

View reviewed changes

frb_dart/test/io_typed_data.dart Outdated Show resolved Hide resolved

frb_rust/src/lib.rs Outdated Show resolved Hide resolved

fzyzcjy approved these changes Sep 6, 2022

View reviewed changes

Document (U)Int64List platform differences

e6a07e5

Desdaemon mentioned this pull request Sep 6, 2022

Refactor Rust wire2api's return type #674

Closed

Desdaemon and others added 3 commits September 6, 2022 20:28

Minor adjustments

783b717

- Rename `wasm2api_body` to `wire2api_jsvalue` - Run library unit tests in CI - Update Scoop manifest

Simplify library unit tests

9f580bc

Update README.md

d5e4e45

Desdaemon added 2 commits September 6, 2022 23:26

Merge branch 'master' into wasm_workers

5b4d740

Fix dep conflicts

2bead28

Desdaemon added 2 commits September 6, 2022 23:38

Merge remote-tracking branch 'origin/master' into wasm_workers

03c7def

Regen files

ec785d7

fzyzcjy merged commit c687acb into fzyzcjy:master Sep 7, 2022

github-actions bot locked as resolved and limited conversation to collaborators Sep 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Web support #589

Web support #589

Desdaemon commented Jul 24, 2022 •

edited

Loading

fzyzcjy commented Jul 24, 2022 •

edited

Loading

fzyzcjy commented Jul 24, 2022

fzyzcjy commented Jul 24, 2022

Desdaemon commented Jul 25, 2022

fzyzcjy commented Jul 25, 2022 •

edited

Loading

Desdaemon commented Jul 25, 2022

fzyzcjy commented Jul 25, 2022

fzyzcjy left a comment •

edited

Loading

fzyzcjy commented Jul 25, 2022

fzyzcjy left a comment

Desdaemon commented Jul 25, 2022

fzyzcjy commented Jul 25, 2022

fzyzcjy left a comment

fzyzcjy commented Jul 26, 2022

mz2 commented Jul 27, 2022

fzyzcjy commented Jul 27, 2022

mz2 commented Jul 28, 2022 •

edited

Loading

fzyzcjy commented Jul 28, 2022 •

edited

Loading

mz2 commented Jul 28, 2022

fzyzcjy commented Jul 28, 2022

Desdaemon commented Sep 6, 2022

fzyzcjy commented Sep 6, 2022 •

edited

Loading

fzyzcjy left a comment

fzyzcjy left a comment •

edited

Loading

fzyzcjy commented Sep 7, 2022

fzyzcjy commented Sep 7, 2022 •

edited

Loading

fzyzcjy commented Sep 7, 2022

Web support #589

Web support #589

Conversation

Desdaemon commented Jul 24, 2022 • edited Loading

Checklist

fzyzcjy commented Jul 24, 2022 • edited Loading

fzyzcjy commented Jul 24, 2022

fzyzcjy commented Jul 24, 2022

Desdaemon commented Jul 25, 2022

fzyzcjy commented Jul 25, 2022 • edited Loading

Desdaemon commented Jul 25, 2022

fzyzcjy commented Jul 25, 2022

fzyzcjy left a comment • edited Loading

Choose a reason for hiding this comment

fzyzcjy commented Jul 25, 2022

fzyzcjy left a comment

Choose a reason for hiding this comment

Desdaemon commented Jul 25, 2022

fzyzcjy commented Jul 25, 2022

fzyzcjy left a comment

Choose a reason for hiding this comment

fzyzcjy commented Jul 26, 2022

mz2 commented Jul 27, 2022

fzyzcjy commented Jul 27, 2022

mz2 commented Jul 28, 2022 • edited Loading

fzyzcjy commented Jul 28, 2022 • edited Loading

mz2 commented Jul 28, 2022

fzyzcjy commented Jul 28, 2022

Desdaemon commented Sep 6, 2022

fzyzcjy commented Sep 6, 2022 • edited Loading

fzyzcjy left a comment

Choose a reason for hiding this comment

fzyzcjy left a comment • edited Loading

Choose a reason for hiding this comment

fzyzcjy commented Sep 7, 2022

fzyzcjy commented Sep 7, 2022 • edited Loading

fzyzcjy commented Sep 7, 2022

Desdaemon commented Jul 24, 2022 •

edited

Loading

fzyzcjy commented Jul 24, 2022 •

edited

Loading

fzyzcjy commented Jul 25, 2022 •

edited

Loading

fzyzcjy left a comment •

edited

Loading

mz2 commented Jul 28, 2022 •

edited

Loading

fzyzcjy commented Jul 28, 2022 •

edited

Loading

fzyzcjy commented Sep 6, 2022 •

edited

Loading

fzyzcjy left a comment •

edited

Loading

fzyzcjy commented Sep 7, 2022 •

edited

Loading