[go_router_builder] Fix generated onExit handling by agarwalrahul2702 · Pull Request #12239 · flutter/packages

agarwalrahul2702 · 2026-07-18T13:31:54Z

Description

Generated typed GoRouteData and RelativeGoRouteData route constructors now pass hasOverriddenOnExit based on whether the route data class actually overrides onExit.

This keeps generated routes without onExit overrides aligned with handwritten GoRoute behavior, while still enabling onExit for routes that define it.

Root cause

go_router treats an omitted hasOverriddenOnExit marker as true for backward compatibility with older generated code. go_router_builder was omitting that marker for every generated typed route, so routes that did not override onExit still received an onExit callback. That changes pop timing and can surface Flutter's Navigator _RouteLifecycle.idle assertion during rapid system-back/re-enter navigation.

Tests

Reproduced the Navigator assertion locally with typed routes using GoRouteData.$route without hasOverriddenOnExit.
Added generator coverage in test_inputs/on_exit.dart / .expect for routes with and without onExit overrides.
Regenerated checked-in example .g.dart files.
Ran dart run tool/run_tests.dart in packages/go_router_builder.
Ran dart analyze --fatal-infos in packages/go_router_builder.
Ran dart test in packages/go_router_builder.
Ran dart format --output=none --set-exit-if-changed packages/go_router_builder.
Ran flutter_plugin_tools analyze --packages=go_router_builder --exact-match-only --no-downgrade.
Ran flutter_plugin_tools dart-test --packages=go_router_builder --exact-match-only.
Ran flutter_plugin_tools validate --packages=go_router_builder --exact-match-only.
Ran flutter_plugin_tools custom-test --packages=go_router_builder --exact-match-only.

Pre-Review Checklist

CLA status is left for the GitHub CLA bot/account owner to verify.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

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

google-cla · 2026-07-18T13:32:05Z

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.

gemini-code-assist

Code Review

This pull request updates the go_router_builder package to only enable onExit on generated routes when the route data class explicitly overrides it. It introduces a _hasOverriddenOnExit check in route_config.dart to inspect the class hierarchy and appends the hasOverriddenOnExit parameter to the generated route configurations. Additionally, the package version is bumped to 4.3.2, and corresponding test inputs and generated files are updated. Feedback suggests making the _hasOverriddenOnExit check more robust by also excluding GoRouteData and RelativeGoRouteData from being identified as user-overridden classes.

agarwalrahul2702 · 2026-07-18T13:58:07Z

Follow-up pushed in 2321012 to cover review feedback:

Added generator coverage for an inherited GoRouteData.onExit override.
Added generator coverage for RelativeGoRouteData.onExit.
Simplified the onExit test fixture to return bool synchronously.
Clarified CHANGELOG entry with [go_router_builder] Navigator assertion (_RouteLifecycle.idle) during rapid back-and-forth navigation flutter#189503 and the ^17.3.0 go_router requirement.

Local checks run with the repo Flutter SDK (Dart 3.12.2):

dart run script/tool/bin/flutter_plugin_tools.dart custom-test --packages=go_router_builder --exact-match-only
dart run script/tool/bin/flutter_plugin_tools.dart analyze --packages=go_router_builder --custom-analysis=script/configs/custom_analysis.yaml
dart run script/tool/bin/flutter_plugin_tools.dart dart-test --packages=go_router_builder --exact-match-only

justinmc · 2026-07-28T22:07:51Z

FYI @agarwalrahul2702 there are a bunch of conflicts to fix here.

hannah-hyj · 2026-08-31T23:47:51Z

@agarwalrahul2702 Hi there! This PR have some conflicts to resolve, do you still plan to work on this?

agarwalrahul2702 · 2026-09-01T03:33:19Z

@hannah-hyj I resolved conflicts can u review?

elliette · 2026-09-22T20:52:20Z

[from triage] gentle ping @hannah-hyj - are you still reviewing this PR or would ecosystem team to take over review? Thanks!

hannah-hyj · 2026-09-28T17:45:33Z

ecosystem team can take over this!

Piinks

Thanks for working on this and adding test coverage for inherited onExit overrides!

