[go_router] Fix shell route key collisions and dynamic branch updates by Gorniv · Pull Request #12549 · flutter/packages

Gorniv · 2026-08-22T13:49:10Z

This supersedes #11143, which was closed as inactive. It incorporates the review feedback from that PR.

Problem

When push() navigates between sibling or nested regular ShellRoute branches, multiple shell pages and navigators can coexist in the widget tree.

This could produce duplicate page or widget keys because:

_CustomNavigator was keyed using GlobalObjectKey(navigatorKey.hashCode), so distinct navigator keys with the same hash were treated as the same key.
ShellRouteMatch.pageKey was derived only from route.hashCode.
Re-entering a ShellRoute that was already present in the imperative stack reused its page and navigator identities.

The first implementation in #11143 created fresh page and navigator keys for every ShellRouteMatch. Review correctly pointed out that this made navigator identity unstable across route reparses and prevented the configured ShellRoute.navigatorKey from consistently identifying its navigator.

RoutingConfig updates could also leave inactive loaded StatefulShellRoute branches holding matches and navigator widgets built from the old route objects.

Fix

This version addresses that feedback by:

preserving the configured navigator key during normal route matching and reparsing;
making shell page keys identity-aware, so distinct routes remain distinct even when their hashCode values collide;
keying the nested _CustomNavigator wrapper with a value-equal ValueKey of its navigator key;
assigning scoped page and navigator keys only when an imperative push would otherwise collide with another live regular ShellRoute match;
deriving that scope from the persistent ImperativeRouteMatch.pageKey;
carrying existing shell matches through recursive imperative push matching, so nested key collisions are detected;
reparsing loaded inactive StatefulShellRoute branches and rebuilding their cached navigator widgets when route objects change;
scoping saved matches to their shell and resetting branches whose base location was removed;
identifying reparsed branches by their navigator key, including after nested pushReplacement();
preserving branch and root observers when rebuilding inactive or preloaded navigators, honoring notifyRootObserver;
leaving the root navigator wrapper and normal configured-key navigation unchanged.

Tests

The regression test push to a sibling shell route under the same parent shell route forces both navigator-key and route-hash collisions.

It covers:

pushing to a sibling shell route;
re-entering the first shell route while its original instance is still in the stack;
preserving navigator and page state across match-list reconstruction and refresh();
detecting and scoping collisions in nested ShellRouteMatch trees;
popping back through both shell routes;
navigating with go() after the imperative stack is removed.

Additional regressions cover the nested push sequence in the review comment, inactive loaded branches after a RoutingConfig update (review comment), and the exact flows from flutter/flutter#148712 and flutter/flutter#140586.

Eight additional regression cases cover the follow-up review findings: a removed base route with a surviving push (with and without preload), nested pushReplacement(), a removed pushed route, and branch/root observer behavior for both preload and notifyRootObserver settings. The observer regression uses pageless Navigator.push() so a GoRouter rebuild cannot mask detached observers; it also verifies navigator-state preservation and updated clipBehavior.

No screenshots are included because this is a routing/runtime fix.

Validation

flutter test --no-pub: 471 passed
flutter analyze --no-pub: no issues
flutter_plugin_tools validate --check-for-missing-changes --packages go_router: no issues
flutter_plugin_tools publish-check --packages go_router: no issues
License check: passed

Release note: This package uses batch releases, so the fix is recorded in pending_changelogs/shell_route_key_collisions.yaml; pubspec.yaml and CHANGELOG.md remain unchanged.

Pre-Review Checklist

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩²

gemini-code-assist

Code Review

This pull request resolves ShellRoute key collisions during imperative navigation by introducing custom page and navigator key classes (_ShellRoutePageKey and _ShellRouteNavigatorKey) and updating the match cloning logic to scope keys when collisions are detected. Feedback from the review suggests replacing GlobalObjectKey with ValueKey to prevent state loss during rebuilds, and implementing a recursive collision check to correctly handle nested ShellRouteMatches.

MaximeRougieux · 2026-08-24T15:23:50Z

Hi! I've been working on a related fix and wanted to discuss it so we can land on a common implementation that fixes everything right the first time.

My case is a little different from yours, I have an app that rebuild their route tree from application state, via GoRouter.routingConfig. Every rebuild produces new RouteBase objects, so a page key based on the route object identity changes even though nothing about the route actually changed. Flutter then replaces the shell's page instead of updating it, which destroys and recreates the nested Navigators and their state, and sometimes crashes with "Multiple widgets used the same GlobalKey".

