Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[go_router] fix context extension for replaceNamed #3927

Merged
merged 13 commits into from
May 19, 2023
Merged

[go_router] fix context extension for replaceNamed #3927

merged 13 commits into from
May 19, 2023

Conversation

Dreamersoul
Copy link
Contributor

@Dreamersoul Dreamersoul commented May 7, 2023 •
edited
Loading

The extension method is not passing all the data to the function, such as pathParams and queryParams, fixes: flutter/flutter#126222

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

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

@Dreamersoul
fix context extension for replaceNamed
b2c1548 
The extension method is not passing all the data to the function, such as pathParams and queryParams
@Dreamersoul Dreamersoul requested a review from chunhtai as a code owner May 7, 2023 08:33
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

chunhtai
chunhtai approved these changes May 8, 2023
View reviewed changes
Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

packages/go_router/test/extension_test.dart Outdated Show resolved Hide resolved
@chunhtai chunhtai requested a review from hannah-hyj May 9, 2023 16:29
hannah-hyj
hannah-hyj approved these changes May 9, 2023
View reviewed changes
Copy link
Member

@hannah-hyj hannah-hyj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chunhtai
Copy link
Contributor

Hi @Dreamersoul It looks like you need to bump up the pubspec.yaml version inorder to merge this pr

Dreamersoul and others added 2 commits May 13, 2023 14:19
@Dreamersoul @chunhtai
Update packages/go_router/CHANGELOG.md
d6a5db1 
Co-authored-by: chunhtai <47866232+chunhtai@users.noreply.github.com>
@stuartmorgan
Merge branch 'main' into patch-1
8f680d5
@stuartmorgan stuartmorgan added autosubmit Merge PR when tree becomes green via auto submit App and removed needs tests labels May 16, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 16, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented May 16, 2023

auto label is removed for flutter/packages, pr: 3927, due to - The status or check suite Mac_arm64 ios_platform_tests_shard_3 master has failed. Please fix the issues identified (or deflake) before re-applying this label.

@chunhtai chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label May 18, 2023
@auto-submit auto-submit bot merged commit 1dea5f9 into flutter:main May 19, 2023
@Dreamersoul Dreamersoul deleted the patch-1 branch May 19, 2023 20:35
@koji-1009 koji-1009 mentioned this pull request May 21, 2023
Closed
11 tasks
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 22, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 22, 2023
@engine-flutter-autoroll
1dea5f9 [go_router] fix context extension for replaceNamed (flutter/p…
baca958 
…ackages#3927)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request May 22, 2023
@engine-flutter-autoroll
Roll Packages from 1e214d7 to 83959fb (9 revisions) (#127322)
5b0615c 
flutter/packages@1e214d7...83959fb

2023-05-22 JeroenWeener@users.noreply.github.com [in_app_purchases] Fix mismatching method signature strings (flutter/packages#4040)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 engine-flutter-autoroll@skia.org Roll Flutter from 077d644 to ab57304 (18 revisions) (flutter/packages#4051)
2023-05-19 engine-flutter-autoroll@skia.org Roll Flutter from 5ae6438 to 077d644 (23 revisions) (flutter/packages#4043)
2023-05-19 stuartmorgan@google.com [local_auth] Migrate iOS to Pigeon (flutter/packages#3974)
2023-05-19 hamadyalghanim@gmail.com [go_router] fix context extension for replaceNamed (flutter/packages#3927)
2023-05-19 JeroenWeener@users.noreply.github.com [image_picker] Fix crash due to `SecurityException` (flutter/packages#4004)

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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
sybrands-place pushed a commit to sybrands-place/packages that referenced this pull request May 23, 2023
Merge branch 'main' into webview_patchedv5
a3ba9c3 
* main: (104 commits)
  [various] Remove unnecessary null checks (flutter#4060)
  [ci] Add a legacy Android build-all test (flutter#4005)
  Roll Flutter from ab57304 to 3437189 (5 revisions) (flutter#4062)
  [go_router] Nested stateful navigation with ShellRoute (flutter#2650)
  [go_router] Nested stateful navigation with ShellRoute (flutter#2650)
  [go_router] Nested stateful navigation with ShellRoute (flutter#2650)
  [in_app_purchases] Fix mismatching method signature strings (flutter#4040)
  [go_router] Nested stateful navigation with ShellRoute (flutter#2650)
  [go_router] Nested stateful navigation with ShellRoute (flutter#2650)
  [go_router] Nested stateful navigation with ShellRoute (flutter#2650)
  Roll Flutter from 077d644 to ab57304 (18 revisions) (flutter#4051)
  Roll Flutter from 5ae6438 to 077d644 (23 revisions) (flutter#4043)
  [local_auth] Migrate iOS to Pigeon (flutter#3974)
  [go_router] fix context extension for replaceNamed (flutter#3927)
  [image_picker] Fix crash due to `SecurityException` (flutter#4004)
  Roll Flutter from d0d1feb to 5ae6438 (42 revisions) (flutter#4038)
  [ci] Lower iOS LUCI timeouts (flutter#4035)
  [ci] Increase Android sharding (flutter#4029)
  [flutter_plugin_android_lifecycle] Fix lints (flutter#4030)
  [rfw] Fix a typo in the API documentation (flutter#4023)
  ...
CaseyHillers pushed a commit to CaseyHillers/flutter that referenced this pull request May 24, 2023
@engine-flutter-autoroll
Roll Packages from 1e214d7 to 83959fb (9 revisions) (flutter#127322)
8f54016 
flutter/packages@1e214d7...83959fb

2023-05-22 JeroenWeener@users.noreply.github.com [in_app_purchases] Fix mismatching method signature strings (flutter/packages#4040)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 engine-flutter-autoroll@skia.org Roll Flutter from 077d644 to ab57304 (18 revisions) (flutter/packages#4051)
2023-05-19 engine-flutter-autoroll@skia.org Roll Flutter from 5ae6438 to 077d644 (23 revisions) (flutter/packages#4043)
2023-05-19 stuartmorgan@google.com [local_auth] Migrate iOS to Pigeon (flutter/packages#3974)
2023-05-19 hamadyalghanim@gmail.com [go_router] fix context extension for replaceNamed (flutter/packages#3927)
2023-05-19 JeroenWeener@users.noreply.github.com [image_picker] Fix crash due to `SecurityException` (flutter/packages#4004)

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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
This was referenced Jul 7, 2023
Merged
Closed
Merged
nploi pushed a commit to nploi/packages that referenced this pull request Jul 16, 2023
@Dreamersoul @nploi
[go_router] fix context extension for replaceNamed (flutter#3927)
d882d45 
The extension method is not passing all the data to the function, such as pathParams and queryParams, fixes: flutter/flutter#126222
@reidbaker reidbaker mentioned this pull request Jul 26, 2023
Merged
11 tasks
@reidbaker reidbaker mentioned this pull request Aug 4, 2023
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chunhtai chunhtai chunhtai approved these changes

@hannah-hyj hannah-hyj hannah-hyj approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: go_router
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[go_router] context extension for replaceNamed missing parameters
4 participants
@Dreamersoul @chunhtai @hannah-hyj @stuartmorgan