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

Param validators #4334

Merged
merged 38 commits into from
Mar 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
5799667
remove fallthrough
Rich-Harris Mar 15, 2022
1e9e2ca
changeset
Rich-Harris Mar 15, 2022
1db07a4
remove fallthrough documentation
Rich-Harris Mar 15, 2022
5fa067b
tweak docs
Rich-Harris Mar 15, 2022
ca3d714
simplify
Rich-Harris Mar 15, 2022
705721b
simplify
Rich-Harris Mar 15, 2022
7a56ac2
simplify a tiny bit
Rich-Harris Mar 15, 2022
29ea702
add failing test of param validators
Rich-Harris Mar 15, 2022
2a480cf
client-side route parsing
Rich-Harris Mar 15, 2022
4ddcc55
tidy up
Rich-Harris Mar 15, 2022
e5245e8
add validators to manifest data
Rich-Harris Mar 15, 2022
273db5f
client-side validation
Rich-Harris Mar 15, 2022
7424b04
simplify
Rich-Harris Mar 15, 2022
098f0e6
server-side param validation
Rich-Harris Mar 15, 2022
eb77dfd
lint
Rich-Harris Mar 15, 2022
e79f4f2
Merge branch 'remove-fallthrough' into param-validators
Rich-Harris Mar 15, 2022
aa415d9
oops
Rich-Harris Mar 15, 2022
ea24c9f
Merge branch 'remove-fallthrough' into param-validators
Rich-Harris Mar 15, 2022
63b7f4a
clarify
Rich-Harris Mar 15, 2022
fc80f31
docs
Rich-Harris Mar 15, 2022
07accf9
minor fixes
Rich-Harris Mar 15, 2022
504ca26
fixes
Rich-Harris Mar 15, 2022
ccc75e3
ease debugging
Rich-Harris Mar 15, 2022
6d6c595
vanquish SPA reloading bug
Rich-Harris Mar 15, 2022
c6ccdf3
simplify
Rich-Harris Mar 15, 2022
bbb1f80
lint
Rich-Harris Mar 15, 2022
ce6efd2
windows fix
Rich-Harris Mar 15, 2022
714f78c
changeset
Rich-Harris Mar 15, 2022
5ab21cd
throw error if validator module is missing a validate export
Rich-Harris Mar 16, 2022
72fcf94
update configuration.md
Rich-Harris Mar 16, 2022
a01d452
Update documentation/docs/01-routing.md
Rich-Harris Mar 16, 2022
d6f12c5
tighten up validator naming requirements
Rich-Harris Mar 16, 2022
31135cc
Merge branch 'param-validators' of github.com:sveltejs/kit into param…
Rich-Harris Mar 16, 2022
316cfca
disallow $ in both param names and types
Rich-Harris Mar 16, 2022
c650933
changeset
Rich-Harris Mar 16, 2022
97c70ae
point fallthrough users at validation docs
Rich-Harris Mar 16, 2022
b45b3d0
add some JSDoc commentsd
Rich-Harris Mar 16, 2022
3555883
merge master
Rich-Harris Mar 16, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/poor-horses-return.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

Add param validators
5 changes: 5 additions & 0 deletions .changeset/tender-plants-smell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

