Repository navigation
[go_router] Refactors imperative APIs and browser history - #4134
Merged
Merged
Conversation
chunhtai
commented
Jun 5, 2023
Contributor
Author
There was a problem hiding this comment.
Nothing really change for this class, most of the code is copy paste from redirect.dart and matching.dart
Contributor
Author
There was a problem hiding this comment.
This is a copy paste from matching.dart
chunhtai
force-pushed
the
refactor-imperative
branch
from
June 5, 2023 17:12
abb9022 to
9305fe5
Compare
Contributor
Author
|
ci seems to be weird this time, will try pushing a new commit later |
callmephil
reviewed
Jun 6, 2023
There was a problem hiding this comment.
Is it safe to bang base and completer without doing a null check beforehand?
Contributor
Author
There was a problem hiding this comment.
If they are null, it means something wrong with the GoRouteInformationProvider that provides malformed RouteInformation
chunhtai
changed the title
johnpryan
approved these changes
Jun 6, 2023
Contributor
There was a problem hiding this comment.
Is there any benefit / reason this doesn't extend the Codec class from dart:convert?
Contributor
There was a problem hiding this comment.
break; isn't needed if you're using Dart 3
chunhtai
force-pushed
the
refactor-imperative
branch
from
June 7, 2023 19:24
44430a3 to
afb4f7a
Compare
Contributor
Author
|
Hi @johnpryan, I updated this pr, the change is in the last commit
|
chunhtai
force-pushed
the
refactor-imperative
branch
from
June 7, 2023 20:51
afb4f7a to
e4b4463
Compare
chunhtai
force-pushed
the
refactor-imperative
branch
from
June 7, 2023 21:46
e4b4463 to
ed16a13
Compare
johnpryan
approved these changes
Jun 7, 2023
Contributor
Author
|
waiting for migration guide to merge flutter/website#8853 |
chunhtai
added
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 8, 2023
auto-submit Bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Jun 8, 2023
flutter/packages@a84b2c2...e13b8c4 2023-06-08 [email protected] [tool] Only run unit tests in Chrome for inline web (flutter/packages#4153) 2023-06-08 [email protected] [in_app_purchase] Make the _FeatureCard constructor const in the Android example app (flutter/packages#4162) 2023-06-08 [email protected] [shared_preferences] Fix initialization race (flutter/packages#4159) 2023-06-07 [email protected] [go_router] Refactors imperative APIs and browser history (flutter/packages#4134) 2023-06-07 [email protected] [various] Add `http` 1.0 compatibility (flutter/packages#4147) 2023-06-07 [email protected] [go_router_builder] Accept required parameters not in path (flutter/packages#4039) 2023-06-07 [email protected] Roll Flutter from 0b74153 to 8a5c22e (46 revisions) (flutter/packages#4160) 2023-06-07 [email protected] [pigeon] Require analyzer 5.13.0, prepare for NamedType refactoring. (flutter/packages#4127) 2023-06-07 [email protected] Roll Flutter (stable) from f92f441 to 682aa38 (1 revision) (flutter/packages#4157) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
This was referenced Jul 7, 2023
10 of 11 tasks
10 of 11 tasks
creatorpiyush
pushed a commit
to creatorpiyush/packages
that referenced
this pull request
Jun 10, 2026
Several thing. 1. I move all the imperative logic from RouterDelegate to RouteInformationParser, so that the imperative API can go through Router parsing pipeline. The Parser will handle modifying mutating RouteMatchList and produce the final RouteMatchList. The RouterDelegate would only focus on building the widget base on the final RouteMatchList 2. combine RouteMatcher and Redirector with RouteConfiguration. I feel that instead of passing three class instances around, we should probably just have one class for all the route parsing related utility. 3. serialize routeMatchList and store into browser history. This way we can let backward and forward button to reflect imperative operation as well. 4. Some minor clean ups
bisor0627
pushed a commit
to bisor0627/packages
that referenced
this pull request
Jun 19, 2026
Several thing. 1. I move all the imperative logic from RouterDelegate to RouteInformationParser, so that the imperative API can go through Router parsing pipeline. The Parser will handle modifying mutating RouteMatchList and produce the final RouteMatchList. The RouterDelegate would only focus on building the widget base on the final RouteMatchList 2. combine RouteMatcher and Redirector with RouteConfiguration. I feel that instead of passing three class instances around, we should probably just have one class for all the route parsing related utility. 3. serialize routeMatchList and store into browser history. This way we can let backward and forward button to reflect imperative operation as well. 4. Some minor clean ups
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Several thing.
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.