Skip to content

[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

Conversation

ValentinVignal
Copy link
Contributor

@ValentinVignal ValentinVignal commented Apr 3, 2025

Fixes flutter/flutter#162487
Fixes flutter/flutter#166576

Pre-Review Checklist

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

Footnotes

  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. 2 3

@@ -275,6 +275,7 @@ class GoRoute extends RouteBase {
super.parentNavigatorKey,
super.redirect,
this.onExit,
this.caseSensitive = false,
Copy link
Contributor Author

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?

Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok ! Will do !

@ValentinVignal
Copy link
Contributor Author

Converting as draft while the design doc is being written and the code updated

@ValentinVignal
Copy link
Contributor Author

@ValentinVignal ValentinVignal marked this pull request as ready for review April 4, 2025 12:18
@ValentinVignal
Copy link
Contributor Author

@chunhtai The URL is now case sensitive by default

sfshaza2 pushed a commit to flutter/website that referenced this pull request Apr 7, 2025
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.
@ValentinVignal
Copy link
Contributor Author

The migration guide has been merged and is now available at the go url

@ValentinVignal ValentinVignal force-pushed the go-router/Add-case-sensitive-to-go-route branch from e4aff26 to ca1432f Compare April 8, 2025 15:03
@@ -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;
Copy link
Contributor

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

Copy link
Contributor Author

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.
Copy link
Contributor

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

Copy link
Contributor Author

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

@ValentinVignal ValentinVignal requested a review from chunhtai April 15, 2025 09:53
Copy link
Contributor

@chunhtai chunhtai left a 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!

@ValentinVignal ValentinVignal added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 16, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 16, 2025
Copy link
Contributor

auto-submit bot commented Apr 16, 2025

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.

@ValentinVignal ValentinVignal force-pushed the go-router/Add-case-sensitive-to-go-route branch from 48ba286 to 12f0136 Compare April 16, 2025 06:43
@ValentinVignal ValentinVignal added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 16, 2025
@auto-submit auto-submit bot merged commit b03228f into flutter:main Apr 16, 2025
82 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 16, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 21, 2025
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Apr 21, 2025
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
CodixNinja pushed a commit to CodixNinja/flutter that referenced this pull request May 15, 2025
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
androidseb pushed a commit to androidseb/packages that referenced this pull request Jun 8, 2025
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.
FMorschel pushed a commit to FMorschel/packages that referenced this pull request Jun 9, 2025
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: go_router
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add go_router 15 migration guide Case Sensitivity Issue in go_router When Manually Entering URLs with Incorrect Case in Path Segments
2 participants