[breaking] disallow \$ character in dynamic parameters
22 changes: 22 additions & 0 deletions documentation/docs/01-routing.md
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,27 @@ A route can have multiple dynamic parameters, for example `src/routes/[category]

> `src/routes/a/[...rest]/z.svelte` will match `/a/z` as well as `/a/b/z` and `/a/b/c/z` and so on. Make sure you check that the value of the rest parameter is valid.

#### Validation

A route like `src/routes/archive/[page]` would match `/archive/3`, but it would also match `/archive/potato`. We don't want that. You can ensure that route parameters are well-formed by adding a _validator_ — which takes the parameter string (`"3"` or `"potato"`) and returns `true` if it is valid — to your [`params`](/docs/configuration#files) directory...

```js
/// file: src/params/integer.js
/** @type {import('@sveltejs/kit').ParamValidator} */
export function validate(param) {
return /^\d+$/.test(param);
Rich-Harris marked this conversation as resolved.
Show resolved Hide resolved
}
```

...and augmenting your routes:

```diff
-src/routes/archive/[page]
+src/routes/archive/[page=integer]
```

If the pathname doesn't validate, SvelteKit will try to match other routes (using the sort order specified below), before eventually returning a 404.

#### Sorting

It's possible for multiple routes to match a given path. For example each of these routes would match `/foo-abc`:
Expand All @@ -333,6 +354,7 @@ SvelteKit needs to know which route is being requested. To do so, it sorts them

- More specific routes are higher priority
- Standalone endpoints have higher priority than pages with the same specificity
- Parameters with [validators](#validation) (`[name=type]`) are higher priority than those without (`[name]`)
- Rest parameters have lowest priority
- Ties are resolved alphabetically

Expand Down
2 changes: 2 additions & 0 deletions documentation/docs/13-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const config = {
assets: 'static',
hooks: 'src/hooks',
lib: 'src/lib',
params: 'src/params',
routes: 'src/routes',
serviceWorker: 'src/service-worker',
template: 'src/app.html'
Expand Down Expand Up @@ -148,6 +149,7 @@ An object containing zero or more of the following `string` values:
- `assets` — a place to put static files that should have stable URLs and undergo no processing, such as `favicon.ico` or `manifest.json`
- `hooks` — the location of your hooks module (see [Hooks](/docs/hooks))
- `lib` — your app's internal library, accessible throughout the codebase as `$lib`
- `params` — a directory containing [parameter validators](/docs/routing#advanced-routing-validation)
- `routes` — the files that define the structure of your app (see [Routing](/docs/routing))
- `serviceWorker` — the location of your service worker's entry point (see [Service workers](/docs/service-workers))
- `template` — the location of the template for HTML responses
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ const config = {

prerender: {
default: true
},

vite: {
build: {
minify: false
}
}
}
};
Expand Down
8 changes: 7 additions & 1 deletion packages/adapter-static/test/apps/spa/svelte.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@ const config = {
kit: {
adapter: adapter({
fallback: '200.html'
})
}),

vite: {
build: {
minify: false
}
}
}
};

Expand Down
7 changes: 6 additions & 1 deletion packages/kit/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,12 @@ export default [
format: 'esm',
chunkFileNames: 'chunks/[name].js'
},
external: ['svelte', 'svelte/store', '__GENERATED__/root.svelte', '__GENERATED__/manifest.js'],
external: [
'svelte',
'svelte/store',
'__GENERATED__/root.svelte',
'__GENERATED__/client-manifest.js'
],
plugins: [
resolve({
extensions: ['.mjs', '.js', '.ts']
Expand Down
8 changes: 7 additions & 1 deletion packages/kit/src/core/build/build_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ export async function build_server(
}
});

// ...and every component used by pages
// ...and every component used by pages...
manifest_data.components.forEach((file) => {
const resolved = path.resolve(cwd, file);
const relative = path.relative(config.kit.files.routes, resolved);
Expand All @@ -163,6 +163,12 @@ export async function build_server(
input[name] = resolved;
});

// ...and every validator
Object.entries(manifest_data.validators).forEach(([key, file]) => {
const name = posixify(path.join('entries/validators', key));
input[name] = path.resolve(cwd, file);
});

/** @type {(file: string) => string} */
const app_relative = (file) => {
const relative_file = path.relative(build_dir, path.resolve(cwd, file));
Expand Down
10 changes: 4 additions & 6 deletions packages/kit/src/core/config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,10 @@ export async function load_config({ cwd = process.cwd() } = {}) {

validated.kit.outDir = path.resolve(cwd, validated.kit.outDir);

validated.kit.files.assets = path.resolve(cwd, validated.kit.files.assets);
validated.kit.files.hooks = path.resolve(cwd, validated.kit.files.hooks);
validated.kit.files.lib = path.resolve(cwd, validated.kit.files.lib);
validated.kit.files.routes = path.resolve(cwd, validated.kit.files.routes);
validated.kit.files.serviceWorker = path.resolve(cwd, validated.kit.files.serviceWorker);
validated.kit.files.template = path.resolve(cwd, validated.kit.files.template);
for (const key in validated.kit.files) {
// @ts-expect-error this is typescript at its stupidest
validated.kit.files[key] = path.resolve(cwd, validated.kit.files[key]);
}

return validated;
}
Expand Down
1 change: 1 addition & 0 deletions packages/kit/src/core/config/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ const get_defaults = (prefix = '') => ({
assets: join(prefix, 'static'),
hooks: join(prefix, 'src/hooks'),
lib: join(prefix, 'src/lib'),
params: join(prefix, 'src/params'),
routes: join(prefix, 'src/routes'),
serviceWorker: join(prefix, 'src/service-worker'),
template: join(prefix, 'src/app.html')
Expand Down
1 change: 1 addition & 0 deletions packages/kit/src/core/config/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ const options = object(
assets: string('static'),
hooks: string(join('src', 'hooks')),
lib: string(join('src', 'lib')),
params: string(join('src', 'params')),
routes: string(join('src', 'routes')),
serviceWorker: string(join('src', 'service-worker')),
template: string(join('src', 'app.html'))
Expand Down
56 changes: 28 additions & 28 deletions packages/kit/src/core/dev/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { coalesce_to_error } from '../../utils/error.js';
import { load_template } from '../config/index.js';
import { sequence } from '../../hooks.js';
import { posixify } from '../../utils/filesystem.js';
import { parse_route_key } from '../../utils/routing.js';

/**
* @param {import('types').ValidatedConfig} config
Expand Down Expand Up @@ -104,12 +105,15 @@ export async function create_plugin(config, cwd) {
};
}),
routes: manifest_data.routes.map((route) => {
const { pattern, names, types } = parse_route_key(route.key);

if (route.type === 'page') {
return {
type: 'page',
key: route.key,
pattern: route.pattern,
params: get_params(route.params),
pattern,
names,
types,
shadow: route.shadow
? async () => {
const url = path.resolve(cwd, /** @type {string} */ (route.shadow));
Expand All @@ -123,14 +127,33 @@ export async function create_plugin(config, cwd) {

return {
type: 'endpoint',
pattern: route.pattern,
params: get_params(route.params),
pattern,
names,
types,
load: async () => {
const url = path.resolve(cwd, route.file);
return await vite.ssrLoadModule(url);
}
};
})
}),
validators: async () => {
/** @type {Record<string, import('types').ParamValidator>} */
const validators = {};

for (const key in manifest_data.validators) {
const file = manifest_data.validators[key];
const url = path.resolve(cwd, file);
const module = await vite.ssrLoadModule(url);

if (module.validate) {
validators[key] = module.validate;
} else {
throw new Error(`${file} does not export a \`validate\` function`);
}
}

