feat: (continued) allow omission of dart-opaque feature #2681

aran · 2025-04-07T16:38:48Z

Changes

In this implementation flutter_rust_bridge::frb_generated_boilerplate_io and web are still present in the generated code, and now they have a reference to a dart-opaque feature that isn't present by default in the client's Cargo.toml. This can be worked around by adding the feature declaration. Fixing this so the code isn't generated at all would be a much bigger change.

…--no-default-features

aran · 2025-04-07T16:45:21Z

This addresses https://github.com/fzyzcjy/flutter_rust_bridge/actions/runs/14270290721/job/40001795109?pr=2662 by adding a dart-opaque feature across all the Cargo.toml.

The "pseudo-manual" tests using dart-opaque aren't fixed yet, I'm not quite sure what's wrong there or what would be a good way to approach it.

fzyzcjy · 2025-04-07T23:53:46Z

Hmm, firstly do we have this symbol or is it accidentially removed?

aran · 2025-04-08T03:43:55Z

Hmm, firstly do we have this symbol or is it accidentially removed?

That symbol is supposed to be gated on dart-opaque, so need to find all the places where dart-opaque is needed but not enabled. Found one more in 3682d0c.

codecov · 2025-04-08T05:12:06Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.84%. Comparing base (3aeceba) to head (3682d0c).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2681   +/-   ##
=======================================
  Coverage   98.84%   98.84%           
=======================================
  Files         469      469           
  Lines       20498    20510   +12     
=======================================
+ Hits        20262    20274   +12     
  Misses        236      236

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

fzyzcjy · 2025-04-08T08:29:35Z

Looks good! Now we only have 2 failing checks and seems it is still related

aran · 2025-04-09T04:11:05Z

I had to dig into a couple screens to figure out why the ubuntu CI jobs were "canceled". They failed with:

This is a scheduled Ubuntu 20.04 brownout. Ubuntu 20.04 LTS runner will be removed on 2025-04-15. For more details, see https://github.com/actions/runner-images/issues/11101

fzyzcjy · 2025-04-09T13:09:03Z

Ah, looks like we need to upgrade the ubuntu version in the CI. Do you have interest in making a PR for this?

aran · 2025-04-09T14:57:12Z

Ah, looks like we need to upgrade the ubuntu version in the CI. Do you have interest in making a PR for this?

I put up a quick find/replace PR but if there are issues due to the change I won't have time this week to look into them

fzyzcjy · 2025-04-17T15:14:05Z

Wow I did not realize CI became green, good job! I will have a review on it hopefully within a day.

fzyzcjy · 2025-04-18T14:17:23Z

frb_codegen/assets/integration_template/shared/REPLACE_ME_RUST_CRATE_DIR/Cargo.toml.template

@@ -11,3 +11,6 @@ flutter_rust_bridge = REPLACE_ME_RUST_FRB_DEPENDENCY

 [lints.rust]
 unexpected_cfgs = { level = "warn", check-cfg = ['cfg(frb_expand)'] }
+
+[features]
+dart-opaque = ["flutter_rust_bridge/dart-opaque"]


nit: this file is for all new projects' Cargo.toml, so it would be great to keep it unchanged

EDIT: Oh I see the reason

This can be worked around by adding the feature declaration.

