Repository navigation
[go_router] Fix spurious onExit when refreshListenable re-parses a pushed route - #12212
Open
davidmigloz wants to merge 3 commits into
Open
davidmigloz wants to merge 3 commits into
davidmigloz wants to merge 3 commits into
Conversation
github-actions
Bot
added
p: go_router
triage-framework
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request excludes the ephemeral completer from the ImperativeRouteMatch equality (==) and hashCode calculations to prevent spurious onExit calls when a route is re-decoded (e.g., during a refreshListenable tick). It also adds corresponding unit and widget tests to verify this behavior. One review comment suggests wrapping an unawaited router.push call in unawaited to satisfy the unawaited_futures lint rule.
davidmigloz
changed the title
davidmigloz
force-pushed
the
upstream-imperative-match-completer-identity
branch
from
July 16, 2026 14:07
7133569 to
4756afd
Compare
chunhtai
reviewed
Aug 1, 2026
chunhtai
left a comment
chunhtai
left a comment
Contributor
There was a problem hiding this comment.
The approach Looks good to me, left some comments
davidmigloz
added a commit
to davidmigloz/flutter_packages
that referenced
this pull request
Aug 3, 2026
…s onExit A refreshListenable tick re-parses the current location; the decoded ImperativeRouteMatch minted a fresh completer, so the configuration compared unequal and onExit fired without any real exit. Reconciles live completers by pageKey on the re-decode path (reconcileImperativeCompleters) instead of changing ImperativeRouteMatch identity, so same-location replace() keeps working and awaited push() futures resolve. Squash of the two commits on the upstream PR branch: flutter#12212
Contributor
|
From triage: @chunhtai what is the status of this PR? You've said LGTM above, but there's no approval, and no second reviewer added. |
Piinks
reviewed
Oct 30, 2026
…s onExit A `refreshListenable` tick (e.g. tab refocus) re-parses the encoded current configuration. The codec mints a fresh `Completer` per `ImperativeRouteMatch`, so the re-decoded list compares unequal to the live one, the delegate's no-op fast path is defeated, and `onExit` fires on a route that never left the stack. Instead of dropping the completer from `ImperativeRouteMatch` identity — which breaks `replace()` to the same location (it reuses the old pageKey and differs only by its new completer, so the new configuration would compare equal, the `setNewRoutePath` fast path would no-op, and the returned future would be orphaned) — reconcile completers only on the re-decode path: for each decoded imperative match, adopt the live completer of the match with the same pageKey (unique per push). The re-decoded list then compares equal including completer, the fast path holds naturally, and in-flight push futures are preserved. Tests: onExit not fired on a refresh tick; live push completer resolves across a refresh; replace() to the same location still completes its future; reconcileImperativeCompleters adopts live completers by pageKey and keeps fresh ones when there is no live match.
davidmigloz
force-pushed
the
upstream-imperative-match-completer-identity
branch
from
October 30, 2026 23:12
c410139 to
694d712
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GoRoute.onExitfires spuriously for a pushed route that never left the navigation stack wheneverrefreshListenablere-parses that route. The same round-trip can also replace the livestate.extra: JSONMapandListvalues lose identity, a customextraCodecmay return a new object without value equality, and an unserializable in-memory object without a codec decodes asnull.The encoded configuration cannot preserve either the push completer or the identity of its extra. A decoded
ImperativeRouteMatchtherefore compares unequal to the live match even when it represents the same pushed route. That defeatssetNewRoutePath's no-op fast path, runsonExit, and can replace the live route state.On the re-decode path only, this change reconciles each decoded imperative match with the live match that has the same
pageKeyand URI. Reusing the whole live match preserves both its completer and exactextraobject, making an unchanged refresh parse equal to the configuration already on screen. The URI check prevents a key reused byreplacefrom matching a different route. If there is no live match or its URI differs, the decoded match is retained for genuine restoration or history navigation.ImperativeRouteMatch.operator==andhashCoderemain unchanged, and imperativepush/replace/pushReplacementnavigations do not use this reconciliation path. An earlier version of this PR excluded the completer from equality; that regressed same-locationreplace()by orphaning its returned future, so the final design preserves identity at decode time instead.Regression coverage includes:
onExiton a refresh tick;extraidentity for JSONMapandListvalues;extraidentity for a custom-codec object without value equality;replace()future completion;pageKeyand URI match, with decoded matches retained when no live match exists or the URI differs.Validated after rebasing onto current
main(go_router18.1.2):match_test.dartandon_exit_test.dartsuites;Fixes flutter/flutter#189578
Pre-Review Checklist
[shared_preferences]///).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