Repository navigation
fix(mix): report FlutterErrorDetails in debug mode when Prop value conversion fails - #1096
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
leoafarias
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9c8f1ad49
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| debugPrint( | ||
| 'Mix: could not convert value of type ${value.runtimeType} ' | ||
| 'to Mix<$V>. Register a MixConverter for <$V> or pass a Mix via Prop.mix.', | ||
| FlutterError.reportError( |
There was a problem hiding this comment.
When a converter is missing in a debug build, FlutterError.reportError invokes the global FlutterError.onError handler and then returns; it does not throw. Consequently, resolveProp still drops the unconverted value and returns the remaining Mix result, and a handler can swallow the error exactly as this new test does, so the intended fail-fast behavior is not implemented. Throw the FlutterError from the assertion block and test with throwsA if resolution is meant to abort.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Intentional: the first commit threw, and a9c8f1a switched to FlutterError.reportError so a missing converter is reported, like other recoverable framework errors (layout overflow), while the widget still renders the latest source. Reported errors reach FlutterError.onError, which fails widget tests. The CHANGELOG entry now records this and the release-mode fallback change.
Add an Unreleased CHANGELOG entry for the release-mode change (the latest unconverted value now wins over earlier Mix sources) and the debug reportError diagnostic, and split the test so each case asserts once.
Release mix 2.3.1: Prop conversion fallback and debug reporting (#1096).
Summary
Replaces the silent
debugPrint('Mix: could not convert value of type ...')insideassertinProp.resolvePropwithFlutterError.reportError(FlutterErrorDetails(...))so that failedMix<V>conversions surface through Flutter's standard error pipeline (FlutterError.onError) in debug/test mode while still returning the fallback resolved value without crashing the widget build.Root Cause & Motivation
When a
Prop<V>accumulates both aMix<V>source (e.g. a customMix<double>subclass passed viaProp.mix(...)) and a plain value source (ValueSource<V>orTokenSource<V>resolving toV),hasMixValuesistrueandProp.resolvePropconverts every non-Mix<V>entry viaMixConverterRegistry.instance.tryConvert<V>(value).Previously, when no
MixConverter<V>was registered (such as for scalardouble),Prop.resolveProponly calleddebugPrintinside anassertblock and silently dropped the unconverted value. BecausedebugPrintdoes not route throughFlutterError.onError, invalidMix<V>+ scalarVcombinations went unnoticed influtter testwhile printing console noise on every animation frame.Using
FlutterError.reportError(FlutterErrorDetails(...))inside theassert(() { ... return true; }())block follows Flutter's standard non-fatal error reporting pattern (likeRenderFlexoverflow):resolvePropcontinues and returns the resolved fallback value in both debug and release builds instead of throwing mid-build.assertblock is stripped in release builds.flutter testautomatically:TestWidgetsFlutterBindinghooksFlutterError.onError, so widget tests fail immediately (andtester.takeException()captures theFlutterError).Release-mode behavior change
resolvePropalso changes what release builds return when a source can't be converted. Previously the unconverted value was dropped and earlierMixsources were merged, soProp.mix(a).mergeProp(Prop.value(b))resolved toa. Now an unconverted value replaces theMixvalues collected before it, so the latest source wins, as everywhere else in Mix. A laterMixsource still wins over it. The CHANGELOG records this under Bug fixes.Verification
packages/mix/test/src/core/prop_test.dartcovers both orders: a laterMixwins and the failure is reported (20.0), and a later unconverted value wins over an earlierMix(30.0). The second test fails without the fix.flutter test test/src/core/prop_test.dartdart analyze lib/src/core/prop.dart test/src/core/prop_test.dart