[go _route] fragment parameter added #8232

AffanShaikhsurab · 2024-12-05T02:50:48Z

Description of Change

This PR addresses the need for fragments/hashes to be treated as first-party parameters within the go_router package, as highlighted in issue #150155. Previously, users had to manually append the fragment to the URL, which could lead to potential bugs. With this update, the fragment is now a dedicated parameter, allowing for a more seamless and bug-free integration.

Before:

final location = context.namedLocation('some_route'); // const nested records
// Manually adding the fragment, hoping there aren't any weird bugs surrounding it
context.replace('$location#https://a.url/that?i=mightuse');

After:

// Directly passing the fragment as a parameter
context.goNamed("details", fragment: 'https://a.url/that?i=mightuse');

// or

final location = GoRouterState.of(context).namedLocation(
  'details',
  fragment: 'section3',
);
context.go(location);

// or 

final location = context.namedLocation(
 'details',
 fragment: 'section3',
);
context.go(location);

Issues Fixed

This PR resolves issue #150155.

Pre-launch Checklist

[✔️ ] I read the Contributor Guide and followed the process outlined there for submitting PRs.
[✔️ ] I read the Tree Hygiene 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 linked to 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, or this PR is exempt from CHANGELOG changes.
[✔️ ] 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.

hashes to URLs, whichcould lead to potential bugs. This update introduces a dedicated fragment parameter, allowing for more seamless and reliable URL navigation.

to remove the Linux repo_checks error.

AffanShaikhsurab · 2024-12-09T03:12:27Z

@chunhtai @hannah-hyj Can you please tell me what improvements I should make?

chunhtai · 2024-12-09T19:18:10Z

packages/go_router/lib/src/state.dart

    return _configuration.namedLocation(name,
-        pathParameters: pathParameters, queryParameters: queryParameters);
+            pathParameters: pathParameters, queryParameters: queryParameters) +
+        ((fragment?.isNotEmpty ?? false) ? '#$fragment' : '');


we should add fragment to namedLocation api as well

Sorry for overlooking that part . Added the the fragment handling to the namedLocation api.

This doesn't seem addressed yet

namedLocation api final location = context.namedLocation( 'details', fragment: 'section3', ); context.go(location);

packages/go_router/lib/src/router.dart

This fix ensures that the fragment is properly included when generating the URI using namedLocation in router.dart.

packages/go_router/test/go_route_test.dart

- Added logic to redirect to '/route/1' if the fragment is '1' in the '/route' route. - Updated test to verify correct redirection and ensure no infinite redirects occur. - Ensured proper navigation based on fragment values using router.goNamed(). - Added assertions to check for expected behavior and prevent exceptions during navigation.

AffanShaikhsurab · 2025-01-04T03:35:13Z

@chunhtai Could you please tell me what needs to be fixed?

chunhtai

I left a comment https://github.com/flutter/packages/pull/8232/files#r1911396301

- Modified `namedLocation` method to pass `fragment` directly to `_configuration.namedLocation`. - Simplifies handling of the fragment by removing manual concatenation logic.

chunhtai

LGTM

chunhtai · 2025-01-23T17:53:26Z

packages/go_router/CHANGELOG.md

