[go_router_builder]: Add support for relative routes#1
Conversation
LukasMirbt
force-pushed
the
go-router/add-relative-go-route-data
branch
3 times, most recently
from
d59d222 to
f778d2e
Compare
LukasMirbt
force-pushed
the
go-router-builder/add-support-for-relative-routes
branch
from
c280a01 to
a501098
Compare
chunhtai
left a comment
chunhtai
left a comment
There was a problem hiding this comment.
Can you also add a readme section for the goRelative route?
@gemini-code-assist Can you add a section to talk about how to use goRelative in the readme.md follow the same format as the existing content in the readme ?
| TypedRelativeGoRoute<DetailsRoute>( | ||
| path: 'details/:detailId', | ||
| routes: <TypedRoute<RouteData>>[ | ||
| TypedRelativeGoRoute<SettingsRoute>(path: 'settings/:settingId'), |
There was a problem hiding this comment.
What will happen if this is another TypedGoRoute? or should we assert or force the type of the routes to TypedRelativeGoRoute or TypedShellRoute?
There was a problem hiding this comment.
I added a build-time check in RouteBaseConfig._fromAnnotation. Do you agree with that approach?
Do you think it's necessary to add an assert in TypedRelativeGoRoute as well?
| // if there are any queryParam objects, add in the `queryParam` bits | ||
| String get _locationArgs { | ||
| final Map<String, String> pathParameters = Map<String, String>.fromEntries( | ||
| _pathParams.map((String pathParameter) { |
There was a problem hiding this comment.
can you factor this out so we don't duplicate the logic between this and goRoute version?
| return "'$location'"; | ||
| } | ||
|
|
||
| ParameterElement? get _extraParam => _ctor.parameters |
There was a problem hiding this comment.
same for these getter, or consider using mixin
|
oh looks like this is a pr in fork. Once you are ready, can you open a pr against main repo? we usually review both go_router and go_router_builder pr together, and only start merging once both are approved |
LukasMirbt
force-pushed
the
go-router/add-relative-go-route-data
branch
from
f778d2e to
e8c8f2b
Compare
LukasMirbt
force-pushed
the
go-router-builder/add-support-for-relative-routes
branch
from
a501098 to
ba35aa3
Compare
LukasMirbt
force-pushed
the
go-router-builder/add-support-for-relative-routes
branch
from
b139041 to
11e0b72
Compare
Reopened in flutter/packages (flutter#9749) and added a readme section. |
2 participants
Adds support for
TypedRelativeGoRoute.Builds on flutter#9732
Continuation of flutter#8476 by @ThangVuNguyenViet
Compared to flutter#8476, the approach taken in this PR avoids breaking changes and creates a separate RouteData subclass instead of extending GoRouteData. The approach increases flexibility and avoids extending behavior that might not make sense for a relative route.
Fully resolves flutter/flutter#108177.
Pre-Review Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2 ↩3