return validators;
}
}
};
}
Expand Down Expand Up @@ -343,29 +366,6 @@ export async function create_plugin(config, cwd) {
};
}

/** @param {string[]} array */
function get_params(array) {
// given an array of params like `['x', 'y', 'z']` for
// src/routes/[x]/[y]/[z]/svelte, create a function
// that turns a RegExpExecArray into ({ x, y, z })

/** @param {RegExpExecArray} match */
const fn = (match) => {
/** @type {Record<string, string>} */
const params = {};
array.forEach((key, i) => {
if (key.startsWith('...')) {
params[key.slice(3)] = match[i + 1] || '';
} else {
params[key] = match[i + 1];
}
});
return params;
};

return fn;
}

/** @param {import('http').ServerResponse} res */
function not_found(res) {
res.statusCode = 404;
Expand Down
58 changes: 29 additions & 29 deletions packages/kit/src/core/generate_manifest/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { s } from '../../utils/misc.js';
import { parse_route_key } from '../../utils/routing.js';
import { get_mime_lookup } from '../utils.js';

/**
Expand Down Expand Up @@ -41,10 +42,13 @@ export function generate_manifest({ build_data, relative_path, routes, format =
});

/** @type {(path: string) => string} */
const importer =
const load =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally liked importer a bit more to avoid conflating with the page load

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is quite far away from anything touching page, but the endpoints constructed here do have a load method, so it arguably makes more sense this way. though the reason i changed it was because we need both import(...) and () => import(...), and while import/importer would have been a good combo, import isn't a valid variable name whereas load is.

format === 'esm'
? (path) => `() => import('${path}')`
: (path) => `() => Promise.resolve().then(() => require('${path}'))`;
? (path) => `import('${path}')`
: (path) => `Promise.resolve().then(() => require('${path}'))`;

/** @type {(path: string) => string} */
const loader = (path) => `() => ${load(path)}`;

const assets = build_data.manifest_data.assets.map((asset) => asset.file);
if (build_data.service_worker) {
Expand All @@ -54,6 +58,8 @@ export function generate_manifest({ build_data, relative_path, routes, format =
/** @param {string} id */
const get_index = (id) => id && /** @type {LookupEntry} */ (bundled_nodes.get(id)).index;

const validators = new Set();

// prettier-ignore
return `{
appDir: ${s(build_data.app_dir)},
Expand All @@ -62,18 +68,25 @@ export function generate_manifest({ build_data, relative_path, routes, format =
_: {
entry: ${s(build_data.client.entry)},
nodes: [
${Array.from(bundled_nodes.values()).map(node => importer(node.path)).join(',\n\t\t\t\t')}
${Array.from(bundled_nodes.values()).map(node => loader(node.path)).join(',\n\t\t\t\t')}
],
routes: [
${routes.map(route => {
const { pattern, names, types } = parse_route_key(route.key);

types.forEach(type => {
if (type) validators.add(type);
});

if (route.type === 'page') {
return `{
type: 'page',
key: ${s(route.key)},
pattern: ${route.pattern},
params: ${get_params(route.params)},
pattern: ${pattern},
names: ${s(names)},
types: ${s(types)},
path: ${route.path ? s(route.path) : null},
shadow: ${route.shadow ? importer(`${relative_path}/${build_data.server.vite_manifest[route.shadow].file}`) : null},
shadow: ${route.shadow ? loader(`${relative_path}/${build_data.server.vite_manifest[route.shadow].file}`) : null},
a: ${s(route.a.map(get_index))},
b: ${s(route.b.map(get_index))}
}`.replace(/^\t\t/gm, '');
Expand All @@ -86,31 +99,18 @@ export function generate_manifest({ build_data, relative_path, routes, format =

return `{
type: 'endpoint',
pattern: ${route.pattern},
params: ${get_params(route.params)},
load: ${importer(`${relative_path}/${build_data.server.vite_manifest[route.file].file}`)}
pattern: ${pattern},
names: ${s(names)},
types: ${s(types)},
load: ${loader(`${relative_path}/${build_data.server.vite_manifest[route.file].file}`)}
}`.replace(/^\t\t/gm, '');
}
}).filter(Boolean).join(',\n\t\t\t\t')}
]
],
validators: async () => {
${Array.from(validators).map(type => `const { validate: ${type} } = await ${load(`${relative_path}/entries/validators/${type}.js`)}`).join('\n\t\t\t\t')}
return { ${Array.from(validators).join(', ')} };
}
}
}`.replace(/^\t/gm, '');
}

/** @param {string[]} array */
function get_params(array) {
// given an array of params like `['x', 'y', 'z']` for
// src/routes/[x]/[y]/[z]/svelte, create a function
// that turns a RexExpMatchArray into ({ x, y, z })
return array.length
? '(m) => ({ ' +
array
.map((param, i) => {
return param.startsWith('...')
? `${param.slice(3)}: m[${i + 1}] || ''`
: `${param}: m[${i + 1}]`;
})
.join(', ') +
'})'
: 'null';
}
Loading