Repository navigation
[go_router] Clarify onEnter/redirect ordering, add regression test - #12337
auto-submit[bot] merged 14 commits into
Conversation
github-actions
Bot
added
p: go_router
triage-framework
|
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[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the documentation for RoutingConfig.onEnter in go_router to clarify that onEnter is evaluated exactly once per navigation against the literal target URI, strictly before redirects run. It also adds regression tests in on_enter_test.dart to verify this behavior and demonstrate how to guard redirect targets. Feedback on the changes suggests simplifying a test route definition by removing an unused builder callback on a route that always redirects.
|
@googlebot I signed it! |
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
Hey @developerashkan thanks for contributing!
Co-authored-by: Kate Lovett <[email protected]>
|
Because of the volume of PRs we receive, we require that new contributors use our checklist to guide them through critical steps in creating a Flutter PR. This PR's description is missing that checklist, and is missing required elements from the checklist. Please edit the PR description to add the checklist, then ensure that you have completed all of the steps. If you need help, consider asking for advice on the #hackers-new channel on Discord. |
developerashkan
changed the title
…ub.com/developerashkan/packages into docs/onenter-redirect-chain-visibility
|
An existing Git SHA, To re-trigger presubmits after closing or re-opeing a PR, or pushing a HEAD commit (i.e. with |
Format on_enter_test.dart
Updated comments for clarity in onEnter test cases.
…er#191734) flutter/packages@df2ba94...740f093 2026-08-25 [email protected] [cupertino_ui] Remove unused parameters from constructors of generic classes. (flutter/packages#12457) 2026-08-25 [email protected] [material_ui] Remove unused parameters from constructors of generic classes. (flutter/packages#12458) 2026-08-25 [email protected] Ignore shared code for iOS platform implementation of Google Maps plugin (flutter/packages#12529) 2026-08-25 [email protected] [cross_file] fixed `readAsString` decoding in-memory bytes as UTF-16 (flutter/packages#12479) 2026-08-25 [email protected] [cupertino_ui] Remove two items assert to allow CupertinoTabBar to have one tab (flutter/packages#12546) 2026-08-25 [email protected] [cupertino_ui] Fix covered sheet revealing root route through top gap (flutter/packages#12530) 2026-08-25 [email protected] Sync release-go_router-18.0.0 to main (flutter/packages#12575) 2026-08-25 [email protected] Sync release-material_ui-1.1.0 to main (flutter/packages#12577) 2026-08-25 [email protected] Sync release-cupertino_ui-1.0.1 to main (flutter/packages#12576) 2026-08-24 [email protected] [quick_actions_ios]unskip XCUITests (flutter/packages#12436) 2026-08-24 [email protected] Fix RangeSlider thumb overlay remains visible after touch interaction ends (flutter/packages#12560) 2026-08-24 [email protected] [google_sign_in] Increase iOS coverage tests (flutter/packages#12484) 2026-08-24 [email protected] [camera_android_camerax] Migrate from dart_skills_lint to skills_lint (flutter/packages#12543) 2026-08-24 [email protected] [go_router] Clarify onEnter/redirect ordering, add regression test (flutter/packages#12337) 2026-08-24 [email protected] [material_ui] Port flutter/flutter flutter#185149 "Slider label clips the screen" (flutter/packages#12572) 2026-08-24 [email protected] Roll Flutter from 65c9a8d to 9a82789 (17 revisions) (flutter/packages#12578) 2026-08-24 [email protected] [tool] Fix dart_test.yaml parsing (flutter/packages#12574) If this roll has caused a breakage, revert this CL and stop the roller 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: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…lutter#12337) ## What this does Clarifies the `onEnter` doc comment on `RoutingConfig` to explicitly warn that `onEnter` is evaluated once, against the literal navigated-to URI, and is never re-invoked for locations reached only through a `redirect:` chain. Adds a regression test locking in this behavior for the exact scenario reported in flutter/flutter#188014, plus a second test demonstrating the correct workaround (putting the guard inside `redirect:` instead of `onEnter`). ## Why flutter/flutter#188014 reports what looks like a bug: an `onEnter` guard checking for an intermediate redirect target (`/a`, reached via a `redirect:` on `/`) never fires. Tracing the pipeline shows this is actually intended, existing behavior — see the `'onEnter called once when top-level redirect chains'` test (written for the related #178984), which asserts the same ordering. The docs didn't make this ordering/visibility gap explicit though, so it's an easy trap. This PR closes that documentation gap and adds test coverage so the behavior can't silently change without a test failing. ## Related Fixes flutter/flutter#188014 (as a documentation clarification, not a behavior change — see discussion on the issue). ## Pre-Review Checklist > [!IMPORTANT] > This PR was prepared with assistance from an AI agent (Claude). The > code, doc comment, and tests have been reviewed by the author, who > understands the change and takes responsibility for it. [^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.
5 participants
What this does
Clarifies the
onEnterdoc comment onRoutingConfigto explicitly warnthat
onEnteris evaluated once, against the literal navigated-to URI,and is never re-invoked for locations reached only through a
redirect:chain. Adds a regression test locking in this behavior for the exact
scenario reported in flutter/flutter#188014, plus a second test
demonstrating the correct workaround (putting the guard inside
redirect:instead ofonEnter).Why
flutter/flutter#188014 reports what looks like a bug: an
onEnterguardchecking for an intermediate redirect target (
/a, reached via aredirect:on/) never fires. Tracing the pipeline shows this isactually intended, existing behavior — see the
'onEnter called once when top-level redirect chains'test (written forthe related #178984), which asserts the same ordering. The docs didn't
make this ordering/visibility gap explicit though, so it's an easy trap.
This PR closes that documentation gap and adds test coverage so the
behavior can't silently change without a test failing.
Related
Fixes flutter/flutter#188014 (as a documentation clarification, not a
behavior change — see discussion on the issue).
Pre-Review Checklist
[shared_preferences]///).Important
This PR was prepared with assistance from an AI agent (Claude). The
code, doc comment, and tests have been reviewed by the author, who
understands the change and takes responsibility for it.
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