Uh oh!
There was an error while loading. Please reload this page.
Handle automatic PopScope - #9856
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request addresses a test failure related to PopScope behavior by introducing a single tester.pump() call in packages/go_router/test/go_router_test.dart. The change is located within a test that simulates multiple back button presses and ensures that PopScope widgets have a frame to rebuild their state between gestures. The change is well-contained and the accompanying comment clearly explains its purpose. I find the change to be correct and have no further recommendations.
1 task
justinmcforce-pushed
the
automatic-pop-scope-fix
branch
3 times, most recently
from
August 20, 2025 16:54
bc8c463 to
fa5a567CompareNeeds one frame for PopScope to rerender after the navigation state changes.
justinmcforce-pushed
the
automatic-pop-scope-fix
branch
from
August 20, 2025 16:55
fa5a567 to
7af232bComparechunhtai
approved these changes
Aug 20, 2025
chunhtai
left a comment
Contributor
There was a problem hiding this comment.
nice detective work, LGTM
Uh oh!
There was an error while loading. Please reload this page.
engine-flutter-autoroll added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Aug 22, 2025
engine-flutter-autoroll added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Aug 22, 2025
github-merge-queueBot
pushed a commit
to flutter/flutter
that referenced
this pull request
Aug 22, 2025
flutter/packages@58c02e0...092d832 2025-08-21 engine-flutter-autoroll@skia.org Roll Flutter from 960d107 to d2ac021 (12 revisions) (flutter/packages#9866) 2025-08-21 jmccandless@google.com Handle automatic PopScope (flutter/packages#9856) 2025-08-20 engine-flutter-autoroll@skia.org Manual roll Flutter from e65380a to 960d107 (36 revisions) (flutter/packages#9862) 2025-08-20 10687576+bparrishMines@users.noreply.github.com [interactive_media_ads] Updates ProxyApis to prepare to add support for `AdEvent.ad` (flutter/packages#9785) 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
mboetger pushed a commit
to mboetger/flutter
that referenced
this pull request
Sep 18, 2025
flutter/packages@58c02e0...092d832 2025-08-21 engine-flutter-autoroll@skia.org Roll Flutter from 960d107 to d2ac021 (12 revisions) (flutter/packages#9866) 2025-08-21 jmccandless@google.com Handle automatic PopScope (flutter/packages#9856) 2025-08-20 engine-flutter-autoroll@skia.org Manual roll Flutter from e65380a to 960d107 (36 revisions) (flutter/packages#9862) 2025-08-20 10687576+bparrishMines@users.noreply.github.com [interactive_media_ads] Updates ProxyApis to prepare to add support for `AdEvent.ad` (flutter/packages#9785) 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
korca0220 pushed a commit
to korca0220/flutter
that referenced
this pull request
Sep 22, 2025
flutter/packages@58c02e0...092d832 2025-08-21 engine-flutter-autoroll@skia.org Roll Flutter from 960d107 to d2ac021 (12 revisions) (flutter/packages#9866) 2025-08-21 jmccandless@google.com Handle automatic PopScope (flutter/packages#9856) 2025-08-20 engine-flutter-autoroll@skia.org Manual roll Flutter from e65380a to 960d107 (36 revisions) (flutter/packages#9862) 2025-08-20 10687576+bparrishMines@users.noreply.github.com [interactive_media_ads] Updates ProxyApis to prepare to add support for `AdEvent.ad` (flutter/packages#9785) 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
Jaineel-Mamtora pushed a commit
to Jaineel-Mamtora/flutter_forked
that referenced
this pull request
Sep 24, 2025
flutter/packages@58c02e0...092d832 2025-08-21 engine-flutter-autoroll@skia.org Roll Flutter from 960d107 to d2ac021 (12 revisions) (flutter/packages#9866) 2025-08-21 jmccandless@google.com Handle automatic PopScope (flutter/packages#9856) 2025-08-20 engine-flutter-autoroll@skia.org Manual roll Flutter from e65380a to 960d107 (36 revisions) (flutter/packages#9862) 2025-08-20 10687576+bparrishMines@users.noreply.github.com [interactive_media_ads] Updates ProxyApis to prepare to add support for `AdEvent.ad` (flutter/packages#9785) 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
lucaantonelli pushed a commit
to lucaantonelli/flutter
that referenced
this pull request
Nov 21, 2025
flutter/packages@58c02e0...092d832 2025-08-21 engine-flutter-autoroll@skia.org Roll Flutter from 960d107 to d2ac021 (12 revisions) (flutter/packages#9866) 2025-08-21 jmccandless@google.com Handle automatic PopScope (flutter/packages#9856) 2025-08-20 engine-flutter-autoroll@skia.org Manual roll Flutter from e65380a to 960d107 (36 revisions) (flutter/packages#9862) 2025-08-20 10687576+bparrishMines@users.noreply.github.com [interactive_media_ads] Updates ProxyApis to prepare to add support for `AdEvent.ad` (flutter/packages#9785) 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
creatorpiyush pushed a commit
to creatorpiyush/packages
that referenced
this pull request
Jun 10, 2026
This test was failing in flutter/flutter#152330. That PR adds a PopScope inside of nested Navigators. When a back gesture happens, the PopScope takes 1 frame to rerender and update the state of its pop handling. The test was breaking because it performed two back gestures in one frame. Putting a pump in between them fixes it when the PR is merged (and works fine without the PR too). It took me so many hours to realize this one little pump was the fix I needed 😭 .
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This test was failing in flutter/flutter#152330. That PR adds a PopScope inside of nested Navigators. When a back gesture happens, the PopScope takes 1 frame to rerender and update the state of its pop handling. The test was breaking because it performed two back gestures in one frame. Putting a pump in between them fixes it when the PR is merged (and works fine without the PR too).
It took me so many hours to realize this one little pump was the fix I needed 😭 .