-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Feat/resolve route imp #11406
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
base: main
Are you sure you want to change the base?
Feat/resolve route imp #11406
Conversation
🦋 Changeset detectedLatest commit: 0fdec66 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@@ -2062,16 +2125,14 @@ declare module '$app/paths' { | |||
* ); // `/blog/hello-world/something/else` | |||
* ``` | |||
* */ | |||
export function resolveRoute(id: string, params: Record<string, string | undefined>): string; | |||
export function resolveRoute<K extends RouteWithParams>(id: K, params: RouteIds[K]): string; | |||
export function resolveRoute<K extends RouteWithoutParams>(id: K): string; | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lint is not passing because I wrote the type here directly. It seems that we have to do it in JSDoc
, but I don't manage to do it.
I would need some guidance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I too am somewhat flummoxed by JSDoc in this case. For now I've put this stuff in a src .d.ts
file, which sort of works, though there are some other problems that need solving separately.
I took the liberty of moving the RouteId
type to $types
rather than $app/paths
— it's arguably more consistent with the various ./$types
files, and it'll be convenient to have a single place to put all the generated types.
Right now it doesn't seem like that ambient declaration is getting picked up — working on it...
This is awesome! |
Maybe @Rich-Harris you can share your initial thoughts here or in the issue? |
Here's a wrinkle: the generated types need to be in an ambient module declaration (per #11406 (comment), I changed this from I think that's probably fine, since the |
ok, this is working pretty well for me locally now — will stop fiddling about with the PR for a bit now that the types are being generated more or less as expected excited for this! not totally sure what parts 3, 4 and 5 will entail? |
I kinda liked a lot the export const match = (param: 'a' | 'b') => {
return ['a', 'b'].includes(param)
}
// or
export const match = (param: number) => {
return /^\d+$/.test(param.toString())
} Maybe reconsidering |
I'm not sure how it would — it's an ambient module declaration either way |
Oh... unless it wasn't an ambient module declaration, but was actually an alias instead? Hmm |
Fixes: #11386
id
typed withRouteId
params
typed with inferred params from the route (optionals, matcher)Current status

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpm changeset
and following the prompts. Changesets that add features should beminor
and those that fix bugs should bepatch
. Please prefix changeset messages withfeat:
,fix:
, orchore:
.