-
Notifications
You must be signed in to change notification settings - Fork 382
feat: Add oxidized Result support for fallible functions #2950
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.
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
base: master
Are you sure you want to change the base?
Conversation
When oxidized package is detected in pubspec.yaml dependencies: - Fallible Rust functions return Future<Result<T, E>> instead of Future<T> - Errors are returned as Err(...) instead of throwing exceptions - Panics still throw PanicException (bugs, not expected errors) Runtime (frb_dart): - Add decodeAsResult() to SimpleDecoder for Result-based decoding - Add decodeObjectAsResult/decodeWireSyncTypeAsResult to BaseCodec - Add executeNormalAsResult/executeSyncAsResult to BaseHandler - Export Result, Ok, Err from flutter_rust_bridge_for_generated_common.dart Codegen (frb_codegen): - Add has_dependency() to DartRepository - Add detect_oxidized_dependency() in generator_parser.rs - Add use_oxidized field to GeneratorApiDartInternalConfig - Wrap fallible function return types in Result<T, E> - Use executeNormalAsResult/executeSyncAsResult for fallible functions Tested with dart_minimal example: - Async functions: Future<Result<T, E>> - Sync functions: Result<T, E>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2950 +/- ##
==========================================
- Coverage 98.57% 1.87% -96.71%
==========================================
Files 464 357 -107
Lines 19202 14152 -5050
==========================================
- Hits 18928 265 -18663
- Misses 274 13887 +13613 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
When a type alias like \`type Result<T> = std::result::Result<T, MyError>\` is used, FRB now correctly extracts the error type from the alias definition instead of falling back to AnyhowException. - Store generic type aliases separately in HirFlatTypeAlias - Look up generic Result aliases when parsing function return types - Substitute type parameters to resolve the actual error type
- Add example error types and functions using type aliases - Update generated Dart/Rust bindings with new test cases
|
You are welcome and thanks for the great job! |
fzyzcjy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tiny nits (I will review more ASAP when having time)
frb_codegen/src/library/codegen/config/internal_config_parser/generator_parser.rs
Outdated
Show resolved
Hide resolved
frb_codegen/src/library/codegen/config/internal_config_parser/generator_parser.rs
Show resolved
Hide resolved
|
Thanks @fzyzcjy ! Addressed changes. |
|
Looks great! I will review ASAP, but have a tight ddl now thus may not have time very recently :( |
|
Hey @fzyzcjy would you be able to review this when you get some time? Got like 3 projects using this fork and it's getting a little annoying lol. No rush, I understand you're busy. Just thought I'd remind! |
|
Hi, happy to see your work is being used by multi projects, and I also think this is a great feature! I am really sorry for being super busy w/ a very emergent and important ddl (or indeed, multiple overlapped deadlines, that's why I have been non resting for a long time) :( I will review as soon as the current ddl ends, potentially it will not be overlapped with another emergent ddl. By the way, if you (or maybe Cursor / Claude Code etc) put the tests in pure_dart etc instead of the dart_minimal, then I do not need to manually do the moving / refactoring / checking, then I will potentially just merge it with a lightweight review, since it is does not change current code a lot, and can be refactored and further reviewed later. |
|
No worries at all mate. Sorry to hear you're so busy, best of luck with it! I'll handle the tests, lmk if I've done anything wrong when I get around to it. Again no stress at all, take your time. |
|
Thank you! |
Hey @fzyzcjy
This PR adds support for serializing
Result<T, E>to the equivalent exposed by oxidized. I think the package's API has stabilized, obviously idiomatic w.r.t. Rust conventions; it's well-tested and doesn't pull in a crap-ton of API's like a lot of the sibling packages.Thanks again for this package, I hope to contribute more over the coming years; what you've done is magnificent and IMO the best addition to the Flutter ecosystem thus far.
Changes
Fixes #2683
Procedure and Checklist
In order to quickly iterate and avoid slowing down development pace by making CI pass, only the following simplified steps are needed, and I (fzyzcjy) will handle the rest of CI / moving the tests currently (will have more automation in the future).
frb_example/dart_minimal.dart_minimal's CI green.Utility commands
cargo run --manifest-path /path/to/flutter_rust_bridge/frb_codegen/Cargo.toml -- generate./frb_internal test-dart-native --package frb_example/dart_minimal