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.

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

Is nested path params supported? #128

Closed
coodoo opened this issue Nov 7, 2022 · 7 comments
Closed

Is nested path params supported? #128

coodoo opened this issue Nov 7, 2022 · 7 comments
Labels
📘 Docs Issue with documentation

Comments

@coodoo
Copy link

coodoo commented Nov 7, 2022

Trying to simulate the URL structure like github.com I've got these routes:

const routes = {
	'/': () => <Home />,
	'/user': () => <User />,
	'/user/:repo': () => <Repo />,
	'/user/:repo/issues': () => <Issues />,
	'/user/:repo/issues/:issue_id': () => <Issue />,
}

I expect <Issue /> shall be invoked and shown on screen when I visit :

http://localhost:5173/user/repo_foobar/issues/222

and I could access issue_id (which should be 222) like this:

const [path, pairs] = usePathParams('/user/:repo/issues/:issue_id')

But for some reasons <Issue /> never showed on screen.

Did I miss something in the route config?

@coodoo
Copy link
Author

coodoo commented Nov 7, 2022

After further tinkering around to find out the root cause is issue_id, changing it to issueId then everything worked like a charm. Now wondering why underscore or dash isn't supported as valid variable name in routes?

PS. Please feel free to close this issue since the problem is solved now.

@kyeotic
Copy link
Owner

kyeotic commented Nov 7, 2022

Now wondering why underscore or dash isn't supported as valid variable name in routes?

Not much is, its just this regex: /:[a-zA-Z]+/. I'm opening to changing these if you have a good reason, but if not then I should at least document this somewhere. I will leave this open in the meantime/

@kyeotic kyeotic added the 📘 Docs Issue with documentation label Nov 7, 2022
@coodoo
Copy link
Author

coodoo commented Nov 7, 2022

One good reason to support them both being following characters are all legal and supported in an URL?

; , / ? : @ & = + $ - _ . ! ~ * ' ( ) #

source

@n33pm
Copy link
Contributor

n33pm commented Nov 7, 2022

These are only legal and supported URL characters but there is no need do support these characters in the route matcher.

In my humble opinion keep it simple. each additional character ( route matcher ) can lead to unexpected problems.

@kyeotic
Copy link
Owner

kyeotic commented Nov 7, 2022

I'm with @n33pm, I don't see a good reason to match all valid characters, especially characters that cannot be valid JS variable characters, since that's ultimately what this route matcher produces (I know you could use quotes but that seems clunky to me).

For the pythonista's I can see why having underscore would be nice. Unfortunately changes to this part of the code constitute a breaking change for the library, so I plan on doing this exactly once. So, I will open a new issue for it where it can be discussed more visibly.

@coodoo
Copy link
Author

coodoo commented Nov 8, 2022

Actually I agree with everyone here on keeping it simple is good, also not supporting _ is really not that of a big deal, just make sure to note that in the doc so future users know ahead of time! 🙌

@kyeotic
Copy link
Owner

kyeotic commented Aug 18, 2023

I've changed my mind on this and plan to add support for more URL characters.

@kyeotic kyeotic closed this as completed Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📘 Docs Issue with documentation
Projects
None yet
Development

No branches or pull requests

3 participants