@@ -1146,3 +1150,4 @@
 ## 0.1.0

 - squatting on the package name (I'm not too proud to admit it)
+


additional line?

chunhtai · 2025-01-23T17:53:52Z

packages/go_router/CHANGELOG.md

@@ -7,6 +11,7 @@
 - Updates minimum supported SDK version to Flutter 3.22/Dart 3.4.
 - Updates readme.

+


additional line?

hannah-hyj

LGTM

chunhtai · 2025-01-23T22:23:24Z

@hannah-hyj I think there are still some minor formatting issue of changelog

flutter/packages@3d3ab7b...258f6dc 2025-01-24 adsonpleal@gmail.com [shared_preferences] Add shared preferences devtool (flutter/packages#8494) 2025-01-24 tarrinneal@gmail.com [shared_preferences] update List<String> encode/decode (flutter/packages#8335) 2025-01-24 engine-flutter-autoroll@skia.org Manual roll Flutter from c1561a4 to c1ffaa9 (21 revisions) (flutter/packages#8498) 2025-01-24 stuartmorgan@google.com [ios_platform_images] Switch to `loadImage` (flutter/packages#8216) 2025-01-24 mchudy@users.noreply.github.com [camera] Remove OCMock from CameraExposureTests and CameraFocusTests (flutter/packages#8351) 2025-01-24 tarrinneal@gmail.com [shared_preferences] Tool for migrating from legacy shared_preferences to shared_preferences_async (flutter/packages#8229) 2025-01-23 stuartmorgan@google.com Revert "[shared_preferences] Add shared preferences devtool" (flutter/packages#8493) 2025-01-23 20254485+kaboc@users.noreply.github.com [go_router] Fix return type of current state getter to be non-nullable (flutter/packages#8173) 2025-01-23 engine-flutter-autoroll@skia.org Manual roll Flutter from b2f515f to c1561a4 (18 revisions) (flutter/packages#8491) 2025-01-23 tarrinneal@gmail.com [pigeon] fixes event channel dart instance name usage and adds test (flutter/packages#8483) 2025-01-23 51104750+AffanShaikhsurab@users.noreply.github.com [go _route] fragment parameter added (flutter/packages#8232) 2025-01-23 mchudy@users.noreply.github.com [in_app_purchase] Update in_app_purchase_android version in in_app_purchase (flutter/packages#8463) 2025-01-23 stuartmorgan@google.com [image_picker] Reference alternate macOS implementations (flutter/packages#8487) 2025-01-23 32538273+ValentinVignal@users.noreply.github.com [rfw] Activate leak testing (flutter/packages#8370) 2025-01-23 32538273+ValentinVignal@users.noreply.github.com [video_player] Activate leak testing (flutter/packages#8379) 2025-01-23 engine-flutter-autoroll@skia.org Manual roll Flutter from b9e86a5 to b2f515f (42 revisions) (flutter/packages#8482) 2025-01-23 olli.helenius@codemate.com [camera] Add API support query for image streaming (app-facing) (flutter/packages#8422) 2025-01-23 engine-flutter-autoroll@skia.org Manual roll Flutter from b9e86a5 to eb6af3d (13 revisions) (flutter/packages#8473) 2025-01-23 adsonpleal@gmail.com [shared_preferences] Add shared preferences devtool (flutter/packages#8322) 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

### Description of Change This PR addresses the need for fragments/hashes to be treated as first-party parameters within the `go_router` package, as highlighted in [issue #150155](flutter/flutter#150155). Previously, users had to manually append the fragment to the URL, which could lead to potential bugs. With this update, the `fragment` is now a dedicated parameter, allowing for a more seamless and bug-free integration. #### Before: ```dart final location = context.namedLocation('some_route'); // const nested records // Manually adding the fragment, hoping there aren't any weird bugs surrounding it context.replace('$location#https://a.url/that?i=mightuse'); ``` #### After: ```dart // Directly passing the fragment as a parameter context.goNamed("details", fragment: 'https://a.url/that?i=mightuse'); // or final location = GoRouterState.of(context).namedLocation( 'details', fragment: 'section3', ); context.go(location); // or final location = context.namedLocation( 'details', fragment: 'section3', ); context.go(location); ``` ### Issues Fixed This PR resolves [issue #150155](flutter/flutter#150155). --- ## Pre-launch Checklist - [✔️ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [✔️ ] I read the [Tree Hygiene] 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`.) - [x] 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 [linked to 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], or this PR is [exempt from CHANGELOG changes]. - [✔️ ] 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].  [Contributor Guide]: https://github.com/flutter/packages/blob/main/CONTRIBUTING.md [Tree Hygiene]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md [relevant style guides]: https://github.com/flutter/packages/blob/main/CONTRIBUTING.md#style [CLA]: https://cla.developers.google.com/ [Discord]: https://github.com/flutter/flutter/blob/master/docs/contributing/Chat.md [linked to at least one issue that this PR fixes]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#overview [pub versioning philosophy]: https://dart.dev/tools/pub/versioning [exempt from version changes]: https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#version [following repository CHANGELOG style]: https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changelog-style [exempt from CHANGELOG changes]: https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changelog [test-exempt]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#tests

Previously, users had to manually append fragments

cc3ac4f

hashes to URLs, whichcould lead to potential bugs. This update introduces a dedicated fragment parameter, allowing for more seamless and reliable URL navigation.

AffanShaikhsurab requested a review from chunhtai as a code owner December 5, 2024 02:50

github-actions bot added the p: go_router label Dec 5, 2024

AffanShaikhsurab changed the title ~~Previously, users had to manually append fragments~~ [go _route] fragment parameter added Dec 5, 2024

extensions.dart file formatted using dart format

7a641b4

to remove the Linux repo_checks error.

chunhtai requested a review from hannah-hyj December 5, 2024 22:40

Merge branch 'main' into main

c740df8

chunhtai reviewed Dec 9, 2024

View reviewed changes

AffanShaikhsurab and others added 3 commits December 10, 2024 07:32

Added the the fragment handling to the

2a0cf73

namedLocation api final location = context.namedLocation( 'details', fragment: 'section3', ); context.go(location);

Merge branch 'main' of https://github.com/AffanShaikhsurab/packages

851905b

Merge branch 'flutter:main' into main

29d8f6c

AffanShaikhsurab requested a review from chunhtai December 10, 2024 02:22

AffanShaikhsurab added 2 commits December 12, 2024 07:46

Merge branch 'main' into main

3f3adcb

Merge branch 'main' into main

b4c2efc

chunhtai reviewed Dec 12, 2024

View reviewed changes

packages/go_router/lib/src/router.dart Outdated Show resolved Hide resolved

AffanShaikhsurab and others added 4 commits December 17, 2024 09:40

Merge branch 'main' into main

080a38e

Fix: Include fragment in URI generated by GoRouter

ebb8cdc

This fix ensures that the fragment is properly included when generating the URI using namedLocation in router.dart.

Merge branch 'main' of https://github.com/AffanShaikhsurab/packages

b6cfe44

updated changeLog

62b6027

AffanShaikhsurab requested a review from chunhtai December 17, 2024 06:22

chunhtai reviewed Dec 17, 2024

View reviewed changes

packages/go_router/test/go_route_test.dart Outdated Show resolved Hide resolved

packages/go_router/test/go_route_test.dart Outdated Show resolved Hide resolved

AffanShaikhsurab and others added 5 commits December 18, 2024 09:36

Merge branch 'main' into main

58a1b12

dart format

0816efd

Merge branch 'main' of https://github.com/AffanShaikhsurab/packages

32555fc

added anotation

0fee133

AffanShaikhsurab requested a review from chunhtai December 18, 2024 04:48

AffanShaikhsurab added 2 commits December 19, 2024 09:26

Merge branch 'main' into main

8dec200

Merge branch 'main' into main

ea4d752

AffanShaikhsurab added 4 commits December 28, 2024 19:52

Merge branch 'main' into main

362e23e

Merge branch 'main' into main

fcb1b91

Merge branch 'main' into main

1bcd8b3

Merge branch 'main' into main

0b507b0

chunhtai reviewed Jan 10, 2025

View reviewed changes

AffanShaikhsurab and others added 4 commits January 11, 2025 13:14

Refactor: Update namedLocation method to include fragment parameter

6651f6c

- Modified `namedLocation` method to pass `fragment` directly to `_configuration.namedLocation`. - Simplifies handling of the fragment by removing manual concatenation logic.

Merge branch 'main' of https://github.com/AffanShaikhsurab/packages

1ef08a4

Delete packages/go_router/log.txt

b6546a4

Merge branch 'main' into main

aed6d08

AffanShaikhsurab requested a review from chunhtai January 11, 2025 08:00

AffanShaikhsurab added 2 commits January 17, 2025 08:37

Merge branch 'main' into main

a45bb0d

Merge branch 'main' into main

feae635

chunhtai approved these changes Jan 23, 2025

View reviewed changes

hannah-hyj approved these changes Jan 23, 2025

View reviewed changes

hannah-hyj merged commit ac08525 into flutter:main Jan 23, 2025
77 checks passed

engine-flutter-autoroll mentioned this pull request Jan 24, 2025

Roll Packages from 3d3ab7bcb07e to 73857c68c1ce (15 revisions) flutter/flutter#162160

Closed

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

ac0852545 [go _route] fragment parameter added (flutter/packages#8232)

fea04d2

engine-flutter-autoroll mentioned this pull request Jan 24, 2025

Roll Packages from 3d3ab7bcb07e to 73857c68c1ce (15 revisions) flutter/flutter#162169

Closed

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

ac0852545 [go _route] fragment parameter added (flutter/packages#8232)

bbbce6a

engine-flutter-autoroll mentioned this pull request Jan 24, 2025

Roll Packages from 3d3ab7bcb07e to 1833b326a1d0 (16 revisions) flutter/flutter#162175

Closed

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

ac0852545 [go _route] fragment parameter added (flutter/packages#8232)

48cac8e

engine-flutter-autoroll mentioned this pull request Jan 27, 2025

Roll Packages from 3d3ab7bcb07e to 258f6dce6a49 (19 revisions) flutter/flutter#162254

Merged

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

ac0852545 [go _route] fragment parameter added (flutter/packages#8232)

669593d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[go _route] fragment parameter added #8232

[go _route] fragment parameter added #8232

Uh oh!

AffanShaikhsurab commented Dec 5, 2024 •

edited

Loading

Uh oh!

AffanShaikhsurab commented Dec 9, 2024

Uh oh!

chunhtai Dec 9, 2024

Uh oh!

AffanShaikhsurab Dec 10, 2024

Uh oh!

chunhtai Jan 10, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AffanShaikhsurab commented Jan 4, 2025

Uh oh!

chunhtai left a comment

Uh oh!

chunhtai left a comment

Uh oh!

chunhtai Jan 23, 2025

Uh oh!

chunhtai Jan 23, 2025

Uh oh!

hannah-hyj left a comment

Uh oh!

Uh oh!

chunhtai commented Jan 23, 2025

Uh oh!

Uh oh!

		@@ -1146,3 +1150,4 @@
		## 0.1.0

		- squatting on the package name (I'm not too proud to admit it)

		@@ -7,6 +11,7 @@
		- Updates minimum supported SDK version to Flutter 3.22/Dart 3.4.
		- Updates readme.

[go _route] fragment parameter added #8232

[go _route] fragment parameter added #8232

Uh oh!

Conversation

AffanShaikhsurab commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Change

Before:

After:

Issues Fixed

Pre-launch Checklist

Uh oh!

AffanShaikhsurab commented Dec 9, 2024

Uh oh!

chunhtai Dec 9, 2024

Choose a reason for hiding this comment

Uh oh!

AffanShaikhsurab Dec 10, 2024

Choose a reason for hiding this comment

Uh oh!

chunhtai Jan 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AffanShaikhsurab commented Jan 4, 2025

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

chunhtai Jan 23, 2025

Choose a reason for hiding this comment

Uh oh!

chunhtai Jan 23, 2025

Choose a reason for hiding this comment

Uh oh!

hannah-hyj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chunhtai commented Jan 23, 2025

Uh oh!

Uh oh!

AffanShaikhsurab commented Dec 5, 2024 •

edited

Loading