Since PR #11151 landed in 4.4.0 shortly after this PR was opened, main already generates hasOverriddenOnExit for direct route classes, and what this PR now fixes on top of main is detecting onExit overrides inherited from a base class or mixin (which classElement.methods missed). Could you update the PR description to reflect that, and also resolve the current merge conflicts with main (where the version is now at 4.5.2)?

I also left a few inline comments on the override lookup logic and a couple of leftover formatting diffs in unrelated test expectations.

Piinks · 2026-10-28T18:17:18Z

 import 'path_utils.dart';
 import 'type_helpers.dart';

+bool _hasOverriddenOnExit(InterfaceElement routeDataClass) {


Two edge cases can trip up _hasOverriddenOnExit here:

thisType.lookUpMethod defaults to concrete: false, which looks up the interface signature rather than the concrete implementation inherited at runtime. If an abstract base class or mixin redeclares onExit as abstract (for example, to add doc comments) and a concrete subclass does not override it, the runtime implementation is still the default no-op from go_router, but lookUpMethod returns the abstract declaration on the user's base class and generates hasOverriddenOnExit: true.

_GoRouteDataBase is a private class inside go_router, so users have no reason to know that name is reserved. Comparing enclosingElement?.displayName against '_GoRouteDataBase' as a raw string means that if a user names their own private base class _GoRouteDataBase and overrides onExit on it, the builder will treat it as un-overridden and generate hasOverriddenOnExit: false. Checking whether the concrete method originates from package:go_router (for example via its library URI or TypeChecker.fromUrl) avoids any class-name collision.

Piinks · 2026-10-28T18:17:38Z

@@ -1,3 +1,9 @@
+## 4.4.2
+
+* Fixes generated routes so `onExit` is only enabled when the route data class overrides it,


Since 4.4.0 (just below) already added hasOverriddenOnExit generation for direct route classes, could we reword this entry (and bump to 4.5.3 when rebasing onto main) so it specifically describes fixing hasOverriddenOnExit when onExit is inherited from a superclass or mixin?

Piinks · 2026-10-28T18:18:11Z

@@ -0,0 +1,48 @@
+// Copyright 2013 The Flutter Authors


Could we also add cases here for onExit provided via a mixin on GoRouteData, as well as the abstract redeclaration and private _GoRouteDataBase superclass cases from the comment on route_config.dart?

Piinks · 2026-10-28T18:19:35Z

                .cast<Uri>()
                .toList() ??
            const <Uri>[],
-        enumTest:


The changes in this file are formatting-only diffs unrelated to onExit. Could we revert those five files so the diff stays focused?

[go_router_builder] Generate onExit override marker

58c34c7

github-actions Bot added p: go_router_builder triage-framework Should be looked at in framework triage labels Jul 18, 2026

gemini-code-assist Bot reviewed Jul 18, 2026

View reviewed changes

Comment thread packages/go_router_builder/lib/src/route_config.dart Resolved

agarwalrahul2702 added 3 commits July 18, 2026 19:08

[go_router_builder] Update go_router constraint

fa44929

[go_router_builder] Harden onExit override detection

27c444a

[go_router_builder] Add onExit override coverage

2321012

Piinks requested a review from hannah-hyj July 21, 2026 22:18

Merge branch 'main' into fix-go-router-builder-on-exit-189503

92502f7

hannah-hyj removed their request for review September 28, 2026 17:45

Piinks self-requested a review September 29, 2026 21:20

Piinks reviewed Oct 28, 2026

View reviewed changes

		@@ -1,3 +1,9 @@
		## 4.4.2

		* Fixes generated routes so `onExit` is only enabled when the route data class overrides it,

Conversation

agarwalrahul2702 commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Root cause

Tests

Pre-Review Checklist

Footnotes

Uh oh!

google-cla Bot commented Jul 18, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

agarwalrahul2702 commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

justinmc commented Jul 28, 2026

Uh oh!

hannah-hyj commented Aug 31, 2026

Uh oh!

agarwalrahul2702 commented Sep 1, 2026

Uh oh!

elliette commented Sep 22, 2026

Uh oh!

hannah-hyj commented Sep 28, 2026

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

Piinks Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

Piinks Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

Piinks Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

Piinks Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

agarwalrahul2702 commented Jul 18, 2026 •

edited

Loading

agarwalrahul2702 commented Jul 18, 2026 •

edited

Loading