Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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/light-lamps-crash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

[breaking] throw an error on invalid load response
17 changes: 15 additions & 2 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -673,10 +673,23 @@ export function create_client({ target, base }) {
}
});

if (import.meta.env.DEV) {
if (__SVELTEKIT_DEV__) {
Comment thread
Rich-Harris marked this conversation as resolved.
try {
lock_fetch();
data = (await node.shared.load.call(null, load_input)) ?? null;
if (data != null && Object.getPrototypeOf(data) !== Object.prototype) {
throw new Error(
`a load function related to route '${route.id}' returned ${
typeof data !== 'object'
? `a ${typeof data}`
: data instanceof Response
? 'a Response object'
: Array.isArray(data)
? 'an array'
: 'a non-plain object'
}, but must return a plain object at the top level (i.e. \`return {...}\`)`
);
}
Comment thread
Rich-Harris marked this conversation as resolved.
} finally {
unlock_fetch();
}
Expand Down Expand Up @@ -1298,7 +1311,7 @@ export function create_client({ target, base }) {
},

disable_scroll_handling: () => {
if (import.meta.env.DEV && started && !updating) {
if (__SVELTEKIT_DEV__ && started && !updating) {
throw new Error('Can only disable scroll handling during navigation');
}

Expand Down
1 change: 1 addition & 0 deletions packages/kit/src/runtime/server/data/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export async function render_data(
const node = n == undefined ? n : await options.manifest._.nodes[n]();
return load_server_data({
event: new_event,
options,
state,
node,
parent: async () => {
Expand Down
1 change: 1 addition & 0 deletions packages/kit/src/runtime/server/page/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ export async function render_page(event, route, page, options, state, resolve_op

return await load_server_data({
event,
options,
state,
node,
parent: async () => {
Expand Down
33 changes: 29 additions & 4 deletions packages/kit/src/runtime/server/page/load_data.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@ import { unwrap_promises } from '../../../utils/promises.js';
* Calls the user's server `load` function.
* @param {{
* event: import('types').RequestEvent;
* options: import('types').SSROptions;
* state: import('types').SSRState;
* node: import('types').SSRNode | undefined;
* parent: () => Promise<Record<string, any>>;
* }} opts
* @returns {Promise<import('types').ServerDataNode | null>}
*/
export async function load_server_data({ event, state, node, parent }) {
export async function load_server_data({ event, options, state, node, parent }) {
if (!node?.server) return null;

const uses = {
Expand Down Expand Up @@ -59,6 +60,9 @@ export async function load_server_data({ event, state, node, parent }) {
});

const data = result ? await unwrap_promises(result) : null;
if (options.dev) {
validate_load_response(data, /** @type {string} */ (event.route.id));
}

return {
type: 'data',
Expand Down Expand Up @@ -236,9 +240,10 @@ export async function load_data({
}
});

const data = await node.shared.load.call(null, load_event);

return data ? unwrap_promises(data) : null;
const result = await node.shared.load.call(null, load_event);
const data = result ? await unwrap_promises(result) : null;
validate_load_response(data, /** @type {string} */ (event.route.id));
return data;
}

/**
Expand All @@ -257,3 +262,23 @@ async function stream_to_string(stream) {
}
return result;
}

/**
* @param {any} data
* @param {string} [routeId]
*/
function validate_load_response(data, routeId) {
if (data != null && Object.getPrototypeOf(data) !== Object.prototype) {
throw new Error(
`a load function related to route '${routeId}' returned ${
typeof data !== 'object'
? `a ${typeof data}`
: data instanceof Response
? 'a Response object'
: Array.isArray(data)
? 'an array'
: 'a non-plain object'
}, but must return a plain object at the top level (i.e. \`return {...}\`)`
);
}
}
1 change: 1 addition & 0 deletions packages/kit/src/runtime/server/page/respond_with_error.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export async function respond_with_error({ event, options, state, status, error,

const server_data_promise = load_server_data({
event,
options,
state,
node: default_layout,
parent: async () => ({})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function load() {
return ['nope'];
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<!-- this will never be rendered, load function returns something invalid -->
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function load() {
return ['nope'];
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<!-- this will never be rendered, load function returns something invalid -->
32 changes: 32 additions & 0 deletions packages/kit/test/apps/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,38 @@ test.describe('Errors', () => {
'This is your custom error page saying: "Crashing now"'
);
});

test('errors on invalid load function response', async ({ page, app, javaScriptEnabled }) => {
if (javaScriptEnabled) {
await page.goto('/');
await app.goto('/errors/invalid-load-response');
} else {
await page.goto('/errors/invalid-load-response');
}

expect(await page.textContent('footer')).toBe('Custom layout');
expect(await page.textContent('#message')).toBe(
"This is your custom error page saying: \"a load function related to route '/errors/invalid-load-response' returned an array, but must return a plain object at the top level (i.e. `return {...}`)\""
);
});

test('errors on invalid server load function response', async ({
page,
app,
javaScriptEnabled
}) => {
if (javaScriptEnabled) {
await page.goto('/');
await app.goto('/errors/invalid-server-load-response');
} else {
await page.goto('/errors/invalid-server-load-response');
}

expect(await page.textContent('footer')).toBe('Custom layout');
expect(await page.textContent('#message')).toBe(
"This is your custom error page saying: \"a load function related to route '/errors/invalid-server-load-response' returned an array, but must return a plain object at the top level (i.e. `return {...}`)\""
);
});
}

test('server-side load errors', async ({ page }) => {
Expand Down