-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat: typed route ids #13864
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: typed route ids #13864
Changes from 26 commits
74c0fd7
c95a69f
4fe5f0b
5669c42
3e6b274
df2293d
039fa0f
e2afa93
42ea4f8
26beba7
5865e34
e155d92
a483961
7987509
7cf2cfc
8c8c037
c6c3355
cfd16ed
c000bb4
945f9cc
9bd9a67
82d6695
6cd32d4
eb8b876
f6c4b35
c1987e0
96de94b
5a30a83
eac2a0e
5445a93
22b26dc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@sveltejs/kit': minor | ||
--- | ||
|
||
feat: better type-safety for `page.route.id`, `page.params`, page.url.pathname` and various other places |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@sveltejs/kit': minor | ||
--- | ||
|
||
feat: `resolve(...)` and `asset(...)` helpers for resolving paths |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@sveltejs/kit': minor | ||
--- | ||
|
||
feat: Add `$app/types` module with `Asset`, `RouteId`, `Pathname`, `ResolvedPathname` `RouteParams<T>` and `LayoutParams<T>` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
--- | ||
title: $app/types | ||
--- | ||
|
||
This module contains generated types for the routes in your app. | ||
|
||
```js | ||
// @noErrors | ||
import type { RouteId, RouteParams, LayoutParams } from '$app/types'; | ||
``` | ||
|
||
## Asset | ||
|
||
A union of all the filenames of assets contained in your `static` directory. | ||
|
||
<div class="ts-block"> | ||
|
||
```dts | ||
type Asset = '/favicon.png' | '/robots.txt'; | ||
``` | ||
|
||
</div> | ||
|
||
## RouteId | ||
|
||
A union of all the route IDs in your app. Used for `page.route.id` and `event.route.id`. | ||
|
||
<div class="ts-block"> | ||
|
||
```dts | ||
type RouteId = '/' | '/my-route' | '/my-other-route/[param]'; | ||
``` | ||
|
||
</div> | ||
|
||
## Pathname | ||
|
||
A union of all valid pathnames in your app. | ||
|
||
<div class="ts-block"> | ||
|
||
```dts | ||
type Pathname = '/' | '/my-route' | `/my-other-route/${string}`; | ||
``` | ||
|
||
</div> | ||
|
||
## ResolvedPathname | ||
|
||
`Pathname`, but possibly prefixed with a [base path](https://svelte.dev/docs/kit/configuration#paths). Used for `page.url.pathname`. | ||
|
||
<div class="ts-block"> | ||
|
||
```dts | ||
type Pathname = '/' | '/my-route' | `/my-other-route/${string}`; | ||
``` | ||
Rich-Harris marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
</div> | ||
|
||
## RouteParams | ||
|
||
A utility for getting the parameters associated with a given route. | ||
|
||
<div class="ts-block"> | ||
|
||
```dts | ||
type RouteParams<T extends RouteId> = { /* generated */ } | Record<string, never>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is there a way this could be more useful? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. such as? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps an example with a concrete type param passed would be helpful? Same for the one below |
||
``` | ||
|
||
</div> | ||
|
||
## LayoutParams | ||
|
||
A utility for getting the parameters associated with a given layout, which is similar to `RouteParams` but also includes optional parameters for any child route. | ||
|
||
<div class="ts-block"> | ||
|
||
```dts | ||
type RouteParams<T extends RouteId> = { /* generated */ } | Record<string, never>; | ||
``` | ||
|
||
</div> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,8 @@ | |
"target": "es2022", | ||
"module": "node16", | ||
"moduleResolution": "node16", | ||
"baseUrl": "." | ||
"baseUrl": ".", | ||
"skipLibCheck": true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
EDIT: Alright, I'm seeing the errors now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm thinking of looking into There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth a shot I guess, though dts-buddy is operating on emitted declaration files and I'm pretty sure the comments are lost by that point - might be tricky to correctly recover them from the source There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I managed to get it working in Rich-Harris/dts-buddy#110 . This should remove the need for the hack in The ts-ignore is only preserved if it meets these two rules:
|
||
}, | ||
"include": ["**/*.js"] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ | |
"baseUrl": ".", | ||
"paths": { | ||
"@sveltejs/kit": ["../kit/types/index"] | ||
} | ||
}, | ||
"skipLibCheck": true | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import { createBundle } from 'dts-buddy'; | ||
import { readFileSync } from 'node:fs'; | ||
import { readFileSync, writeFileSync } from 'node:fs'; | ||
|
||
await createBundle({ | ||
output: 'types/index.d.ts', | ||
|
@@ -28,3 +28,9 @@ if (types.includes('__sveltekit/')) { | |
types | ||
); | ||
} | ||
|
||
// this is hacky as all hell but it gets the tests passing. might be a bug in dts-buddy? | ||
// prettier-ignore | ||
writeFileSync('./types/index.d.ts', types.replace("declare module '$app/server' {", `declare module '$app/server' { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. :wat: |
||
// @ts-ignore | ||
import { LayoutParams as AppLayoutParams, RouteId as AppRouteId } from '$app/types'`)); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ import MagicString from 'magic-string'; | |
import { posixify, rimraf, walk } from '../../../utils/filesystem.js'; | ||
import { compact } from '../../../utils/array.js'; | ||
import { ts } from '../ts.js'; | ||
import { s } from '../../../utils/misc.js'; | ||
|
||
/** | ||
* @typedef {{ | ||
|
@@ -49,6 +50,67 @@ export function write_all_types(config, manifest_data) { | |
} | ||
} | ||
|
||
/** @type {string[]} */ | ||
const pathnames = []; | ||
|
||
/** @type {string[]} */ | ||
const dynamic_routes = []; | ||
|
||
/** @type {string[]} */ | ||
const layouts = []; | ||
|
||
for (const route of manifest_data.routes) { | ||
if (route.params.length > 0) { | ||
const params = route.params.map((p) => `${p.name}${p.optional ? '?:' : ':'} string`); | ||
const route_type = `${s(route.id)}: { ${params.join('; ')} }`; | ||
|
||
dynamic_routes.push(route_type); | ||
|
||
pathnames.push( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as a general rule, could we pull out regexes like this into util functions that say what they do? Like... I'm pretty sure this is removing all square brackets but it is really hard to read, and there are a bunch of similar-but-different ones in the file |
||
`\`${route.id.replace(/\/\[\[[^\]]+\]\]/g, '${string}').replace(/\/\[[^\]]+\]/g, '/${string}')}\` & {}` | ||
); | ||
} else { | ||
pathnames.push(s(route.id)); | ||
} | ||
|
||
/** @type {Map<string, boolean>} */ | ||
const child_params = new Map(route.params.map((p) => [p.name, p.optional])); | ||
|
||
for (const child of manifest_data.routes.filter((r) => r.id.startsWith(route.id))) { | ||
for (const p of child.params) { | ||
if (!child_params.has(p.name)) { | ||
child_params.set(p.name, true); // always optional | ||
} | ||
} | ||
} | ||
|
||
const layout_params = Array.from(child_params) | ||
.map(([name, optional]) => `${name}${optional ? '?:' : ':'} string`) | ||
.join('; '); | ||
|
||
const layout_type = `${s(route.id)}: ${layout_params.length > 0 ? `{ ${layout_params} }` : 'undefined'}`; | ||
layouts.push(layout_type); | ||
} | ||
|
||
try { | ||
fs.mkdirSync(types_dir, { recursive: true }); | ||
} catch {} | ||
|
||
fs.writeFileSync( | ||
`${types_dir}/index.d.ts`, | ||
[ | ||
`type DynamicRoutes = {\n\t${dynamic_routes.join(';\n\t')}\n};`, | ||
`type Layouts = {\n\t${layouts.join(';\n\t')}\n};`, | ||
// we enumerate these rather than doing `keyof Routes` so that the list is visible on hover | ||
`export type RouteId = ${manifest_data.routes.map((r) => s(r.id)).join(' | ')};`, | ||
'export type RouteParams<T extends RouteId> = T extends keyof DynamicRoutes ? DynamicRoutes[T] : Record<string, never>;', | ||
'export type LayoutParams<T extends RouteId> = Layouts[T] | Record<string, never>;', | ||
`export type Pathname = ${pathnames.join(' | ')};`, | ||
'export type ResolvedPathname = `${"" | `/${string}`}${Pathname}`;', | ||
`export type Asset = ${manifest_data.assets.map((asset) => s('/' + asset.file)).join(' | ') || 'never'};` | ||
].join('\n\n') | ||
); | ||
|
||
// Read/write meta data on each invocation, not once per node process, | ||
// it could be invoked by another process in the meantime. | ||
const meta_data_file = `${types_dir}/route_meta_data.json`; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
import type { RouteId, RouteParams, Pathname } from './.svelte-kit/types/index.d.ts'; | ||
|
||
declare let id: RouteId; | ||
|
||
// okay | ||
id = '/'; | ||
id = '/foo/[bar]/[baz]'; | ||
|
||
// @ts-expect-error | ||
id = '/nope'; | ||
|
||
id; | ||
elliott-with-the-longest-name-on-github marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
declare let params: RouteParams<'/foo/[bar]/[baz]'>; | ||
|
||
// @ts-expect-error | ||
params.foo; // not okay | ||
params.bar; // okay | ||
params.baz; // okay | ||
|
||
declare let pathname: Pathname; | ||
|
||
// @ts-expect-error | ||
pathname = '/nope'; | ||
pathname = '/foo'; | ||
pathname = '/foo/1/2'; | ||
|
||
pathname; | ||
elliott-with-the-longest-name-on-github marked this conversation as resolved.
Show resolved
Hide resolved
|
Uh oh!
There was an error while loading. Please reload this page.