You can look at what I did here : https://github.com/MaximeRougieux/flutter-packages/commits/fix/go-router-dynamic-shell-routes-handling/. Both our fixes does not cover the issue the other is trying to fix, but we are touching the same logic in match.dart.

As my AI chatbot tells it : we're fixing two different problems on the same line, in opposite directions: you're making the key stricter about object identity, I'm making it not depend on object identity at all.

This being said, your choice of using a custom ValueKey class which overrides == is better than what I did, but for it to fix my issue, we'd need it to compare a stable identity. What I chose to look at is:

The navigatorKey for ShellRoutes
The _shellStateKey (key property in the constructor) for StatefulShellRoutes

It would not impact your fix, which we can verify with the tests you added.

I already sent you my fork, if you want, I can directly implement the changes and push them on this PR, or you can have a look, whichever is the simplest for you. If you and/or the maintainers prefer, I can wait for this to land before pushing a separate PR.

Some related issues:

Use stable configured keys for shell page identity, retain stateful branch state by navigator key, and reparse imperative matches nested under shell routes. Co-authored-by: Maxime Rougieux <[email protected]>

MaximeRougieux · 2026-08-25T09:58:08Z

Thank you @Gorniv! Everything works as expected on my end 😄

Gorniv · 2026-08-25T10:27:42Z

Thanks for confirming, @MaximeRougieux!

I added follow-up regression coverage in 34bdd15:

verifies that multiple imperative matches nested under a ShellRoute survive a routing configuration rebuild, preserve their state and stack order, and complete the correct push futures when popped;
enables the existing obsolete-branch test with stable branch navigator keys, verifying that retained branches preserve their NavigatorState while the removed branch is disposed.

The full go_router suite passes with 443 tests, flutter analyze reports no issues, and the original sibling shell-route push regression also passes separately.

elliette

Thank you! I have one suggestion, but otherwise looks good to me.

MaximeRougieux · 2026-09-09T14:47:01Z

@elliette FYI, I'm waiting on this PR to be merged because it contains multiple fixes I need, if @Gorniv's doesn't come back, I'll open up another one integrating the requested change to his changes.

Could you tell me what's the typical acceptable delay before closing the PR due to inactivity please?

Piinks

Thanks for working on this!

Can you add regression tests covering the specific reproduction flows cited:

flutter/flutter#148712
- Rebuilding GoRouter with ShellRoute(builder: ..., navigatorKey: ...) without duplicate GlobalKey crashes.
flutter/flutter#140586
- Navigating A -> Push B -> Push C -> PushReplacement A -> Push B without !keyReservation.contains(key) failures.

Gorniv · 2026-09-18T10:36:02Z

Thanks for the review! I added exact regression coverage for both reported flows in 64ec450:
• flutter/flutter#148712: rebuilds GoRouter with ShellRoute(builder: ..., navigatorKey: ...), verifies there is no duplicate GlobalKey exception, and confirms the shell NavigatorState is preserved. 
• flutter/flutter#140586: exercises A -> push B -> push C -> pushReplacement A -> push B using NoTransitionPage keys from GoRouterState.pageKey, verifying each step completes without !keyReservation.contains(key) failures. 
I also addressed the value-equality suggestions for _ShellRoutePageKey and _ShellRouteNavigatorKey. The full go_router suite passes with 445 tests, and flutter analyze reports no issues.

elliette

Thank you for the updates! This all LGTM.

Piinks

I think there are a few more things to address, can you take a look at and incorporate these tests cases to make sure they work as expected?

Widen cloned match lists, reset invalid branch bases, and scope saved imperative matches to the shell. Identify reparsed branches by navigator key and share observer merging across active, inactive, and preloaded navigators. Add regression coverage for removed routes, nested replacement, observer opt-out, and clipping.

Piinks

LGTM thank you for contributing!

