Fix some type checks in the `Actions` widget by LongCatIsLooong · Pull Request #180883 · flutter/flutter

LongCatIsLooong · 2026-01-13T04:28:25Z

The type parameter T in Action<T extends Intent> is contravariant which dart doesnt support yet, so unfortunately compile time type check is not enough, also the assert in _castAction is incorrect.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

gemini-code-assist

Code Review

This pull request provides a solid fix for a subtle type-checking issue within the Actions widget, stemming from Dart's current limitations with contravariant generic types. The approach of widening types to Action<Intent> and then performing safe runtime casts with detailed assertions is robust and developer-friendly. The new _debugCanHandleIntent method is an excellent addition for catching type mismatches during development. The test suite has also been appropriately updated with a valuable regression test. Overall, this is a well-executed and important fix.

loic-sharma · 2026-01-13T18:34:56Z

+      T() => null,
+      // The List literal is needed to reify the type I
+      // ignore: literal_only_boolean_expressions
+      null when <I>[] is List<T> => null,


Dang. I would've never thought of this in a million years. Very nice trick 👍

LongCatIsLooong · 2026-01-14T00:00:46Z

Added a test and reverted the public interface change.

I originally planned to add this to Actions.maybeFind

  /// ## Limitations:
  ///
  /// The return type `Action<T>?` may prevent this method from returning
  /// perfectly capable `Action`s bound to the given [Intent]. For instance, an
  /// `Action<Intent>` can be bound to any `Intent` because its type signature
  /// claims it can take any [Intent] object. However this method will not be
  /// able to return an `Action<Intent>` unless `T` is exactly `Intent`, because
  /// `Action<Intent>` is not a subtype of `Action<T>?`. The method will return
  /// null in this case, and assert in debug mode.
  ///
  /// It is recommanded to use `maybeInvoke` instead if , or explicitly set the type
  /// parameter `T` to `Intent`, for example:
  /// `final action = Actions.maybeFind<Intent>(context, intent);`

But neither alternative suggested above replaces maybeFind perfectly:

Actions.maybeInvoke does not create dependency between context and the Actions widget.
explicitly specify the return type to be Action would be breaking if declare-site variance is added to the language and the Action type signature is updated.

So I'm going to only give that suggestion when people actually run into the problem (as an assert).

loic-sharma

Excellent work!

loic-sharma · 2026-01-16T00:02:11Z

So I'm going to only give that suggestion when people actually run into the problem (as an assert).

SGTM, thanks for considering this! 👍

justinmc · 2026-01-20T23:24:21Z

@LongCatIsLooong FYI there are test failures (from triage).

flutter-dashboard · 2026-02-09T23:24:54Z

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Piinks · 2026-03-17T22:30:05Z

Hey @LongCatIsLooong is this still something you would like to land?

LongCatIsLooong · 2026-04-13T22:04:56Z

Added a limitation section to find and maybeFind, also made a few members on overrideable action classes private (they currently should be private as we never posed any subtypes of these classes but just to be extra careful).

loic-sharma · 2026-04-13T22:46:47Z

  /// If no [Actions] widget surrounds the given context, this function will
  /// assert in debug mode, and throw an exception in release mode.
  ///
+  /// {@macro flutter.widgets.actions.findLimitations}


I feel a bit iffy about reusing the exact same language here given that find cannot return null, but the limitations section assumes a nullable return type.

loic-sharma · 2026-04-13T23:15:06Z

  /// return null.
  ///
+  /// {@template flutter.widgets.actions.findLimitations}
+  /// ## Limitations:


What do you think of something like this?

/// ## Limitations /// /// It is strongly recommended that callers explicitly set the type parameter to /// `Intent` when the `intent` parameter is not null: /// /// ```dart /// Actions.maybeFind<Intent>(context, intent); // GOOD /// Actions.maybeFind(context, intent); // BAD /// ``` /// /// If the type parameter is not set to `Intent` when the `intent` parameter is /// not null, this method might be unable to return a perfectly capable `Action`. /// For instance, this method cannot return an `Action<Intent>` - an action /// that can be bound to any intent - unless `T` is exactly `Intent`. /// This will trigger assertions in debug mode.

Summary of changes:

I moved the recommendation before the explanation. This way you get the actionable advice immediately, and then you can keep reading if you want to learn why this is necessary. Or you can skip the explanation entirely!

I added a // BAD example to make it clearer what the recommendation is. I suspect folks might get confused by the Intent type parameter vs the intent parameter.

I tried to avoid "return null" so that this template also works for Actions.find. FYI, I expect that in release mode, Actions.find will throw an exception due to its use of a bang operator on a null value.

LMKWYT!

loic-sharma

The code changes look good to me, but I left a suggestion on the docs. Let me know what you think!

LongCatIsLooong · 2026-04-14T21:07:54Z

-      return callingAction!.invoke(intent);
-    }
-    return invokeAction(intent, context);
+    return callingAction?.invoke(intent) ?? invokeAction(intent, context);


