[go_router] fix: PopScope.onPopInvokedWithResult not called in branch routes by Gustl22 · Pull Request #9245 · flutter/packages

Gustl22 · 2025-05-13T15:18:09Z

If using PopScope inside StatefulShellRoute.indexedStack it is not called when on top most route.

flutter/flutter#138737 (comment)

Pre-Review Checklist

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

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

chunhtai

I think the approach is fine. just a nitpick

chunhtai · 2025-05-16T21:07:22Z

packages/go_router/lib/src/delegate.dart

-      if (potentialCandidate.canPop()) {
-        state = walker.navigatorKey.currentState;
-      }
+      states.insert(0, potentialCandidate);


you can do a reversed at the end

Thanks, I updated the code to use reversed and iterable as return type.

…p-scope

chunhtai

LGTM

…p-scope # Conflicts: # packages/go_router/CHANGELOG.md

hannah-hyj

LGTM

…p-scope # Conflicts: # packages/go_router/CHANGELOG.md

Gustl22 · 2025-06-09T21:24:35Z

@chunhtai any chance to have this get merged ;D Thx in advance :D

…p-scope # Conflicts: # packages/go_router/CHANGELOG.md # packages/go_router/pubspec.yaml

Gustl22 · 2025-06-18T13:10:39Z

@chunhtai @hannah-hyj reminder to get this merged. Unfortunately, as I am not part of that organization, I cannot add the autosubmit label.

Piinks · 2025-06-18T19:06:23Z

reminder to get this merged. Unfortunately, as I am not part of that organization, I cannot add the autosubmit label.

Ack! This will likely have a merge conflict once #8352 lands. I should be able to fix the conflict here and get this merged once that one lands.

packages/go_router/CHANGELOG.md

auto-submit · 2025-06-18T23:14:11Z

autosubmit label was removed for flutter/packages/9245, because - The status or check suite Linux_android android_build_all_packages master has failed. Please fix the issues identified (or deflake) before re-applying this label.

Gustl22 · 2025-06-19T05:39:53Z

Thank you @Piinks for landing this 🎉

