Feat: Add readFrom method to ModalRoute #164684

rkishan516 · 2025-03-06T07:51:01Z

Feat: Add readFrom method to ModalRoute
fixes: #163467

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.

justinmc

This approach makes sense but we might be setting a precedent here about how to access a value from an InheritedWidget/Model without subscribing, so I want to make sure we get it right. See my comment about the option of making _ModalScopeStatus public.

justinmc · 2025-04-10T23:05:04Z

packages/flutter/lib/src/widgets/routes.dart

+  ///  * [of], which provides the same information but will cause rebuilds when
+  ///    the route changes.
+  static ModalRoute<T>? readFrom<T extends Object?>(BuildContext context) {
+    return context.getInheritedWidgetOfExactType<_ModalScopeStatus>()?.route as ModalRoute<T>?;


If we made _ModalScopeStatus public would that also solve the problem? I'm not sure whether that's a good idea, just thinking this through.

Yes, I think making _ModalScopeStatus public would solve the problem, but i don't know reasoning behind why this was made private at first place.

Just an idea, in #165289 I am planning to migrate Theme subscribing to selector pattern. So, if we start following that same pattern everywhere, we may have 2 methods for subscription and for read. WDYT ?

I just left another review over on that PR. I'm really liking that approach but still thinking through the Object? types that end up all over. Maybe we should finish that PR first and then come back to this one?

Yup, make sense.

justinmc · 2025-04-10T23:08:06Z

packages/flutter/test/widgets/routes_test.dart

+    // Should just build page 2.
+    expect(log, <String>['building page 2']);
+    log.clear();
+  });


Indeed this fails when using ModalRoute.of 👍

vcgupta-indihood · 2025-06-16T06:20:59Z

Hi, any plans to complete this PR?

rkishan516 · 2025-06-16T07:45:16Z

Yes, @justinmc since we are not going with #165289, we can have another review on this MR now. Would you mind reviewing again ?

justinmc

Ok coming back to this now. Some things we should decide before committing to this:

This is a big precedent for when methods create dependencies or not, so we should think this through and get the naming right. Related: #169539. A discussion on of methods and whether or not they should create dependencies.
Is the alternative of making _ModalScopeStatus public viable?

Overall I think this is a problem we should probably solve though, since there is no way around the fact that _ModalScopeStatus is private now.

rkishan516 · 2025-06-18T01:15:06Z

@justinmc I am not sure, why _ModalScopeStatus was made private in first place ? If that can be public that we should go on that path.

justinmc

@chunhtai @dkwingsmt Do you guys have any thoughts on whether we should add a non-subscribing method here versus making _ModalScopeStatus public? I'm leaning the latter. This relates to your discussion in #169539.

dkwingsmt · 2025-06-23T21:13:00Z

I prefer having non-subscribing methods over publicizing inherited classes such as _ModalScopeStatus for the following reasons:

It is a common problem across all widgets to query inherited properties in a non-subscribing way, and what we are deciding here is probably going to be applied to all widgets. Since currently most inherited classes are private, it'll be a much larger change to publicize all such inherited classes than just adding a new method beside all the .ofs, which doesn't occupy the public namespace and provide better discoverability.
Publicizing methods instead of the full inherited classes allows better control over what and how the properties can be retrieved, and is easier to learn for users.
With the .of methods as a well-established pattern, it is cleaner and more intuitive to have something similar parallel to it. Publicizing inherited classes overlaps the design intention of the .of pattern, i.e. it would make more sense if we didn't have .of in the first place.

dkwingsmt · 2025-06-23T21:42:47Z

In general I'm in favor of this PR. My only concern is the name, since, again, we're setting a precedence for all such methods. I consulted a few AIs and I picked the following suggestions:

Name	Note
`ModalRoute.readFrom(context)` (the current proposal)	Another benefit: Riverpod has established the pattern of "read" being a non-subscribing query
`ModalRoute.lookup(context)`	Simple, and hard to misunderstand
`ModalRoute.onceOf(context)`	Similar to `of` and therefore easy to learn. However not as grammarly correct in English, especially if we combine with more properties, such as `.onceIsOpenOf`
Notable mentions
`ModalRoute.getFrom(context)`
`ModalRoute.get(context)`
`ModalRoute.nowOf(context)`

