Skip to content

Commit

Permalink
Populate event.params before calling handle (#4344)
Browse files Browse the repository at this point in the history
* remove fallthrough

* changeset

* remove fallthrough documentation

* tweak docs

* simplify

* simplify

* simplify a tiny bit

* add failing test of param validators

* client-side route parsing

* tidy up

* add validators to manifest data

* client-side validation

* simplify

* server-side param validation

* lint

* oops

* clarify

* docs

* minor fixes

* fixes

* ease debugging

* vanquish SPA reloading bug

* simplify

* lint

* windows fix

* changeset

* match route before calling handle - closes #1194

* changeset

* throw error if validator module is missing a validate export

* update configuration.md

* Update documentation/docs/01-routing.md

* tighten up validator naming requirements

* disallow $ in both param names and types

* changeset

* point fallthrough users at validation docs

* add some JSDoc commentsd

* expose `event.routeId` and `page.routeId` (#4345)

* expose event.routeKey - closes #3840

* change routeKey to routeId

* rename routeKey to routeId, expose page.routeId

* rename route.key -> route.id everywhere

* adapter-node sure picked a weird time to stop typechecking

* oops

* Update .changeset/mean-crews-unite.md
  • Loading branch information
Rich-Harris committed Mar 16, 2022
1 parent 71ed93a commit 04dd532
Show file tree
Hide file tree
Showing 30 changed files with 271 additions and 190 deletions.
5 changes: 5 additions & 0 deletions .changeset/mean-crews-unite.md
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

Expose `event.routeId` and `page.routeId`
5 changes: 5 additions & 0 deletions .changeset/thick-oranges-walk.md
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

Populate event.params before calling handle
3 changes: 2 additions & 1 deletion packages/adapter-node/src/handler.js
Expand Up @@ -44,7 +44,8 @@ const ssr = async (req, res) => {
request = await getRequest(origin || get_origin(req.headers), req);
} catch (err) {
res.statusCode = err.status || 400;
return res.end(err.reason || 'Invalid request body');
res.end(err.reason || 'Invalid request body');
return;
}

if (address_header && !(address_header in req.headers)) {
Expand Down
7 changes: 4 additions & 3 deletions packages/kit/src/core/dev/plugin.js
Expand Up @@ -12,7 +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';
import { parse_route_id } from '../../utils/routing.js';

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

if (route.type === 'page') {
return {
type: 'page',
key: route.key,
id: route.id,
pattern,
names,
types,
Expand All @@ -127,6 +127,7 @@ export async function create_plugin(config, cwd) {

return {
type: 'endpoint',
id: route.id,
pattern,
names,
types,
Expand Down
7 changes: 4 additions & 3 deletions packages/kit/src/core/generate_manifest/index.js
@@ -1,5 +1,5 @@
import { s } from '../../utils/misc.js';
import { parse_route_key } from '../../utils/routing.js';
import { parse_route_id } from '../../utils/routing.js';
import { get_mime_lookup } from '../utils.js';

/**
Expand Down Expand Up @@ -72,7 +72,7 @@ export function generate_manifest({ build_data, relative_path, routes, format =
],
routes: [
${routes.map(route => {
const { pattern, names, types } = parse_route_key(route.key);
const { pattern, names, types } = parse_route_id(route.id);
types.forEach(type => {
if (type) validators.add(type);
Expand All @@ -81,7 +81,7 @@ export function generate_manifest({ build_data, relative_path, routes, format =
if (route.type === 'page') {
return `{
type: 'page',
key: ${s(route.key)},
id: ${s(route.id)},
pattern: ${pattern},
names: ${s(names)},
types: ${s(types)},
Expand All @@ -99,6 +99,7 @@ export function generate_manifest({ build_data, relative_path, routes, format =
return `{
type: 'endpoint',
id: ${s(route.id)},
pattern: ${pattern},
names: ${s(names)},
types: ${s(types)},
Expand Down
22 changes: 11 additions & 11 deletions packages/kit/src/core/sync/create_manifest_data/index.js
Expand Up @@ -61,13 +61,13 @@ export default function create_manifest_data({

/**
* @param {string} dir
* @param {string[]} parent_key
* @param {string[]} parent_id
* @param {Part[][]} parent_segments
* @param {string[]} parent_params
* @param {Array<string|undefined>} layout_stack // accumulated __layout.svelte components
* @param {Array<string|undefined>} error_stack // accumulated __error.svelte components
*/
function walk(dir, parent_key, parent_segments, parent_params, layout_stack, error_stack) {
function walk(dir, parent_id, parent_segments, parent_params, layout_stack, error_stack) {
/** @type {Item[]} */
let items = [];
fs.readdirSync(dir).forEach((basename) => {
Expand Down Expand Up @@ -135,7 +135,7 @@ export default function create_manifest_data({
items = items.sort(comparator);

items.forEach((item) => {
const key = parent_key.slice();
const id = parent_id.slice();
const segments = parent_segments.slice();

if (item.is_index) {
Expand All @@ -161,13 +161,13 @@ export default function create_manifest_data({
}

segments[segments.length - 1] = last_segment;
key[key.length - 1] += item.route_suffix;
id[id.length - 1] += item.route_suffix;
} else {
segments.push(item.parts);
}
}
} else {
key.push(item.name);
id.push(item.name);
segments.push(item.parts);
}

Expand Down Expand Up @@ -201,7 +201,7 @@ export default function create_manifest_data({

walk(
path.join(dir, item.basename),
key,
id,
segments,
params,
layout_reset ? [layout_reset] : layout_stack.concat(layout),
Expand Down Expand Up @@ -236,7 +236,7 @@ export default function create_manifest_data({

routes.push({
type: 'page',
key: key.join('/'),
id: id.join('/'),
segments: simple_segments,
pattern,
params,
Expand All @@ -250,7 +250,7 @@ export default function create_manifest_data({

routes.push({
type: 'endpoint',
key: key.join('/'),
id: id.join('/'),
segments: simple_segments,
pattern,
file: item.file,
Expand All @@ -272,15 +272,15 @@ export default function create_manifest_data({
const lookup = new Map();
for (const route of routes) {
if (route.type === 'page') {
lookup.set(route.key, route);
lookup.set(route.id, route);
}
}

let i = routes.length;
while (i--) {
const route = routes[i];
if (route.type === 'endpoint' && lookup.has(route.key)) {
lookup.get(route.key).shadow = route.file;
if (route.type === 'endpoint' && lookup.has(route.id)) {
lookup.get(route.id).shadow = route.file;
routes.splice(i, 1);
}
}
Expand Down
44 changes: 22 additions & 22 deletions packages/kit/src/core/sync/create_manifest_data/index.spec.js
Expand Up @@ -41,7 +41,7 @@ test('creates routes', () => {
assert.equal(routes, [
{
type: 'page',
key: '',
id: '',
segments: [],
pattern: /^\/$/,
params: [],
Expand All @@ -53,7 +53,7 @@ test('creates routes', () => {

{
type: 'page',
key: 'about',
id: 'about',
segments: [{ rest: false, dynamic: false, content: 'about' }],
pattern: /^\/about\/?$/,
params: [],
Expand All @@ -65,7 +65,7 @@ test('creates routes', () => {

{
type: 'endpoint',
key: 'blog.json',
id: 'blog.json',
segments: [{ rest: false, dynamic: false, content: 'blog.json' }],
pattern: /^\/blog\.json$/,
file: 'samples/basic/blog/index.json.js',
Expand All @@ -74,7 +74,7 @@ test('creates routes', () => {

{
type: 'page',
key: 'blog',
id: 'blog',
segments: [{ rest: false, dynamic: false, content: 'blog' }],
pattern: /^\/blog\/?$/,
params: [],
Expand All @@ -86,7 +86,7 @@ test('creates routes', () => {

{
type: 'endpoint',
key: 'blog/[slug].json',
id: 'blog/[slug].json',
segments: [
{ rest: false, dynamic: false, content: 'blog' },
{ rest: false, dynamic: true, content: '[slug].json' }
Expand All @@ -98,7 +98,7 @@ test('creates routes', () => {

{
type: 'page',
key: 'blog/[slug]',
id: 'blog/[slug]',
segments: [
{ rest: false, dynamic: false, content: 'blog' },
{ rest: false, dynamic: true, content: '[slug]' }
Expand Down Expand Up @@ -127,7 +127,7 @@ test('creates routes with layout', () => {
assert.equal(routes, [
{
type: 'page',
key: '',
id: '',
segments: [],
pattern: /^\/$/,
params: [],
Expand All @@ -139,7 +139,7 @@ test('creates routes with layout', () => {

{
type: 'page',
key: 'foo',
id: 'foo',
segments: [{ rest: false, dynamic: false, content: 'foo' }],
pattern: /^\/foo\/?$/,
params: [],
Expand Down Expand Up @@ -223,7 +223,7 @@ test('disallows rest parameters inside segments', () => {
assert.equal(routes, [
{
type: 'page',
key: 'prefix-[...rest]',
id: 'prefix-[...rest]',
segments: [
{
dynamic: true,
Expand All @@ -240,7 +240,7 @@ test('disallows rest parameters inside segments', () => {
},
{
type: 'endpoint',
key: '[...rest].json',
id: '[...rest].json',
segments: [
{
dynamic: true,
Expand Down Expand Up @@ -307,7 +307,7 @@ test('allows multiple slugs', () => {
[
{
type: 'endpoint',
key: '[file].[ext]',
id: '[file].[ext]',
segments: [{ dynamic: true, rest: false, content: '[file].[ext]' }],
pattern: /^\/([^/]+?)\.([^/]+?)$/,
file: 'samples/multiple-slugs/[file].[ext].js',
Expand All @@ -329,7 +329,7 @@ test('ignores things that look like lockfiles', () => {
assert.equal(routes, [
{
type: 'endpoint',
key: 'foo',
id: 'foo',
segments: [{ rest: false, dynamic: false, content: 'foo' }],
file: 'samples/lockfiles/foo.js',
params: [],
Expand All @@ -353,7 +353,7 @@ test('works with custom extensions', () => {
assert.equal(routes, [
{
type: 'page',
key: '',
id: '',
segments: [],
pattern: /^\/$/,
params: [],
Expand All @@ -365,7 +365,7 @@ test('works with custom extensions', () => {

{
type: 'page',
key: 'about',
id: 'about',
segments: [{ rest: false, dynamic: false, content: 'about' }],
pattern: /^\/about\/?$/,
params: [],
Expand All @@ -377,7 +377,7 @@ test('works with custom extensions', () => {

{
type: 'endpoint',
key: 'blog.json',
id: 'blog.json',
segments: [{ rest: false, dynamic: false, content: 'blog.json' }],
pattern: /^\/blog\.json$/,
file: 'samples/custom-extension/blog/index.json.js',
Expand All @@ -386,7 +386,7 @@ test('works with custom extensions', () => {

{
type: 'page',
key: 'blog',
id: 'blog',
segments: [{ rest: false, dynamic: false, content: 'blog' }],
pattern: /^\/blog\/?$/,
params: [],
Expand All @@ -398,7 +398,7 @@ test('works with custom extensions', () => {

{
type: 'endpoint',
key: 'blog/[slug].json',
id: 'blog/[slug].json',
segments: [
{ rest: false, dynamic: false, content: 'blog' },
{ rest: false, dynamic: true, content: '[slug].json' }
Expand All @@ -410,7 +410,7 @@ test('works with custom extensions', () => {

{
type: 'page',
key: 'blog/[slug]',
id: 'blog/[slug]',
segments: [
{ rest: false, dynamic: false, content: 'blog' },
{ rest: false, dynamic: true, content: '[slug]' }
Expand Down Expand Up @@ -448,7 +448,7 @@ test('includes nested error components', () => {
assert.equal(routes, [
{
type: 'page',
key: 'foo/bar/baz',
id: 'foo/bar/baz',
segments: [
{ rest: false, dynamic: false, content: 'foo' },
{ rest: false, dynamic: false, content: 'bar' },
Expand Down Expand Up @@ -481,7 +481,7 @@ test('resets layout', () => {
assert.equal(routes, [
{
type: 'page',
key: '',
id: '',
segments: [],
pattern: /^\/$/,
params: [],
Expand All @@ -492,7 +492,7 @@ test('resets layout', () => {
},
{
type: 'page',
key: 'foo',
id: 'foo',
segments: [{ rest: false, dynamic: false, content: 'foo' }],
pattern: /^\/foo\/?$/,
params: [],
Expand All @@ -507,7 +507,7 @@ test('resets layout', () => {
},
{
type: 'page',
key: 'foo/bar',
id: 'foo/bar',
segments: [
{ rest: false, dynamic: false, content: 'foo' },
{ rest: false, dynamic: false, content: 'bar' }
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/src/core/sync/write_manifest.js
Expand Up @@ -35,7 +35,7 @@ export function write_manifest(manifest_data, base, output) {
const tuple = [get_indices(route.a), get_indices(route.b)];
if (route.shadow) tuple.push('1');
return `${s(route.key)}: [${tuple.join(', ')}]`;
return `${s(route.id)}: [${tuple.join(', ')}]`;
}
})
.filter(Boolean)
Expand Down

0 comments on commit 04dd532

Please sign in to comment.