…n branch routes (flutter/packages#9245)

flutter/packages@715a0a5...0ec4053 2025-06-19 engine-flutter-autoroll@skia.org Roll Flutter from 8303a96 to 85a9b4f (93 revisions) (flutter/packages#9457) 2025-06-19 32538273+ValentinVignal@users.noreply.github.com [go_router] Update sype safe routing topic to use mixin from go_router_builder 3.0.0 (flutter/packages#9422) 2025-06-19 august.oberhauser@swissinfo.ch [go_router] fix: PopScope.onPopInvokedWithResult not called in branch routes (flutter/packages#9245) 2025-06-18 10687576+bparrishMines@users.noreply.github.com [pigeon] Create a message call free InstanceManager when running unit tests (flutter/packages#9395) 2025-06-18 737941+loic-sharma@users.noreply.github.com [go_router] Use library prefix for meta (flutter/packages#9434) 2025-06-18 41765610+ahyangnb@users.noreply.github.com [go_router] fix Popping state and re-rendering scaffold at the same time doesn't update the URL on web [new] (flutter/packages#8352) 2025-06-18 robert.odrowaz@leancode.pl [camera_avfoundation] Fix incorrect types in image stream events (flutter/packages#9418) 2025-06-18 stuartmorgan@google.com [go_router_builder] Temporarily restrict `build` (flutter/packages#9453) 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 flutter-ecosystem@google.com 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

… routes (flutter#9245)

flutter/packages@715a0a5...0ec4053 2025-06-19 engine-flutter-autoroll@skia.org Roll Flutter from 8303a96 to 85a9b4f (93 revisions) (flutter/packages#9457) 2025-06-19 32538273+ValentinVignal@users.noreply.github.com [go_router] Update sype safe routing topic to use mixin from go_router_builder 3.0.0 (flutter/packages#9422) 2025-06-19 august.oberhauser@swissinfo.ch [go_router] fix: PopScope.onPopInvokedWithResult not called in branch routes (flutter/packages#9245) 2025-06-18 10687576+bparrishMines@users.noreply.github.com [pigeon] Create a message call free InstanceManager when running unit tests (flutter/packages#9395) 2025-06-18 737941+loic-sharma@users.noreply.github.com [go_router] Use library prefix for meta (flutter/packages#9434) 2025-06-18 41765610+ahyangnb@users.noreply.github.com [go_router] fix Popping state and re-rendering scaffold at the same time doesn't update the URL on web [new] (flutter/packages#8352) 2025-06-18 robert.odrowaz@leancode.pl [camera_avfoundation] Fix incorrect types in image stream events (flutter/packages#9418) 2025-06-18 stuartmorgan@google.com [go_router_builder] Temporarily restrict `build` (flutter/packages#9453) 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 flutter-ecosystem@google.com 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

… routes (flutter#9245)

github-actions bot added the p: go_router label May 13, 2025

Gustl22 mentioned this pull request May 13, 2025

[go_router] PopScope seems to be incompatible with GoRouter flutter/flutter#138737

Open

2 tasks

Gustl22 added 4 commits May 14, 2025 11:25

add tests

210f6f1

fix: PopScope.onPopInvokedWithResult not called in branch routes

2d55e2b

alter structure

7dbe8f6

changelog

f2b12ec

Gustl22 force-pushed the 138737-go-router-pop-scope branch from 4dc074a to f2b12ec Compare May 14, 2025 09:29

Gustl22 marked this pull request as ready for review May 14, 2025 09:30

Gustl22 requested a review from chunhtai as a code owner May 14, 2025 09:30

chunhtai reviewed May 16, 2025

View reviewed changes

Gustl22 added 3 commits May 19, 2025 12:23

Merge remote-tracking branch 'upstream/main' into 138737-go-router-po…

83746f9

…p-scope

use reversed list

231d4e5

use Iterable as return type

476b912

Gustl22 requested a review from chunhtai May 19, 2025 12:49

chunhtai approved these changes May 22, 2025

View reviewed changes

chunhtai requested a review from hannah-hyj May 22, 2025 19:22

Merge remote-tracking branch 'upstream/main' into 138737-go-router-po…

708f3fe

…p-scope # Conflicts: # packages/go_router/CHANGELOG.md

hannah-hyj approved these changes May 28, 2025

View reviewed changes

Gustl22 added 3 commits May 29, 2025 07:36

Merge branch 'main' into 138737-go-router-pop-scope

e29fd3c

Merge remote-tracking branch 'upstream/main' into 138737-go-router-po…

5b4b0b5

…p-scope # Conflicts: # packages/go_router/CHANGELOG.md

Adapt version after merge once again

11dd49c

Merge remote-tracking branch 'upstream/main' into 138737-go-router-po…

8c8634f

…p-scope # Conflicts: # packages/go_router/CHANGELOG.md # packages/go_router/pubspec.yaml

Piinks mentioned this pull request Jun 18, 2025

[go_router] Update sype safe routing topic to use mixin from go_router_builder 3.0.0 #9422

Merged

11 tasks

Merge branch 'main' into 138737-go-router-pop-scope

02142e3

Piinks reviewed Jun 18, 2025

View reviewed changes

packages/go_router/CHANGELOG.md Outdated Show resolved Hide resolved

Update packages/go_router/CHANGELOG.md

b2c0b51

Piinks reviewed Jun 18, 2025

View reviewed changes

packages/go_router/CHANGELOG.md Outdated Show resolved Hide resolved

Piinks added 2 commits June 18, 2025 17:39

Update packages/go_router/CHANGELOG.md

56a2575

Bump pubspec

d9bfa90

Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 18, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 18, 2025

Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 19, 2025

Piinks merged commit 8dad1d6 into flutter:main Jun 19, 2025
78 checks passed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 19, 2025

8dad1d6 [go_router] fix: PopScope.onPopInvokedWithResult not called i…

ebc718d

…n branch routes (flutter/packages#9245)

engine-flutter-autoroll mentioned this pull request Jun 19, 2025

Roll Packages from 715a0a5c4fdd to ca07b70466b0 (7 revisions) flutter/flutter#170876

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 19, 2025

8dad1d6 [go_router] fix: PopScope.onPopInvokedWithResult not called i…

d781896

…n branch routes (flutter/packages#9245)

engine-flutter-autoroll mentioned this pull request Jun 19, 2025

Roll Packages from 715a0a5c4fdd to 0ec405358e43 (8 revisions) flutter/flutter#170879

Merged

Ortes pushed a commit to Ortes/packages that referenced this pull request Jun 25, 2025

[go_router] fix: PopScope.onPopInvokedWithResult not called in branch…

d859c86

… routes (flutter#9245)

vashworth pushed a commit to vashworth/packages that referenced this pull request Jul 30, 2025

[go_router] fix: PopScope.onPopInvokedWithResult not called in branch…

13ae085

… routes (flutter#9245)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

[go_router] fix: PopScope.onPopInvokedWithResult not called in branch routes#9245

[go_router] fix: PopScope.onPopInvokedWithResult not called in branch routes#9245
Piinks merged 16 commits intoflutter:mainfrom
Gustl22:138737-go-router-pop-scope

Gustl22 commented May 13, 2025 •

edited

Loading

Uh oh!

chunhtai left a comment

Uh oh!

chunhtai May 16, 2025

Uh oh!

Gustl22 May 19, 2025

Uh oh!

chunhtai left a comment

Uh oh!

hannah-hyj left a comment

Uh oh!

Gustl22 commented Jun 9, 2025

Uh oh!

Gustl22 commented Jun 18, 2025

Uh oh!

Piinks commented Jun 18, 2025

Uh oh!

Uh oh!

Uh oh!

auto-submit bot commented Jun 18, 2025

Uh oh!

Uh oh!

Gustl22 commented Jun 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

Conversation

Gustl22 commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-Review Checklist

Footnotes

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

chunhtai May 16, 2025

Choose a reason for hiding this comment

Uh oh!

Gustl22 May 19, 2025

Choose a reason for hiding this comment

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

hannah-hyj left a comment

Choose a reason for hiding this comment

Uh oh!

Gustl22 commented Jun 9, 2025

Uh oh!

Gustl22 commented Jun 18, 2025

Uh oh!

Piinks commented Jun 18, 2025

Uh oh!

Uh oh!

Uh oh!

auto-submit bot commented Jun 18, 2025

Uh oh!

Uh oh!

Gustl22 commented Jun 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Gustl22 commented May 13, 2025 •

edited

Loading