Skip to content

[go_router] Fix spurious onExit when refreshListenable re-parses a pushed route#12212

Open
davidmigloz wants to merge 1 commit into
flutter:mainfrom
davidmigloz:upstream-imperative-match-completer-identity
Open

[go_router] Fix spurious onExit when refreshListenable re-parses a pushed route#12212
davidmigloz wants to merge 1 commit into
flutter:mainfrom
davidmigloz:upstream-imperative-match-completer-identity

Conversation

@davidmigloz

@davidmigloz davidmigloz commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

GoRoute.onExit fires spuriously for a pushed route that never left the navigation stack whenever refreshListenable fires (auth refresh, a tab-refocus listener) while that route is on top. If the route uses onExit for a "discard unsaved changes?" prompt, the user sees it on every refresh tick.

Root cause: ImperativeRouteMatch.completer is the ephemeral result channel for push(...).then(...) and can't survive a codec round-trip — a refreshListenable re-parse (and state restoration) decodes a brand-new Completer. The re-decoded match list therefore compares unequal to the live one even though nothing changed, defeating setNewRoutePath's no-op fast path; the router treats the re-parse as a real navigation change and runs onExit.

Fix: on the re-decode path only (the parser branch handling restoration, browser history, and refresh re-parses), carry over the live completer for each imperative match by pageKey (unique per push) via an internal RouteMatchList.reconcileImperativeCompleters. The re-decoded configuration then compares equal to the live one — completer included — so the fast path holds, onExit doesn't fire, and in-flight push futures are preserved. operator==/hashCode are untouched, and imperative navigations (push/replace/pushReplacement) never hit this path.

An earlier version of this PR excluded the completer from ImperativeRouteMatch equality instead. That regressed same-location replace() — whose match deliberately reuses pageKey and differs only by its new completer — orphaning the returned Future; see the PR thread for details.

Tests: no spurious onExit on a refresh tick; a live push future resolves across the tick; same-location replace() still completes its future; reconciliation contract tests in match_test.dart. Full go_router suite and analyzer green.

Fixes flutter/flutter#189578

Pre-Review Checklist

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

@github-actions github-actions Bot added p: go_router triage-framework Should be looked at in framework triage labels Jul 16, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment thread packages/go_router/test/on_exit_test.dart Outdated
…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 davidmigloz changed the title [go_router] Exclude completer from ImperativeRouteMatch equality [go_router] Fix spurious onExit when refreshListenable re-parses a pushed route Jul 16, 2026
@davidmigloz
davidmigloz force-pushed the upstream-imperative-match-completer-identity branch from 7133569 to 4756afd Compare July 16, 2026 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p: go_router triage-framework Should be looked at in framework triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[go_router] onExit fires spuriously for a pushed route when refreshListenable fires

1 participant