[go_router] Use the correct configuration to build the state passed to the onExit
#6623
Conversation
| @@ -233,7 +233,7 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList> | |||
| exitingMatches.length - 1, | |||
| context: navigatorContext, | |||
| matches: exitingMatches, | |||
| configuration: configuration, | |||
| // configuration: configuration, | |||
There was a problem hiding this comment.
Oh my ! Sorry about that. I removed it in refactor: Remove commented code
| ); | ||
| } | ||
| return SynchronousFuture<bool>(false); | ||
| } | ||
|
|
||
| final FutureOr<bool> exitFuture = goRoute.onExit!( | ||
| context, | ||
| match.buildState(_configuration, configuration), | ||
| match.buildState(_configuration, currentConfiguration), |
There was a problem hiding this comment.
so the difference is whether we use the new RouteMatchList after the change vs old RouteMatchList?
I think for redirect we use the new RouteMatchList. it seems to previous implementation is more inline with the redirect API no?
There was a problem hiding this comment.
I maybe wrong here. do you have use case that using the old routematchList is more preferred? on the other hand, I plan to redesign redirect, so maybe we should just provide both before and after?
There was a problem hiding this comment.
Yes, I found one in #6614. I was making go_router_builder use onExit in some tests failed https://github.com/flutter/packages/pull/6614/checks?check_run_id=24284539541.
The issue was when using .go(). The onExit had the state of the new page, not the previous page.
The test I wrote in this PR
'It should provide the correct path parameters to the onExit callback during a go'fails without my changes.
On the other hand, I plan to redesign redirect, so maybe we should just provide both before and after?
Sure, do you want me to change something then?
There was a problem hiding this comment.
I will need to redesign the onExit in the future. It also has issue with popscope and cupertinobackswipe. Thing may become a lot different after I resolve all these issues.
After thinking about it more, I think we should just keep it simple for now. I think what you have is fine.
…ct-configuration-in-on-exit # Conflicts: # packages/go_router/CHANGELOG.md
…ct-configuration-in-on-exit # Conflicts: # packages/go_router/CHANGELOG.md # packages/go_router/pubspec.yaml
|
@hangyujin Is there something I need to do about this PR ? |
hangyujin
added
the
autosubmit
…e passed to the `onExit` (flutter/packages#6623)
flutter/packages@1412041...fd714bd 2024-05-14 32538273+ValentinVignal@users.noreply.github.com [go_router] Use the correct configuration to build the state passed to the `onExit` (flutter/packages#6623) 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,rmistry@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
While working on #6614, I notice some issues with the state that is given to the
onExit(see the run https://github.com/flutter/packages/pull/6614/checks?check_run_id=24284539541)This PR uses the correct configuration to build the state and pass it to the
onExitPre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.