chunhtai · 2025-06-23T21:43:22Z

Also agree to @dkwingsmt 's point, though instead of adding a new API, I think we should standarize this and use createdependencies bool https://api.flutter.dev/flutter/widgets/Focus/of.html

dkwingsmt · 2025-06-23T21:45:55Z

@chunhtai There was a discussion in #169539 (comment), where @davidhicks980 said that an optional parameter createDependency might not be the best idea in practice and preferred separate methods.

chunhtai · 2025-06-23T21:49:08Z

Thanks for the link, I will continue the discussion in the issue

Piinks · 2025-07-29T22:27:19Z

Greetings from stale PR triage! 👋
What is the status of this PR?

rkishan516 · 2025-07-30T01:33:01Z

Hey @Piinks, waiting on decision from #169539.

Piinks · 2025-10-20T22:14:11Z

Is this still waiting on #169539?

dkwingsmt · 2025-10-20T22:17:32Z

I think we haven't really made up our mind to make a decision for #169539. Maybe I can host a dash forum for this.

rkishan516 · 2025-12-12T13:40:05Z

I think we haven't really made up our mind to make a decision for #169539. Maybe I can host a dash forum for this.

@dkwingsmt did we came up with some conclusion on this?

dkwingsmt · 2025-12-15T22:40:34Z

Yeah. Sorry I haven't had time, although I did plan to write a design doc for it, maybe within the next weeks.

rkishan516 · 2025-12-16T00:32:26Z

Yeah. Sorry I haven't had time, although I did plan to write a design doc for it, maybe within the next weeks.

Cool cool, take your time.

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: routes Navigator, Router, and related APIs. labels Mar 6, 2025

Feat: Add readFrom method to ModalRoute

e900d4b

rkishan516 force-pushed the read-from-modal-route branch from 3100adb to e900d4b Compare March 31, 2025 01:39

rkishan516 marked this pull request as ready for review March 31, 2025 01:40

justinmc reviewed Apr 10, 2025

View reviewed changes

justinmc reviewed Jun 17, 2025

View reviewed changes

justinmc reviewed Jun 23, 2025

View reviewed changes

justinmc marked this pull request as draft August 5, 2025 22:17

Feat: Add readFrom method to ModalRoute #164684

Are you sure you want to change the base?

Feat: Add readFrom method to ModalRoute #164684

Conversation

rkishan516 commented Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

rkishan516 Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

rkishan516 Apr 11, 2025 • 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!

justinmc Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

rkishan516 Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

justinmc Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

vcgupta-indihood commented Jun 16, 2025

Uh oh!

rkishan516 commented Jun 16, 2025

Uh oh!

justinmc left a comment • 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!

rkishan516 commented Jun 18, 2025

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

dkwingsmt commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dkwingsmt commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chunhtai commented Jun 23, 2025

Uh oh!

dkwingsmt commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chunhtai commented Jun 23, 2025

Uh oh!

Piinks commented Jul 29, 2025

Uh oh!

rkishan516 commented Jul 30, 2025

Uh oh!

Piinks commented Oct 20, 2025

Uh oh!

dkwingsmt commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rkishan516 commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dkwingsmt commented Dec 15, 2025

Uh oh!

rkishan516 commented Dec 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rkishan516 commented Mar 6, 2025 •

edited

Loading

rkishan516 Apr 11, 2025 •

edited

Loading

justinmc left a comment •

edited

Loading

dkwingsmt commented Jun 23, 2025 •

edited

Loading

dkwingsmt commented Jun 23, 2025 •

edited

Loading

dkwingsmt commented Jun 23, 2025 •

edited

Loading

dkwingsmt commented Oct 20, 2025 •

edited

Loading

rkishan516 commented Dec 12, 2025 •

edited

Loading