Roll Packages from a1ed23c to da40eaf (33 revisions) flutter/packages@a1ed23c...da40eaf 2026-10-15 [email protected] [pigeon]add support for sealed classes in native interop (flutter/packages#13175) 2026-10-15 [email protected] [pigeon] Replace full JNI bindings with stubs for API classes that aren't needed (flutter/packages#13173) 2026-10-14 [email protected] [material_ui] Fix self-comparison and assert bugs in semantics_tester (flutter/packages#12722) 2026-10-14 [email protected] [camera_avfoundation] Replace the deprecated 'videoOrientation' with 'videoRotationAngle' on iOS 17+ (flutter/packages#13247) 2026-10-14 [email protected] [material_ui] Fix GridTile crash when the incoming constraints are unbounded (flutter/packages#12586) 2026-10-14 [email protected] [in_app_purchase_storekit] Remove unused and deprecated "SKDownload" hosted content (flutter/packages#13222) 2026-10-14 [email protected] [material_ui] Add supportingTextPadding to InputDecorator (flutter/packages#12769) 2026-10-14 [email protected] [vector_graphics_codec] Add support for encoding and decoding paint blur commands (flutter/packages#13148) 2026-10-14 [email protected] [go_router] Fix redirect zone async error handling (flutter/packages#12836) 2026-10-14 [email protected] Sync release-go_router-18.1.0 to main (flutter/packages#13260) 2026-10-14 [email protected] Sync release-cupertino_ui-1.1.3 to main (flutter/packages#13258) 2026-10-14 [email protected] [material_ui] add enableInlinePrediction parameter to TextFormField (flutter/packages#12793) 2026-10-14 [email protected] Sync release-material_ui-1.7.0 to main (flutter/packages#13259) 2026-10-14 [email protected] [material_ui] Add the Material shapes. (flutter/packages#12771) 2026-10-14 [email protected] [material_ui] Add tertiary tap callbacks to InkResponse and InkWell (flutter/packages#12676) 2026-10-14 [email protected] Roll Flutter from 4249ccb to 426be96 (44 revisions) (flutter/packages#13256) 2026-10-14 [email protected] [material_ui] fixes expanded state in the a11y_assessment home page dropdown (flutter/packages#13219) 2026-10-14 [email protected] [VPAT] Fix semantics in a11y_assessments drawer, radio, and rail demos (flutter/packages#13159) 2026-10-14 [email protected] [google_maps_flutter] Add onPointOfInterestTap (flutter/packages#11872) 2026-10-14 [email protected] [go_router] Fix shell route key collisions and dynamic branch updates (flutter/packages#12549) 2026-10-14 [email protected] Manual roll Flutter from d03768e to 4249ccb (158 revisions) (flutter/packages#13220) 2026-10-14 [email protected] [image_picker_ios] Require Flutter 3.47 / iOS 15 and drop deprecated kUTType code paths (flutter/packages#13224) 2026-10-13 [email protected] [in_app_purchase_storekit] Fixes Storekit 2 flaky tests (flutter/packages#13213) 2026-10-13 [email protected] [local_auth] Convert Android to new Pigeon suspend support (flutter/packages#12894) 2026-10-13 [email protected] [material_ui] Add enabled parameter to DropdownButtonFormField (flutter/packages#12610) 2026-10-13 [email protected] [go_router] Link ShellRoute docs to StatefulShellRoute (flutter/packages#13120) 2026-10-13 [email protected] FadeForwards page transition performance (flutter/packages#13161) 2026-10-13 [email protected] [in_app_purchase_android] Guard async billing callbacks against duplicate listener responses (flutter/packages#12640) 2026-10-13 [email protected] [google_sign_in] Remove the Objective-C exception catcher (flutter/packages#13156) 2026-10-13 [email protected] [camera_avfoundation] Remove availability checks below the package minimum iOS 13 (flutter/packages#13214) 2026-10-13 [email protected] [google_maps_flutter_ios] Remove iOS 14 availability check from example UI tests (flutter/packages#13218) 2026-10-13 [email protected] [google_sign_in] Convert implementation to Kotlin (flutter/packages#13005) 2026-10-13 [email protected] [image_picker_ios] PR 2/5 Add a PHImageManager image-data seam and migrate plugin native tests to Swift Testing (flutter/packages#13079) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: ...

[go_router] Fix ShellRoute key collisions

d16c403

github-actions Bot added p: go_router triage-framework Should be looked at in framework triage labels Aug 22, 2026

Gorniv marked this pull request as ready for review August 22, 2026 13:50

gemini-code-assist Bot reviewed Aug 22, 2026

View reviewed changes

Comment thread packages/go_router/lib/src/builder.dart Resolved

Comment thread packages/go_router/lib/src/match.dart Resolved

[go_router] Address ShellRoute key review feedback

679ce61

Gorniv and others added 2 commits August 25, 2026 11:01

[go_router] Preserve shell state across routing updates

c93e3c6

Use stable configured keys for shell page identity, retain stateful branch state by navigator key, and reparse imperative matches nested under shell routes. Co-authored-by: Maxime Rougieux <[email protected]>

[go_router] Add routing config regression coverage

34bdd15

Piinks requested review from Piinks and elliette August 25, 2026 20:52

elliette added the CICD Run CI/CD label Aug 27, 2026

elliette reviewed Sep 1, 2026

View reviewed changes

Comment thread packages/go_router/lib/src/match.dart Resolved

elliette added the waiting for response The Flutter team cannot make further progress on this PR until the author responds label Sep 8, 2026

[go_router] Reuse route match traversal helper

a8cab5c

flutter-dashboard Bot removed the CICD Run CI/CD label Sep 9, 2026

github-actions Bot removed the waiting for response The Flutter team cannot make further progress on this PR until the author responds label Sep 9, 2026

Piinks reviewed Sep 10, 2026

View reviewed changes

Comment thread packages/go_router/lib/src/builder.dart Resolved

Comment thread packages/go_router/lib/src/match.dart Resolved

Comment thread packages/go_router/lib/src/match.dart Resolved

Gorniv added 2 commits September 18, 2026 12:10

[go_router] Preserve custom navigator state on restore

e4d9867

[go_router] Cover reported shell route regressions

64ec450

Piinks reviewed Sep 24, 2026

View reviewed changes

Comment thread packages/go_router/lib/src/match.dart Resolved

Comment thread packages/go_router/lib/src/route.dart Resolved

Gorniv added 5 commits September 25, 2026 01:30

[go_router] Fix nested shell collisions and stale branch routes

47eacd1

[go_router] Cover nested shell unwinding and branch stacks

05164d1

[go_router] Verify imperative branch stack after config update

d669488

[go_router] Merge upstream/main and resolve release conflicts

662eeee

[go_router] Update branch with latest upstream/main

349fb19

elliette added the CICD Run CI/CD label Oct 1, 2026

elliette approved these changes Oct 1, 2026

View reviewed changes

Piinks reviewed Oct 2, 2026

View reviewed changes

Comment thread packages/go_router/lib/src/match.dart Resolved

Comment thread packages/go_router/lib/src/route.dart Resolved

Gorniv added 3 commits October 2, 2026 23:26

[go_router] Update branch before addressing inactive branch review

23d85f4

[go_router] Sync upstream/main after review fixes

d5d6857

flutter-dashboard Bot removed the CICD Run CI/CD label Oct 2, 2026

Gorniv changed the title ~~[go_router] Fix ShellRoute key collisions during imperative navigation~~ [go_router] Fix shell route key collisions and dynamic branch updates Oct 2, 2026

Gorniv added 3 commits October 3, 2026 00:16

[go_router] Update branch with latest upstream/main

65b751f

[go_router] Sync branch with latest upstream/main

97d2641

[go_router] Sync PR branch with upstream/main

5d854e0

Piinks approved these changes Oct 14, 2026

View reviewed changes

Piinks added CICD Run CI/CD autosubmit Merge PR when tree becomes green via auto submit App labels Oct 14, 2026

auto-submit Bot merged commit 17e14c2 into flutter:main Oct 14, 2026
14 checks passed

engine-flutter-autoroll mentioned this pull request Oct 15, 2026

Roll Packages from a1ed23c8f760 to da40eaf4548a (33 revisions) flutter/flutter#194459

Merged

This was referenced Nov 5, 2026

[go_router] Keep shell navigator state when a parent shell moves it #13565

Open

[go_router] App hangs when leaving a nested ShellRoute whose builder reads GoRouterState.of (regression in 18.1.1) flutter/flutter#196038

Open

Conversation

Gorniv commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Tests

Validation

Pre-Review Checklist

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

MaximeRougieux commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaximeRougieux commented Aug 25, 2026

Uh oh!

Gorniv commented Aug 25, 2026

Uh oh!

elliette left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MaximeRougieux commented Sep 9, 2026

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Gorniv commented Sep 18, 2026

Uh oh!

Uh oh!

Uh oh!

elliette left a comment

Choose a reason for hiding this comment

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Gorniv commented Aug 22, 2026 •

edited

Loading

MaximeRougieux commented Aug 24, 2026 •

edited

Loading