Skip to content

Commit

Permalink
feat: uppercase endpoint methods (#5513)
Browse files Browse the repository at this point in the history
* feat: Endpoint names uppercased

* feat: Throw for old endpoint names

* boring: Update tests

* fix: Missed stuff

* feat: Documentation

* feat: Changeset

* feat: Last few old-method-name references

* feat: Minor tidy

* check page endpoint methods as well as standalone endpoints

* format

* update template

* ofc I miss the entire point of this change

* update kit.svelte.dev

Co-authored-by: Rich Harris <richard.a.harris@gmail.com>
Co-authored-by: Rich Harris <hello@rich-harris.dev>
Co-authored-by: gtm-nayan <gtmnayan@gmail.com>
  • Loading branch information
4 people authored Jul 15, 2022
1 parent 2a796e4 commit 87552ef
Show file tree
Hide file tree
Showing 100 changed files with 174 additions and 151 deletions.
5 changes: 5 additions & 0 deletions .changeset/empty-teachers-cheat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

[breaking] Endpoint method names uppercased to match HTTP specifications
5 changes: 5 additions & 0 deletions .changeset/honest-coins-switch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'create-svelte': patch
---

uppercase handlers
28 changes: 14 additions & 14 deletions documentation/docs/03-routing.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ Their job is to return a `{ status, headers, body }` object representing the res
```js
/// file: src/routes/random.js
/** @type {import('@sveltejs/kit').RequestHandler} */
export async function get() {
export async function GET() {
return {
status: 200,
headers: {
Expand Down Expand Up @@ -117,7 +117,7 @@ export type RequestHandler<Body = any> = GenericRequestHandler<{ id: string }, B
import db from '$lib/database';

/** @type {import('./__types/[id]').RequestHandler} */
export async function get({ params }) {
export async function GET({ params }) {
// `params.id` comes from [id].js
const item = await db.get(params.id);

Expand All @@ -135,7 +135,7 @@ export async function get({ params }) {
}
```

> The type of the `get` function above comes from `./__types/[id].d.ts`, which is a file generated by SvelteKit (inside your [`outDir`](/docs/configuration#outdir), using the [`rootDirs`](https://www.typescriptlang.org/tsconfig#rootDirs) option) that provides type safety when accessing `params`. See the section on [generated types](/docs/types#generated-types) for more detail.
> The type of the `GET` function above comes from `./__types/[id].d.ts`, which is a file generated by SvelteKit (inside your [`outDir`](/docs/configuration#outdir), using the [`rootDirs`](https://www.typescriptlang.org/tsconfig#rootDirs) option) that provides type safety when accessing `params`. See the section on [generated types](/docs/types#generated-types) for more detail.
To get the raw data instead of the page, you can include an `accept: application/json` header in the request, or — for convenience — append `/__data.json` to the URL, e.g. `/items/[id]/__data.json`.

Expand All @@ -158,13 +158,13 @@ Endpoints can handle any HTTP method — not just `GET` — by exporting the cor

```js
// @noErrors
export function post(event) {...}
export function put(event) {...}
export function patch(event) {...}
export function del(event) {...} // `delete` is a reserved word
export function POST(event) {...}
export function PUT(event) {...}
export function PATCH(event) {...}
export function DELETE(event) {...}
```

These functions can, like `get`, return a `body` that will be passed to the page as props. Whereas 4xx/5xx responses from `get` will result in an error page rendering, similar responses to non-GET requests do not, allowing you to do things like render form validation errors:
These functions can, like `GET`, return a `body` that will be passed to the page as props. Whereas 4xx/5xx responses from `GET` will result in an error page rendering, similar responses to non-GET requests do not, allowing you to do things like render form validation errors:

```js
/// file: src/routes/items.js
Expand All @@ -188,7 +188,7 @@ export type RequestHandler<Body = any> = GenericRequestHandler<{}, Body>;
import * as db from '$lib/database';

/** @type {import('./__types/items').RequestHandler} */
export async function get() {
export async function GET() {
const items = await db.list();

return {
Expand All @@ -197,7 +197,7 @@ export async function get() {
}

/** @type {import('./__types/items').RequestHandler} */
export async function post({ request }) {
export async function POST({ request }) {
const [errors, item] = await db.create(request);

if (errors) {
Expand All @@ -221,10 +221,10 @@ export async function post({ request }) {
```svelte
/// file: src/routes/items.svelte
<script>
// The page always has access to props from `get`...
// The page always has access to props from `GET`...
export let items;
// ...plus props from `post` when the page is rendered
// ...plus props from `POST` when the page is rendered
// in response to a POST request, for example after
// submitting the form below
export let errors;
Expand Down Expand Up @@ -260,7 +260,7 @@ export {};
// @filename: index.js
// ---cut---
/** @type {import('@sveltejs/kit').RequestHandler} */
export async function post({ request }) {
export async function POST({ request }) {
const data = await request.formData(); // or .json(), or .text(), etc

await create(data);
Expand All @@ -280,7 +280,7 @@ const cookie2: string;
// @filename: index.js
// ---cut---
/** @type {import('@sveltejs/kit').RequestHandler} */
export function get() {
export function GET() {
return {
headers: {
'set-cookie': [cookie1, cookie2]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { api } from './_api';
import type { RequestHandler } from './__types';

/** @type {import('./__types').RequestHandler} */
export const get: RequestHandler = async ({ locals }) => {
export const GET: RequestHandler = async ({ locals }) => {
// locals.userid comes from src/hooks.js
const response = await api('GET', `todos/${locals.userid}`);

Expand Down Expand Up @@ -30,7 +30,7 @@ export const get: RequestHandler = async ({ locals }) => {
};

/** @type {import('./__types').RequestHandler} */
export const post: RequestHandler = async ({ request, locals }) => {
export const POST: RequestHandler = async ({ request, locals }) => {
const form = await request.formData();

await api('POST', `todos/${locals.userid}`, {
Expand All @@ -50,7 +50,7 @@ const redirect = {
};

/** @type {import('./__types').RequestHandler} */
export const patch: RequestHandler = async ({ request, locals }) => {
export const PATCH: RequestHandler = async ({ request, locals }) => {
const form = await request.formData();

await api('PATCH', `todos/${locals.userid}/${form.get('uid')}`, {
Expand All @@ -62,7 +62,7 @@ export const patch: RequestHandler = async ({ request, locals }) => {
};

/** @type {import('./__types').RequestHandler} */
export const del: RequestHandler = async ({ request, locals }) => {
export const DELETE: RequestHandler = async ({ request, locals }) => {
const form = await request.formData();

await api('DELETE', `todos/${locals.userid}/${form.get('uid')}`);
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/src/core/adapt/builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export function create_builder({ config, build_data, prerendered, log }) {
content: segment
})),
pattern: route.pattern,
methods: route.type === 'page' ? ['get'] : build_data.server.methods[route.file]
methods: route.type === 'page' ? ['GET'] : build_data.server.methods[route.file]
}));

const seen = new Set();
Expand Down
21 changes: 11 additions & 10 deletions packages/kit/src/runtime/server/endpoint.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { to_headers } from '../../utils/http.js';
import { hash } from '../hash.js';
import { is_pojo, normalize_request_method, serialize_error } from './utils.js';
import { check_method_names, is_pojo, serialize_error } from './utils.js';

/** @param {string} body */
function error(body) {
Expand Down Expand Up @@ -43,32 +43,33 @@ export function is_text(content_type) {
* @returns {Promise<Response>}
*/
export async function render_endpoint(event, mod, options) {
const method = normalize_request_method(event);
const { method } = event.request;

check_method_names(mod);

/** @type {import('types').RequestHandler} */
let handler = mod[method];

if (!handler && method === 'head') {
handler = mod.get;
if (!handler && method === 'HEAD') {
handler = mod.GET;
}

if (!handler) {
const allowed = [];

for (const method in ['get', 'post', 'put', 'patch']) {
if (mod[method]) allowed.push(method.toUpperCase());
for (const method in ['GET', 'POST', 'PUT', 'PATCH', 'DELETE']) {
if (mod[method]) allowed.push(method);
}

if (mod.del) allowed.push('DELETE');
if (mod.get || mod.head) allowed.push('HEAD');
if (mod.GET || mod.HEAD) allowed.push('HEAD');

return event.request.headers.get('x-sveltekit-load')
? // TODO would be nice to avoid these requests altogether,
// by noting whether or not page endpoints export `get`
new Response(undefined, {
status: 204
})
: new Response(`${event.request.method} method not allowed`, {
: new Response(`${method} method not allowed`, {
status: 405,
headers: {
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/405
Expand Down Expand Up @@ -132,7 +133,7 @@ export async function render_endpoint(event, mod, options) {
}

return new Response(
method !== 'head' && !bodyless_status_codes.has(status) ? normalized_body : undefined,
method !== 'HEAD' && !bodyless_status_codes.has(status) ? normalized_body : undefined,
{
status,
headers
Expand Down
14 changes: 8 additions & 6 deletions packages/kit/src/runtime/server/page/load_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as set_cookie_parser from 'set-cookie-parser';
import { normalize } from '../../load.js';
import { respond } from '../index.js';
import { LoadURL, PrerenderingURL, is_root_relative, resolve } from '../../../utils/url.js';
import { is_pojo, lowercase_keys, normalize_request_method } from '../utils.js';
import { check_method_names, is_pojo, lowercase_keys } from '../utils.js';
import { coalesce_to_error } from '../../../utils/error.js';
import { domain_matches, path_matches } from './cookie.js';

Expand Down Expand Up @@ -409,13 +409,15 @@ async function load_shadow_data(route, event, options, prerender) {
try {
const mod = await route.shadow();

if (prerender && (mod.post || mod.put || mod.del || mod.patch)) {
check_method_names(mod);

if (prerender && (mod.POST || mod.PUT || mod.DELETE || mod.PATCH)) {
throw new Error('Cannot prerender pages that have endpoints with mutative methods');
}

const method = normalize_request_method(event);
const is_get = method === 'head' || method === 'get';
const handler = method === 'head' ? mod.head || mod.get : mod[method];
const { method } = event.request;
const is_get = method === 'HEAD' || method === 'GET';
const handler = method === 'HEAD' ? mod.HEAD || mod.GET : mod[method];

if (!handler && !is_get) {
return {
Expand Down Expand Up @@ -462,7 +464,7 @@ async function load_shadow_data(route, event, options, prerender) {
data.body = body;
}

const get = (method === 'head' && mod.head) || mod.get;
const get = (method === 'HEAD' && mod.HEAD) || mod.GET;
if (get) {
const { status, headers, body } = validate_shadow_output(await get(event));
add_cookies(/** @type {string[]} */ (data.cookies), headers);
Expand Down
17 changes: 11 additions & 6 deletions packages/kit/src/runtime/server/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,6 @@ export function is_pojo(body) {
return true;
}

/** @param {import('types').RequestEvent} event */
export function normalize_request_method(event) {
const method = event.request.method.toLowerCase();
return method === 'delete' ? 'del' : method; // 'delete' is a reserved word
}

/**
* Serialize an error into a JSON string, by copying its `name`, `message`
* and (in dev) `stack`, plus any custom properties, plus recursively
Expand Down Expand Up @@ -97,6 +91,17 @@ function clone_error(error, get_stack) {
return object;
}

// TODO: Remove for 1.0
/** @param {Record<string, any>} mod */
export function check_method_names(mod) {
['get', 'post', 'put', 'patch', 'del'].forEach((m) => {
if (m in mod) {
const replacement = m === 'del' ? 'DELETE' : m.toUpperCase();
throw Error(`Endpoint method "${m}" has changed to "${replacement}"`);
}
});
}

/** @type {import('types').SSRErrorPage} */
export const GENERIC_ERROR = {
id: '__error'
Expand Down
22 changes: 8 additions & 14 deletions packages/kit/src/vite/build/build_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,13 @@ import { mkdirp, posixify } from '../../utils/filesystem.js';
import { get_vite_config, merge_vite_configs, resolve_entry } from '../utils.js';
import { load_template } from '../../core/config/index.js';
import { get_runtime_directory } from '../../core/utils.js';
import { create_build, find_deps, get_default_config, remove_svelte_kit } from './utils.js';
import {
create_build,
find_deps,
get_default_config,
remove_svelte_kit,
is_http_method
} from './utils.js';
import { s } from '../../utils/misc.js';

/**
Expand Down Expand Up @@ -255,16 +261,6 @@ export async function build_server(options, client) {
};
}

/** @type {Record<string, string>} */
const method_names = {
get: 'get',
head: 'head',
post: 'post',
put: 'put',
del: 'delete',
patch: 'patch'
};

/**
* @param {string} cwd
* @param {import('rollup').OutputChunk[]} output
Expand All @@ -285,9 +281,7 @@ function get_methods(cwd, output, manifest_data) {
const file = route.type === 'endpoint' ? route.file : route.shadow;

if (file && lookup[file]) {
methods[file] = lookup[file]
.map((x) => /** @type {import('types').HttpMethod} */ (method_names[x]))
.filter(Boolean);
methods[file] = lookup[file].filter(is_http_method);
}
});

Expand Down
11 changes: 11 additions & 0 deletions packages/kit/src/vite/build/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,3 +150,14 @@ export function remove_svelte_kit(config) {
.flat(Infinity)
.filter((plugin) => plugin.name !== 'vite-plugin-svelte-kit');
}

const method_names = new Set(['GET', 'HEAD', 'PUT', 'POST', 'DELETE', 'PATCH']);

// If we'd written this in TypeScript, it could be easy...
/**
* @param {string} str
* @returns {str is import('types').HttpMethod}
*/
export function is_http_method(str) {
return method_names.has(str);
}
2 changes: 1 addition & 1 deletion packages/kit/test/apps/amp/src/routes/origin/index.json.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/** @type {import('@sveltejs/kit').RequestHandler} */
export function get({ url }) {
export function GET({ url }) {
return {
body: { origin: url.origin }
};
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/test/apps/amp/src/routes/valid/index.json.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export function get() {
export function GET() {
return {
body: {
answer: 42
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/test/apps/basics/src/routes/answer.json.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export function get() {
export function GET() {
return {
body: {
answer: 42
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export function get() {
export function GET() {
return {
body: {
answer: 42
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/** @type {import('@sveltejs/kit').RequestHandler} */
export function del(req) {
export function DELETE(req) {
return {
status: 200,
body: {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export async function get() {
export async function GET() {
return {
body: {
fruit: '🍎🍇🍌'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/** @type {import('@sveltejs/kit').RequestHandler} */
export function get() {
export function GET() {
return { body: {} };
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/** @type {import('@sveltejs/kit').RequestHandler} */
export function get() {
export function GET() {
return {};
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export function get() {
export function GET() {
return {
headers: {
'x-foo': 'bar'
Expand Down
Loading

0 comments on commit 87552ef

Please sign in to comment.