packages icon indicating copy to clipboard operation
packages copied to clipboard

[go_router] Add support for relative routes

Open ThangVuNguyenViet opened this issue 1 year ago • 4 comments

Add supports for relative routes by allowing going to a path relatively, like go('./$path')

This PR doesn't fully resolve any issue, but it's mandatory to further add examples & tests for TypedRelativeGoRoute (see #7174), which will resolves #108177

Pre-launch Checklist

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

ThangVuNguyenViet avatar May 29 '24 08:05 ThangVuNguyenViet

@chunhtai I separated the PR as suggested. Please let me know the next steps It was my project's need for the goRelative that I created this PR, but it's also tempted to pushRelative along with other imperative APIs. However, that'll result in a new set of APIs (goRelative, pushRelative, pushReplacementRelative , etc). Alternatively we can do make a helper function that helps user construct the relativeRoute, like context.go(context.relativeRoute('settings'). Wdyt?

ThangVuNguyenViet avatar May 31 '24 13:05 ThangVuNguyenViet

Why don't you add it to the existing api.

context.go(`./settings`)

is pretty common in router modules in other frameworks as well as in bash

cedvdb avatar Jun 04 '24 21:06 cedvdb

Why don't you add it to the existing api.


context.go(`./settings`)

is pretty common in router modules in other frameworks as well as in bash

Oh yea that's cool. I didn't know of that pattern

ThangVuNguyenViet avatar Jun 05 '24 10:06 ThangVuNguyenViet

@thangmoxielabs This branch has conflicts that must be resolved

cedvdb avatar Oct 20 '24 21:10 cedvdb

@ThangVuNguyenViet This branch has conflicts that must be resolved @chunhtai @hannah-hyj is there any chance to fix conflicts and merge this very important feature?(How can I help?)

vasilich6107 avatar Nov 02 '24 18:11 vasilich6107

@ThangVuNguyenViet This branch has conflicts that must be resolved @chunhtai @hannah-hyj is there any chance to fix conflicts and merge this very important feature?(How can I help?)

You can fork and redo a pr

cedvdb avatar Nov 02 '24 23:11 cedvdb

@cedvdb hey sorry for the late reply. I didn't see any notification

thangmoxielabs avatar Nov 03 '24 09:11 thangmoxielabs

@chunhtai @hannah-hyj Could you merge this PR and release

vasilich6107 avatar Nov 07 '24 11:11 vasilich6107

This somehow slip through the crack. @thangmoxielabs can you fix the conflict again? sorry about the delay

chunhtai avatar Nov 07 '24 17:11 chunhtai

@chunhtai resolved it

ThangVuNguyenViet avatar Nov 07 '24 18:11 ThangVuNguyenViet

Hi @ThangVuNguyenViet you also need to bump the package.yaml version

chunhtai avatar Nov 07 '24 19:11 chunhtai

Hey @chunhtai it's bumped

ThangVuNguyenViet avatar Nov 08 '24 12:11 ThangVuNguyenViet

Hi @chunhtai go_router was again updated 5 hours ago Is there any chance to merge this PR before all other?

vasilich6107 avatar Nov 12 '24 22:11 vasilich6107

@chunhtai @ThangVuNguyenViet strange behaviour occurs when using context.push instead of context.go

Example: /profile/referral/select-method/withdraw

When using

context.go('./select-method');
context.go('./withdraw');

it's okay.

When using

context.push('./select-method');
context.push('./withdraw');

we got "GoException: no routes for location /profile/referral/withdraw"

subzero911 avatar Feb 13 '25 05:02 subzero911