[go_router] Relax subroute path requirements (allow root and child routes to have the same path forms ) #7647
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
cedvdb
force-pushed
the
remove_unnecessary_path_requirements
branch
from
20a16d8
to
0b1eef0
Compare
cedvdb
changed the title
| loc = '/$loc'; | ||
| } | ||
| return _pathRE.matchAsPrefix('/$loc') as RegExpMatch? ?? | ||
| _pathRE.matchAsPrefix(loc) as RegExpMatch?; |
There was a problem hiding this comment.
Here we can't predict whether or not loc starts with / or not as it is user defined. We could make it so the GoRoute constructor requires all routes to start with / but that would be a breaking change
cedvdb
changed the title
cedvdb
changed the title
cedvdb
changed the title
cedvdb
changed the title
cedvdb
changed the title
| @@ -5502,6 +5473,94 @@ void main() { | |||
| ), | |||
| ); | |||
| }); | |||
|
|
|||
| testWidgets('should allow route paths without leading /', | |||
There was a problem hiding this comment.
not entirely sure about this test, wasn't this already allowed?
There was a problem hiding this comment.
Indeed. I felt it is more descriptive to add both next to each other even if somewhat unnecessary
Co-authored-by: chunhtai <[email protected]>
Before this change
GoRouteconstructor allowed Schroedinger routes which were neither in a valid state nor an invalid state until they were used. EG:GoRoute(path: '/some-route')was valid as a top route but invalid as a child route. This prevents routes from being moved around and prevents building upon go router.To solve this I see two solution:
/(or none)/Since I did not want to introduce a breaking change I followed option 2, which allows all routes to be of the form
/some-routeorsome-route. However, breaking changes aside, I believe it would be better to use solution 1 as it's more predictible.@chunhtai What do you think about those options ?
Related issue: Fix flutter/flutter#145867
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, or this PR is exempt from CHANGELOG changes.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.