[go_router] Retain query parameter during refresh and first redirect#5276
Conversation
| WidgetsBinding.instance.platformDispatcher.defaultRouteName, | ||
| ); | ||
| if (platformDefaultUri.hasEmptyPath) { | ||
| platformDefaultUri = Uri( |
There was a problem hiding this comment.
this can be platformDefaultUri.replace(path: '/')
There was a problem hiding this comment.
That doesn't do the same.
For example, if platformDefaultUri is https://domain.com?param=1, my current implementation creates the Uri /?param=1, while using Uril.replace(path: '/') creates https://domain.com/?param=1. This breaks a handful of tests, and I think go_router expects /?param=1 rather than https://domain.com/?param=1?
Maybe I could change those lines into:
Uri platformDefaultUri = Uri.parse(
WidgetsBinding.instance.platformDispatcher.defaultRouteName,
);
platformDefaultUri = Uri(
path: platformDefaultUri.hasEmptyPath ? '/' platformDefaultUri.path,
queryParameters: platformDefaultUri.queryParameters,
);
final String platformDefault = platformDefaultUri.toString();What do you think ?
There was a problem hiding this comment.
this will be feed into RouteInformation.location so I think you are right, we shouldn't include the scheme and host. Can you add a todo here to remind us that this can be clean up once RouteInformation.uri is available in packages repo?
There was a problem hiding this comment.
I added the todo in doc: Add todo
I put your username. Tell if I should put another one
| WidgetsBinding.instance.platformDispatcher.defaultRouteName, | ||
| ); | ||
| if (platformDefaultUri.hasEmptyPath) { | ||
| platformDefaultUri = Uri( |
There was a problem hiding this comment.
this will be feed into RouteInformation.location so I think you are right, we shouldn't include the scheme and host. Can you add a todo here to remind us that this can be clean up once RouteInformation.uri is available in packages repo?
…y-parameters-on-refresh # Conflicts: # packages/go_router/CHANGELOG.md # packages/go_router/pubspec.yaml
…y-parameters-on-refresh # Conflicts: # packages/go_router/CHANGELOG.md # packages/go_router/pubspec.yaml
hannah-hyj
added
the
autosubmit
auto-submit
bot
removed
the
autosubmit
|
auto label is removed for flutter/packages/5276, due to - The status or check suite Mac_arm64 ios_platform_tests_shard_4 master has failed. Please fix the issues identified (or deflake) before re-applying this label. |
|
@chunhtai @Hangyujin it looks like the Mac_arm64 ios_platform_tests_shard_4 master was cancelled. Is it an issue from my changes? |
chunhtai
added
the
autosubmit
flutter/packages@b69f54e...a682189 2023-11-09 [email protected] Roll Flutter from 4b4a1fe to f662150 (17 revisions) (flutter/packages#5361) 2023-11-09 [email protected] [go_router] Retain query parameter during refresh and first redirect (flutter/packages#5276) 2023-11-09 [email protected] Use specific version of mac_toolchain (flutter/packages#5356) 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],[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
3 participants
Fixes flutter/flutter#137513
Pre-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.