Hmm, however, it would be great if we do not require all user code to have such a new feature in Cargo.toml :(

EDIT: This seems interesting https://stackoverflow.com/questions/64546459/how-can-my-crate-check-the-selected-features-of-a-dependency - is it possible we somehow utilize this approach

That's an interesting hack. Never would have thought of it. Not sure if it's a good idea but I think it might work as long as the number of actually-needed feature combinations was small.

I would generalize that suggestion and say that it comes down to modifying flutter_rust_bridge::frb_generated_boilerplate_io and flutter_rust_bridge::frb_generated_boilerplate_web not to generate the code requiring dart-opaque. Perhaps that StackOverflow macro approach is the best way to do that, or there might be another way.

fzyzcjy · 2025-04-18T14:18:17Z

frb_example/dart_minimal/rust/Cargo.toml

+
+[features]
+default = ["dart-opaque"]
+dart-opaque = ["flutter_rust_bridge/dart-opaque"]


nit: what about naming it flutter-rust-bridge--dart-opaque or something like that, so that it is more clear we are forwarding to there when we read our test infra code.

Not sure of the order of your comments, but just in case: No, this can't be renamed because the generated code from flutter_rust_bridge::frb_generated_boilerplate_* refers to "dart-opaque".

Oh yes, the "EDIT" above is the last one, so we may use that approach etc.

fzyzcjy · 2025-04-18T14:18:59Z

frb_example/pure_dart/rust/Cargo.toml

+flutter_rust_bridge = { path = "../../../frb_rust", features = [
+    "backtrace",
+    "chrono",
+    "dart-opaque",


nit: given that dart_opaque is a default feature, maybe we can keep these lines unchanged

fzyzcjy · 2025-04-18T14:19:08Z

frb_example/pure_dart/rust/Cargo.toml

@@ -26,5 +31,7 @@ tokio = { version = "1.34.0", features = ["rt"] }

 [features]
 internal_feature_for_testing = []
+dart-opaque = ["flutter_rust_bridge/dart-opaque"]


nit: is it possible we delete this line (same as above)

fzyzcjy · 2025-04-18T14:19:16Z

frb_example/pure_dart/build.dart

@@ -2,4 +2,4 @@ import 'package:flutter_rust_bridge_utils/flutter_rust_bridge_utils.dart';

 void main(List<String> args) async => simpleBuild(args,
    // This is solely used for testing
-    features: ['internal_feature_for_testing']);
+    features: ['dart-opaque', 'internal_feature_for_testing']);


nit: (same, can we del it)

aran · 2025-04-18T15:39:05Z

Unfortunately I'm not going to be able to prioritize reworking this to use another approach so I'm going to close it out and maybe revisit another time.

A couple of the semi-related commits like ae8c875 and a75df11 might be independently useful even without the dart-opaque change, not sure.

fzyzcjy · 2025-04-19T00:14:09Z

Sad to hear that :(

A couple of the semi-related commits like ae8c875 and a75df11 might be independently useful even without the dart-opaque change, not sure.

Yes that looks useful - do you have interest in making one (or two) separate PRs for these?

aran and others added 8 commits April 4, 2025 08:19

feat: allow omission of dart-opaque feature

26c79f2

refactor: remove feat_no_dart_opaque example

36e8b9f

feat: support testing with multiple feature configurations including …

ae8c875

…--no-default-features

feat: add --no-default-features test configuration for dart_minimal

b38f974

fix: re-enable build_cli generation and re-generate

a75df11

Merge branch 'master' into feat

60ad9a7

feat: add dart-opaque feature to FRB integration template

7af7ae0

docs: add comment documenting dart-minimal dart-opaque feature removal

52ce82c

aran mentioned this pull request Apr 7, 2025

RFC: feat: allow omission of dart-opaque feature #2662

Merged

aran added 2 commits April 7, 2025 14:24

fix: add dart-opaque feature across frb_examples

55f1ec7

fix: pure_dart specifies the dart-opaque feature since it uses it

5d69cf9

aran force-pushed the feat branch from 726e295 to 5d69cf9 Compare April 7, 2025 21:26

fix: pure_dart[_pde] uses dart-opaque

3682d0c

fix: enable dart-opaque for dart web tests for pure_dart[_pde]

96d2a18

Merge branch 'master' into feat

efa3cbc

fzyzcjy reviewed Apr 18, 2025

View reviewed changes

aran closed this Apr 18, 2025

fzyzcjy mentioned this pull request May 16, 2025

Make "rust-async" feature optional #2723

Open

3 tasks

feat: (continued) allow omission of dart-opaque feature #2681

feat: (continued) allow omission of dart-opaque feature #2681

Uh oh!

Conversation

aran commented Apr 7, 2025

Changes

Uh oh!

aran commented Apr 7, 2025

Uh oh!

fzyzcjy commented Apr 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aran commented Apr 8, 2025

Uh oh!

codecov bot commented Apr 8, 2025

Codecov Report

Uh oh!

fzyzcjy commented Apr 8, 2025

Uh oh!

aran commented Apr 9, 2025

Uh oh!

fzyzcjy commented Apr 9, 2025

Uh oh!

aran commented Apr 9, 2025

Uh oh!

fzyzcjy commented Apr 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aran commented Apr 18, 2025

Uh oh!

fzyzcjy commented Apr 19, 2025

Uh oh!

Uh oh!

fzyzcjy commented Apr 7, 2025 •

edited

Loading