-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[go_router] Adds caseSensitive
to GoRoute
#8992
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
[go_router] Adds caseSensitive
to GoRoute
#8992
Conversation
@@ -275,6 +275,7 @@ class GoRoute extends RouteBase { | |||
super.parentNavigatorKey, | |||
super.redirect, | |||
this.onExit, | |||
this.caseSensitive = false, |
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.
@chunhtai, from your comment flutter/flutter#162487 (comment),
based on https://www.w3.org/TR/WD-html40-970708/htmlweb.html, the URL should be case sensitive
I prefer to set the default to `true, ' but that would be a breaking change. I'm fine writing a migration doc
What do you prefer me to do?
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.
I agree we should probably default it to true to provide the correct behavior by default, but yeah we should bump major version and a breaking change doc if we do that
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.
Ok ! Will do !
Converting as draft while the design doc is being written and the code updated |
Design document has been created: flutter/website#11867 |
@chunhtai The URL is now case sensitive by default |
Adds the migration guide for go_router v15 (See flutter/packages#8992, and more precisely flutter/packages#8992 (comment)) ## Presubmit checklist - [x] This PR is marked as draft with an explanation if not meant to land until a future stable release. - [x] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer.
The migration guide has been merged and is now available at the go url |
e4aff26
to
ca1432f
Compare
@@ -218,7 +218,17 @@ abstract class RouteMatchBase with Diagnosticable { | |||
final String newMatchedLocation = | |||
concatenatePaths(matchedLocation, pathLoc); | |||
final String newMatchedPath = concatenatePaths(matchedPath, route.path); | |||
if (newMatchedLocation.toLowerCase() == uri.path.toLowerCase()) { | |||
|
|||
final String caseSensitiveNewMatchedLocation; |
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.
I think the new name is more confusing than the old name, because it can contains case insensitive string in the else case
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.
Fair enough, What do you think of newMatchedLocationToCompare
and uriPathToCompare
? I added them in Add better name and documentation
@@ -437,6 +438,9 @@ class GoRoute extends RouteBase { | |||
/// ``` | |||
final ExitCallback? onExit; | |||
|
|||
/// Whether the path is case sensitive or not. |
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.
Can you add more description? perhaps with some example
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.
Sure, I added more in Add better name and documentation, tell me if you need more
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.
LGTM, thanks for fixing this!
autosubmit label was removed for flutter/packages/8992, because - The status or check suite Mac_arm64 custom_package_tests stable has failed. Please fix the issues identified (or deflake) before re-applying this label. |
48ba286
to
12f0136
Compare
flutter/packages@2fcc403...ac21f53 2025-04-20 engine-flutter-autoroll@skia.org Roll Flutter from 3ed38e2 to cfb887c (17 revisions) (flutter/packages#9118) 2025-04-19 stuartmorgan@google.com [various] Scrubs pre-SDK-21 Android code (flutter/packages#9112) 2025-04-18 engine-flutter-autoroll@skia.org Roll Flutter from ecabb1a to 3ed38e2 (23 revisions) (flutter/packages#9114) 2025-04-18 ricardodalarme@outlook.com [flutter_svg] feat: Expose the `colorMapper` property in `SvgPicture` (flutter/packages#9043) 2025-04-18 stuartmorgan@google.com [tool] Add initial file-based command skipping (flutter/packages#8928) 2025-04-18 stuartmorgan@google.com [pigeon] Convert test plugins to SPM (flutter/packages#9105) 2025-04-18 10687576+bparrishMines@users.noreply.github.com [webview_flutter] Adds support to control overscrolling (flutter/packages#8451) 2025-04-17 louisehsu@google.com [in_app_purchase] add Storefront.countryCode() and AppStore.sync() (flutter/packages#8900) 2025-04-17 145144088+camfrandsen@users.noreply.github.com [webview_flutter_wkwebview] Expose the allowsLinkPreview property in WKWebView for iOS (flutter/packages#5029) 2025-04-17 10687576+bparrishMines@users.noreply.github.com [webview_flutter_android][webview_flutter_wkwebview] Adds platform implementations to set over-scroll mode (flutter/packages#9101) 2025-04-17 1063596+reidbaker@users.noreply.github.com [shared_preferences] Update AGP to 8.9.1 (flutter/packages#9106) 2025-04-17 10687576+bparrishMines@users.noreply.github.com [pigeon] Adds Kotlin lint tests to example code and fix lints (flutter/packages#9034) 2025-04-17 30872003+misos1@users.noreply.github.com [video_player_avfoundation] enable more than 30 fps (flutter/packages#7466) 2025-04-17 engine-flutter-autoroll@skia.org Roll Flutter from aef4718 to ecabb1a (25 revisions) (flutter/packages#9104) 2025-04-16 stuartmorgan@google.com [pigeon] Unify iOS and macOS test plugins (flutter/packages#9100) 2025-04-16 engine-flutter-autoroll@skia.org Roll Flutter from db68c95 to aef4718 (7 revisions) (flutter/packages#9098) 2025-04-16 10687576+bparrishMines@users.noreply.github.com [webview_flutter_platform_interface] Adds method to set overscroll mode (flutter/packages#9099) 2025-04-16 matanlurey@users.noreply.github.com Update `CODEOWNERS` (flutter/packages#8984) 2025-04-16 stuartmorgan@google.com [google_sign_is] Update iOS SDK to 8.0 (flutter/packages#9081) 2025-04-16 robert.odrowaz@leancode.pl [camera_avfoundation] Implementation swift migration (flutter/packages#8988) 2025-04-16 32538273+ValentinVignal@users.noreply.github.com [go_router] Adds `caseSensitive` to `GoRoute` (flutter/packages#8992) 2025-04-16 engine-flutter-autoroll@skia.org Manual roll Flutter from 30e53b0 to db68c95 (98 revisions) (flutter/packages#9092) 2025-04-15 stuartmorgan@google.com [tool] Run config-only build for iOS/macOS native-test (flutter/packages#9080) 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 flutter-ecosystem@google.com 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
flutter/packages@2fcc403...ac21f53 2025-04-20 engine-flutter-autoroll@skia.org Roll Flutter from 409a8ac to cd51fa3 (17 revisions) (flutter/packages#9118) 2025-04-19 stuartmorgan@google.com [various] Scrubs pre-SDK-21 Android code (flutter/packages#9112) 2025-04-18 engine-flutter-autoroll@skia.org Roll Flutter from d0741df to 409a8ac (23 revisions) (flutter/packages#9114) 2025-04-18 ricardodalarme@outlook.com [flutter_svg] feat: Expose the `colorMapper` property in `SvgPicture` (flutter/packages#9043) 2025-04-18 stuartmorgan@google.com [tool] Add initial file-based command skipping (flutter/packages#8928) 2025-04-18 stuartmorgan@google.com [pigeon] Convert test plugins to SPM (flutter/packages#9105) 2025-04-18 10687576+bparrishMines@users.noreply.github.com [webview_flutter] Adds support to control overscrolling (flutter/packages#8451) 2025-04-17 louisehsu@google.com [in_app_purchase] add Storefront.countryCode() and AppStore.sync() (flutter/packages#8900) 2025-04-17 145144088+camfrandsen@users.noreply.github.com [webview_flutter_wkwebview] Expose the allowsLinkPreview property in WKWebView for iOS (flutter/packages#5029) 2025-04-17 10687576+bparrishMines@users.noreply.github.com [webview_flutter_android][webview_flutter_wkwebview] Adds platform implementations to set over-scroll mode (flutter/packages#9101) 2025-04-17 1063596+reidbaker@users.noreply.github.com [shared_preferences] Update AGP to 8.9.1 (flutter/packages#9106) 2025-04-17 10687576+bparrishMines@users.noreply.github.com [pigeon] Adds Kotlin lint tests to example code and fix lints (flutter/packages#9034) 2025-04-17 30872003+misos1@users.noreply.github.com [video_player_avfoundation] enable more than 30 fps (flutter/packages#7466) 2025-04-17 engine-flutter-autoroll@skia.org Roll Flutter from 9616f9c to d0741df (25 revisions) (flutter/packages#9104) 2025-04-16 stuartmorgan@google.com [pigeon] Unify iOS and macOS test plugins (flutter/packages#9100) 2025-04-16 engine-flutter-autoroll@skia.org Roll Flutter from a7ce7ff to 9616f9c (7 revisions) (flutter/packages#9098) 2025-04-16 10687576+bparrishMines@users.noreply.github.com [webview_flutter_platform_interface] Adds method to set overscroll mode (flutter/packages#9099) 2025-04-16 matanlurey@users.noreply.github.com Update `CODEOWNERS` (flutter/packages#8984) 2025-04-16 stuartmorgan@google.com [google_sign_is] Update iOS SDK to 8.0 (flutter/packages#9081) 2025-04-16 robert.odrowaz@leancode.pl [camera_avfoundation] Implementation swift migration (flutter/packages#8988) 2025-04-16 32538273+ValentinVignal@users.noreply.github.com [go_router] Adds `caseSensitive` to `GoRoute` (flutter/packages#8992) 2025-04-16 engine-flutter-autoroll@skia.org Manual roll Flutter from f2d54fd to a7ce7ff (98 revisions) (flutter/packages#9092) 2025-04-15 stuartmorgan@google.com [tool] Run config-only build for iOS/macOS native-test (flutter/packages#9080) 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 flutter-ecosystem@google.com 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Fixes flutter/flutter#162487 Fixes flutter/flutter#166576 ## Pre-Review Checklist [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
Fixes flutter/flutter#162487 Fixes flutter/flutter#166576 ## Pre-Review Checklist [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
Fixes flutter/flutter#162487
Fixes flutter/flutter#166576
Pre-Review Checklist
[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.md
to add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2 ↩3