Skip to content

Commit

Permalink
check if data is serializable (#5987)
Browse files Browse the repository at this point in the history
* check data is serializable

* ok we do need a separate pass after all, much simpler that way

* update test

* only test serializability in dev

* Update packages/kit/src/runtime/server/index.js
  • Loading branch information
Rich-Harris authored Aug 18, 2022
1 parent a5f0506 commit 566776a
Show file tree
Hide file tree
Showing 11 changed files with 84 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/shiny-scissors-listen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

Check that data is serializable
1 change: 1 addition & 0 deletions packages/kit/src/runtime/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ export async function respond(request, options, state) {
return {
// TODO return `uses`, so we can reuse server data effectively
data: await load_server_data({
dev: options.dev,
event,
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 @@ -161,6 +161,7 @@ export async function render_page(event, route, options, state, resolve_opts) {
}

return await load_server_data({
dev: options.dev,
event,
node,
parent: async () => {
Expand Down
50 changes: 48 additions & 2 deletions packages/kit/src/runtime/server/page/load_data.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ import { LoadURL, PrerenderingURL } from '../../../utils/url.js';
/**
* Calls the user's `load` function.
* @param {{
* dev: boolean;
* event: import('types').RequestEvent;
* node: import('types').SSRNode | undefined;
* parent: () => Promise<Record<string, any>>;
* }} opts
*/
export async function load_server_data({ event, node, parent }) {
export async function load_server_data({ dev, event, node, parent }) {
if (!node?.server) return null;

const server_data = await node.server.load?.call(null, {
Expand All @@ -27,7 +28,13 @@ export async function load_server_data({ event, node, parent }) {
url: event.url
});

return server_data ? unwrap_promises(server_data) : null;
const result = server_data ? await unwrap_promises(server_data) : null;

if (dev) {
check_serializability(result, /** @type {string} */ (node.server_id), 'data');
}

return result;
}

/**
Expand Down Expand Up @@ -79,3 +86,42 @@ async function unwrap_promises(object) {

return unwrapped;
}

/**
* Check that the data can safely be serialized to JSON
* @param {any} value
* @param {string} id
* @param {string} path
*/
function check_serializability(value, id, path) {
const type = typeof value;

if (type === 'string' || type === 'boolean' || type === 'number' || type === 'undefined') {
// primitives are fine
return;
}

if (type === 'object') {
// nulls are fine...
if (!value) return;

// ...so are plain arrays...
if (Array.isArray(value)) {
value.forEach((child, i) => {
check_serializability(child, id, `${path}[${i}]`);
});
return;
}

// ...and objects
const tag = Object.prototype.toString.call(value);
if (tag === '[object Object]') {
for (const key in value) {
check_serializability(value[key], id, `${path}.${key}`);
}
return;
}
}

throw new Error(`${path} returned from 'load' in ${id} cannot be serialized as JSON`);
}
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 @@ -35,6 +35,7 @@ export async function respond_with_error({ event, options, state, status, error,
const default_layout = await options.manifest._.nodes[0](); // 0 is always the root layout

const server_data_promise = load_server_data({
dev: options.dev,
event,
node: default_layout,
parent: async () => ({})
Expand Down
1 change: 1 addition & 0 deletions packages/kit/src/vite/dev/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ export async function dev(vite, vite_config, svelte_config, illegal_imports) {
if (node.server) {
const { module } = await resolve(node.server);
result.server = module;
result.server_id = node.server;
}

// in dev we inline all styles to avoid FOUC. this gets populated lazily so that
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<a href="/shadowed/no-get">no-get</a>
<a href="/shadowed/dynamic/foo">dynamic/foo</a>
<a href="/shadowed/missing-get">missing-get</a>
<a href="/shadowed/serialization">serialization</a>

<form action="/shadowed/redirect-post" method="post">
<button type="submit" id="redirect-post">redirect</button>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export function load() {
return {
regex: /nope/
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<script>
/** @type {import('./$types').PageData} */
export let data;
</script>

<h1>{data.regex.test('nope')}</h1>
12 changes: 12 additions & 0 deletions packages/kit/test/apps/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,18 @@ test.describe('Shadowed pages', () => {
'Page data: {"sub":"sub","data":{"rootlayout":"rootlayout","layout":"layout"}}'
);
});

if (process.env.DEV) {
test('Data must be serializable', async ({ page, clicknav }) => {
await page.goto('/shadowed');
await clicknav('[href="/shadowed/serialization"]');

expect(await page.textContent('h1')).toBe('500');
expect(await page.textContent('#message')).toBe(
'This is your custom error page saying: "data.regex returned from \'load\' in src/routes/shadowed/serialization/+page.server.js cannot be serialized as JSON"'
);
});
}
});

test.describe('Encoded paths', () => {
Expand Down
3 changes: 3 additions & 0 deletions packages/kit/types/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,9 @@ export interface SSRNode {
PUT?: Action;
DELETE?: Action;
};

// store this in dev so we can print serialization errors
server_id?: string;
}

export type SSRNodeLoader = () => Promise<SSRNode>;
Expand Down

0 comments on commit 566776a

Please sign in to comment.