[go_router] Resolve relative ./ navigation against the imperative top (flutter#182441)#11726
Conversation
After `router.push('/products')`, Flutter's Router calls
`GoRouteInformationProvider.routerReportsNewRouteInformation` with the URI
returned by `restoreRouteInformation`. When
`GoRouter.optionURLReflectsImperativeAPIs` is false (the default), that URI
excludes any `ImperativeRouteMatch`es, so the provider's `_value.uri` is
reset to the non-imperative base (e.g. `/home`). The next
`push('./reviews')` then resolves the relative path against that stale
`_value.uri` inside `_setValue`, sending the user to `/home/reviews`
instead of `/products/reviews`.
The user-facing symptom of this in go_router_builder (#182441) is that a
`TypedRelativeGoRoute` child shared by multiple parents always resolves to
the first parent in the route tree: the generated `pushRelative` calls
`context.push('./<child>')`, which the provider misresolves.
Fix: resolve `./` paths up-front in `push`, `pushReplacement`, and
`replace` using the actual top of their `base` (drilling through
`ShellRouteMatch` and falling back to `base.uri` if no
`ImperativeRouteMatch` is on top). The location passed to `_setValue` is
then already absolute, sidestepping the stale `_value.uri`.
`go` is left unchanged: it does not stack and has no `base`, and the
existing test `relative "./" navigation resolves against current location`
in `on_enter_test.dart` continues to cover that path.
Added two regression tests in `imperative_api_test.dart` mirroring the
shared-parent scenario from #182441 across `push`, `pushReplacement`, and
`replace`. Full suite: 431/431 (was 430/430).
Fixes flutter/flutter#182441.
There was a problem hiding this comment.
Code Review
This pull request updates go_router to version 17.2.4 and addresses an issue where relative navigation using imperative APIs resolved against the incorrect base URI. The implementation introduces _topUriOf and _resolveRelativeAgainstBase helper methods in GoRouteInformationProvider to ensure relative paths resolve against the actual visible top route. The fix is applied to the push, pushReplacement, and replace methods, and is accompanied by new regression tests. I have no feedback to provide.
|
Thanks for the contribution! I see you've opened three different PRs here, each of which has a different version of the checklist, and none of which uses this repository's actual checklist. Please update all the PRs to use, and follow, the correct checklist, and then mark them as ready for review. Please pay particular attention to the checklist item about reading and understanding our AI contribution guidelines, as this pattern of variable checklists (along with the opening of a dozen Flutter PRs simultaneously as a new contributor) strongly suggests a heavy reliance on AI in creating the PRs. |
Fixes flutter/flutter#182441.
Description
After
router.push('/products'), Flutter'sRoutercallsGoRouteInformationProvider.routerReportsNewRouteInformationwith the URI returned byrestoreRouteInformation. WhenGoRouter.optionURLReflectsImperativeAPIsisfalse(the default), that URI excludes anyImperativeRouteMatches, so the provider's_value.uriis reset from/productsback to the non-imperative base/home. The nextpush('./reviews')then resolves the relative path against that stale_value.uriinside_setValue, sending the user to/home/reviewsinstead of/products/reviews.The user-facing symptom of this in
go_router_builder(#182441) is that aTypedRelativeGoRoutechild shared by multiple parents always resolves to the first parent in the route tree: the generatedpushRelativecallscontext.push('./<child>'), which the provider misresolves.Verifying the diagnosis
Reproduced at the
GoRouterruntime layer (nogo_router_builderrequired):Instrumenting
_setValueshows_value.uriflipping from/products(set by step 1) back to/home(set by Flutter'srouterReportsNewRouteInformationcallback) before step 2 runs.Fix
Resolve
./paths up-front inpush,pushReplacement, andreplaceusing the actual top of theirbase(drilling throughShellRouteMatchand using the URI of the topImperativeRouteMatch, falling back tobase.uriotherwise). The location passed to_setValueis then already absolute, sidestepping the stale_value.uri.gois left unchanged: it does not stack and has nobase, and the existingrelative "./" navigation resolves against current locationtest inon_enter_test.dartcontinues to cover thego('./...')path.Tests
packages/go_router/test/imperative_api_test.dartmirroring the shared-parent scenario from #182441:push('./<child>')from an imperative parent.pushReplacement('./<child>')andreplace('./<child>').Versioning
Bumped
go_router17.2.3 → 17.2.4 and added a CHANGELOG entry per pub versioning philosophy.List which issues are fixed by this PR.
flutter/flutter#182441
Pre-launch Checklist
dart format).[go_router]).pubspec.yamlwith an appropriate new version.CHANGELOG.mdto add a description of the change.