-
-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
Parse params in TypeScript #8030
Conversation
WHAT MAGIC IS THIS?! Haha, I've never seen anything like this. Very cool, @kddnewton. Thanks for bringing this to our attention. I could go either way with this. I think I understand what's going on in the code (thanks for the comments!) so I'd be fine maintaining it if people really want it. What do you think @chaance? I think we may also need to do a bit more work here to augment the return value of |
Would be also nice to have exported Params type, that could look like type Params<Path extends string> = Record<ParseParamKey<Path>, string> so that we can use it with useParams() hook, like import { useParams } from 'react-router-dom'
import type { Params } from 'react-router-dom'
function UserPost() {
const { userId, postId } = useParams() as Params<'/users/:userId/posts/:postId'>
} |
Oh, I've just realized, that it's not needed, because of the latest changes to useParams hook. So, just exported ParseParamKey type will be enough. import { useParams } from 'react-router-dom'
import type { ParseParamKey } from 'react-router-dom'
function UserPost() {
const { userId, postId } = useParams<ParseParamKey<'/users/:userId/posts/:postId'>>()
} |
@mjackson love this, I actually wanted to do something similar but didn't spend the time on trying to figure out the patterns, but I'm very glad @kddnewton did. I say we do it! |
@mjackson I updated Yeah, template strings are ridiculous. I'm gonna leave some links here in case people find this issue and get curious:
|
@kddnewton Meant to echo Michael on this, but thanks so much for the inline comments. This kind of stuff is very helpful for us and anyone who contributes after us 🙏 |
@mjackson let me know if there's anything else you'd like me to do for this to go out, and thanks for being open to it! |
could you please consider exporting ParamParseKey type |
Sure @smashercosmo I've added it as exported. I wasn't sure if maintenance of that public type was preferable, but I suppose you could derive it anyway. |
Thank you) though we should've probably waited for @mjackson response. There is chance that he might not want to make it public. But as I said before it will be very convenient to use it with useParams hook import { useParams } from 'react-router-dom'
import type { ParseParamKey } from 'react-router-dom'
function UserPost() {
const { userId, postId } = useParams<ParseParamKey<'/users/:userId/posts/:postId'>>()
} |
Just rebased in case y'all get a minute to check this out. <3 |
Pinging @mjackson on this but I'd prefer we didn't export that type. It's more of an internal utility and I could easily see us needing to change or remove it before v7 if TypeScript releases a new feature we can leverage to optimize it further. |
No problem @chaance, I've unexported it now and rebased. |
I saw #8019 go out, which is great! useMatch should definitely be generic to make the types better. One thing that's nice though is that modern TypeScript supports template strings, so these types can actually be parsed statically if they're string literals. This commit makes it attempt to parse the type statically and if not, it falls back to just string. You can play around with the parsing here: https://www.typescriptlang.org/play?ts=4.4.2#code/C4TwDgpgBACghgJzgW3ggzhAYnAlgGwgBMoBeKAbyggQQHsEAuKYBAV2gF8BuAWACgBoSLEQo0mADwBlCAHNkEAHbBqAD2DKi6KOla4lcgHxkBUKAHoLUAMIALCAGMA1lAcJouAGYt30CGq4ejpwUF4MAO6IJOj4cOh2UAa+0HoIBnIAdGZQsgrKqgGaStpQAAYAJBQGXjRQADIQXsB5iiqcFlU1dQBKuHJ2LfJtwJxlOeZWUACSPsB+SSFhkdG6cQkANClKUHDAmshgqsB0UGCImNRwjonouETQdHMOE5bW4QhRCDHrdtn85nMAH5REhUBcIJJGs1WgUTEUtDpuggGk1gD0IOg2PhgK9gaDxBDJH0BkN8ip4RpEUklLUUSTBhisTi8YCQdD0ZjsYUqSUdGkMqzAW8Zs9oIRmrp7p5+WxHI5MegvNj8CAzhCSPFduqwVt5sooDcnK5vELAVN9VB0qSpQ9DXAdgAjVJyhXoJUqtXnDDEXY6CIQfD4TKiqCOujzW2Ys2TayOB1h6DezBEPUOHaWjzM1RBbVsJS4Og7J4pFgRU53B7oGMigAUHjgRGYACJwnRm1AAD5QZuOxDNgCU-2FI6gIIZnOz6mKpQFhjHqOaTO5XagE+XOKgzA5G9xANH5uss1LEtUlZlUCUEd0rsVyqDXo1fu13pQaYNRpcNam3igRdVpbWoMUb2k6Lryneno6pcWqhK+yAhseuB7geh6ge+GYOFaXKbrmABWbB6Fa-SDFsgaXMhADkSxeHg+BsB4w6oVua4kZOK4Inyuj6PO45sbuLFoISPo4AQxBmswQngiJdHifuwpTMelqniBRD3EolGqMmngqKccFiMgGFQARRGfq4mZsayUznkxwqSQZEiQhOsIUtO1LIqxpK7mafFeThPIzvyPFyAu67+YJDkQqJhBEBJBLSZg0VyQpR5ih41GXqcHxfD88R2EZZlJM8uaYOSZ7AIgwD+shiRwK8UyOHQ+BFohPjIVARB0JiRnzLmuahEQIBKCguCOLowwFFs6CnJm4VBPV1gmccWEeMgeAlHUJaWnOWRQAA8vqnxBBAWxUTRdEMRAtksS5AXUmUjBdLSvQQGtBgZGMoWvetGQRWCjlJUQfCCPwwjQFJ6AyBNKhuVxO0mKQORSY5UNlZSgXcekhgLaG23Q6o8ZgTeEHuveAHaSQBgnNqmCqCWO1QPgyE0HA+DoBsOP6ph0BZiuuZyHQv3U862prWAf7PHQlw7SEOjOBAIAhCUONgFLdyOgB+YPF4Bi+jLz6WgAbqzHDoNdIJoMAuCs8STgMEQkjI0St1GFNwVGEYONKVhpUjKBV6qCLWIkx6D7QcQRkBpRHjGYRqgC0LdA43YcCG9AoQC3QJDa3AK60UGfYuBLNPBVAAC0Jg7SCYvXfZCBWzbGKNd8kg7W7WPGEYwMCAEqv12E+aOFbRZQIREAALJ7DcMiw7OwVbDAs86BDMge7W8HoMw0gDpJlA5B4wAMTsFCcM+MDA5wAgCGPk-ADctatnQdAWH2CAv3AABeg7AzfU92A-bYLCMFfu-L+A4f6YFvvfR+z9gGICAX2MBECJ5-wfowQBcC36IO-tfSBqDmzoKfgg+BcCkECCAA.
176800e
to
53847bb
Compare
@kddnewton I fixed the merge conflicts, you wanna put your username in contributors.yaml to sign the CLA and I'll merge this puppy? |
Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳 |
@ryanflorence absolutely! |
When there are bugs, I'm pinging you cause I have no clue what's going on in there 😝 but the typescript sandbox was awesome! Too bad we can't do that for |
Hit me up anytime, happy to help! :) |
Formatting is fixed up for you too 👍 |
Thansk @timdorr, I've been over here trying to get github.dev to use the project prettier config but it won't! |
Yeah, Prettier doesn't work for me on github.dev either :/ |
Woohoo! Thanks for the merge! Excited about this. |
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Hi! 👋
I saw #8019 go out, which is great!
useMatch
should definitely be generic to make the types better. One thing that's nice though is that modern TypeScript supports template strings, so these types can actually be parsed statically if they're string literals. This commit makes it attempt to parse the type statically and if not, it falls back to just string. You can play around with the parsing here.Now when you use
useMatch("foo/:bar")
the type returned is actuallyPathMatch<"bar">
anduseMatch("foo/:bar/:baz")
isPathMatch<"bar" | "baz">
.Totally get it if you don't want to maintain this, but I think it's really nice on the consumer side because you don't have to duplicate that key.
Tagging @chaance since you made that original PR that inspired this one.