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

feat: uppercase endpoint methods #5513

Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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/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
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
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: 14 additions & 7 deletions packages/kit/src/runtime/server/endpoint.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,22 +45,29 @@ export function is_text(content_type) {
export async function render_endpoint(event, mod, options) {
const method = normalize_request_method(event);
elliott-with-the-longest-name-on-github marked this conversation as resolved.
Show resolved Hide resolved

// TODO: Remove for 1.0
['get', 'post', 'put', 'patch', 'del'].forEach((m) => {
if (mod[m] !== undefined) {
const replacement = m === 'del' ? 'DELETE' : m.toUpperCase();
throw Error(`Endpoint method "${m}" has changed to "${replacement}"`);
}
});

/** @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,
Expand Down Expand Up @@ -132,7 +139,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
8 changes: 4 additions & 4 deletions packages/kit/src/runtime/server/page/load_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -409,13 +409,13 @@ async function load_shadow_data(route, event, options, prerender) {
try {
const mod = await route.shadow();

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// TODO: Remove for 1.0
['get', 'post', 'put', 'patch', 'del'].forEach((m) => {
if (mod[m] !== undefined) {
const replacement = m === 'del' ? 'DELETE' : m.toUpperCase();
throw Error(`Endpoint method "${m}" has changed to "${replacement}"`);
}
});

The error should be here as well?

Copy link
Member

Choose a reason for hiding this comment

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

ah yep, i'll rustle up a utility that can be used in both places

if (prerender && (mod.post || mod.put || mod.del || mod.patch)) {
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);
elliott-with-the-longest-name-on-github marked this conversation as resolved.
Show resolved Hide resolved
const is_get = method === 'head' || method === 'get';
const handler = method === 'head' ? mod.head || mod.get : mod[method];
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 +462,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
3 changes: 1 addition & 2 deletions packages/kit/src/runtime/server/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ export function is_pojo(body) {

/** @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
return event.request.method.toUpperCase();
}

elliott-with-the-longest-name-on-github marked this conversation as resolved.
Show resolved Hide resolved
/**
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,
isHttpMethod
elliott-with-the-longest-name-on-github marked this conversation as resolved.
Show resolved Hide resolved
} 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(isHttpMethod);
elliott-with-the-longest-name-on-github marked this conversation as resolved.
Show resolved Hide resolved
}
});

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');
}

// If we'd written this in TypeScript, it could be easy...
/**
* @param {string} maybeHttpMethod
* @returns {maybeHttpMethod is import('types').HttpMethod}
*/
export function isHttpMethod(maybeHttpMethod) {
const method_names = ['GET', 'HEAD', 'PUT', 'POST', 'DELETE', 'PATCH'];

return method_names.includes(maybeHttpMethod);
}
elliott-with-the-longest-name-on-github marked this conversation as resolved.
Show resolved Hide resolved
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
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/** @type {import('@sveltejs/kit').RequestHandler} */
export function get() {
export function GET() {
return {
headers: new Headers({
'X-Foo': 'bar'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/** @type {import('@sveltejs/kit').RequestHandler} */
export function get() {
export function GET() {
return {
headers: {
'Set-Cookie': 'foo=bar'
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: null };
}
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
/** @type {import('@sveltejs/kit').RequestHandler} */
export const get = ({ url }) => fetch(`http://localhost:${url.searchParams.get('port')}`);
export const GET = ({ url }) => fetch(`http://localhost:${url.searchParams.get('port')}`);
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 get() {
export function GET() {
return {
headers: {
'content-type': 'application/octet-stream'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/** @type {import('@sveltejs/kit').RequestHandler} */
export const get = () => {
export const GET = () => {
const body = '<foo />';
return {
status: 200,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/** @type {import('@sveltejs/kit').RequestHandler} */
export const get = () => {
export const GET = () => {
const body = '<foo />';
return { status: 200, headers: { 'content-type': 'application/xml' }, body };
};
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/** @type {import('@sveltejs/kit').RequestHandler} */
export function get() {
export function GET() {
return {
status: 555
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/** @type {import('@sveltejs/kit').RequestHandler} */
export function get() {
export function GET() {
return {
status: 555
};
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() {
throw new Error('nope');
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
export function get() {
export function GET() {
throw new Error('nope');
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// @ts-expect-error
thisvariableisnotdefined; // eslint-disable-line

export function get() {
export function GET() {
return {
body: {
answer: 42
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
export function get() {
export function GET() {
return 'this ought to be an object';
}
Loading