Well this is a bad change because invoke can return null.

…191052) `Actions.handler` was calling `Actions.maybeFind(context)` without an explicit type argument or the intent, so downward inference resolved `T` to `Intent` and the action lookup used the literal `Intent` type instead of the intent's concrete runtime type. Since apps register actions keyed by their concrete `Intent` subtype, the lookup always missed and `handler` returned `null` for every intent. This regressed in flutter#180883 and first shipped in 3.47.0. The fix forwards the type and intent explicitly: `Actions.maybeFind<T>(context, intent: intent)`. ## Tests Adds regression tests for `Actions.handler`: it now finds and invokes the bound action for the intent's runtime type, and still returns null when the action is disabled or when no matching action is found. The first test fails without the fix. Fixes flutter#191045. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md Co-authored-by: Victor Sanni <[email protected]>

github-actions Bot added the framework flutter/packages/flutter repository. See also f: labels. label Jan 13, 2026

gemini-code-assist Bot reviewed Jan 13, 2026

View reviewed changes

Comment thread packages/flutter/lib/src/widgets/actions.dart

LongCatIsLooong commented Jan 13, 2026

View reviewed changes

Comment thread packages/flutter/lib/src/widgets/actions.dart Outdated

Comment thread packages/flutter/lib/src/widgets/actions.dart

loic-sharma self-requested a review January 13, 2026 18:07

loic-sharma reviewed Jan 13, 2026

View reviewed changes

Comment thread packages/flutter/lib/src/widgets/actions.dart Outdated

loic-sharma reviewed Jan 13, 2026

View reviewed changes

Comment thread packages/flutter/lib/src/widgets/actions.dart Outdated

loic-sharma reviewed Jan 13, 2026

View reviewed changes

Comment thread packages/flutter/test/widgets/actions_test.dart

PurplePolyhedron mentioned this pull request Jan 14, 2026

Proposal: Create FlexibleAction<IntentType> that can bind to subclass of IntentType #180635

Open

LongCatIsLooong mentioned this pull request Jan 14, 2026

Action.overridable cannot be overridden by a DoNothingAction #180435

Closed

loic-sharma previously approved these changes Jan 16, 2026

View reviewed changes

PurplePolyhedron mentioned this pull request Mar 10, 2026

Create FlexiableAction<T> binds to Intent that extends T #180590

Closed

9 tasks done

LongCatIsLooong dismissed loic-sharma’s stale review via fb020d2 April 13, 2026 21:56

LongCatIsLooong added the CICD Run CI/CD label Apr 13, 2026

loic-sharma reviewed Apr 13, 2026

View reviewed changes

LongCatIsLooong commented Apr 14, 2026

View reviewed changes

github-actions Bot removed the CICD Run CI/CD label Apr 14, 2026

LongCatIsLooong added the CICD Run CI/CD label Apr 14, 2026

github-actions Bot removed the CICD Run CI/CD label Apr 14, 2026

LongCatIsLooong added the CICD Run CI/CD label Apr 14, 2026

LongCatIsLooong requested a review from loic-sharma April 20, 2026 19:11

LongCatIsLooong mentioned this pull request May 5, 2026

Fix DoNothingAction assertion when overriding Action.overridable #184919

Closed

loic-sharma added CICD Run CI/CD autosubmit Merge PR when tree becomes green via auto submit App labels Jun 25, 2026

auto-submit Bot added this pull request to the merge queue Jun 26, 2026

Merged via the queue into flutter:master with commit f7689c7 Jun 26, 2026
95 checks passed

flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 26, 2026

adam-langley-minrisk mentioned this pull request Aug 13, 2026

Actions.handler always returns null since 3.47.0 (Actions.maybeFind no longer forwards intent type) #191045

Closed

omar-alshyokh mentioned this pull request Aug 13, 2026

Fix Actions.handler to forward the intent type to maybeFind #191052

Merged

8 of 10 tasks

JIWON1923 mentioned this pull request Sep 16, 2026

SelectionArea wrapping a TextField throws a TypeError on vertical caret movement due to an incompatible callingAction type #192844

Open

Conversation

LongCatIsLooong commented Jan 13, 2026

Pre-launch Checklist

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

loic-sharma Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LongCatIsLooong commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

loic-sharma left a comment

Choose a reason for hiding this comment

Uh oh!

loic-sharma commented Jan 16, 2026

Uh oh!

justinmc commented Jan 20, 2026

Uh oh!

flutter-dashboard Bot commented Feb 9, 2026

Uh oh!

Piinks commented Mar 17, 2026

Uh oh!

LongCatIsLooong commented Apr 13, 2026

Uh oh!

loic-sharma Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

loic-sharma Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loic-sharma left a comment

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

LongCatIsLooong commented Jan 14, 2026 •

edited

Loading

loic-sharma Apr 13, 2026 •

edited

Loading