[breaking] use devalue to (de)serialize action data (#7494)

Allows for Date objects and more be part of the response
Closes #7488

Author: dummdidumm

.changeset/cool-emus-drive.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+[breaking] use devalue to (de)serialize action data

documentation/docs/20-core-concepts/30-form-actions.md

@@ -389,7 +389,7 @@ We can also implement progressive enhancement ourselves, without `use:enhance`,
 /// file: src/routes/login/+page.svelte
 <script>
 	import { invalidateAll, goto } from '$app/navigation';
-	import { applyAction } from '$app/forms';
+	import { applyAction, deserialize } from '$app/forms';
 
 	/** @type {import('./$types').ActionData} */
 	export let form;
@@ -406,7 +406,7 @@ We can also implement progressive enhancement ourselves, without `use:enhance`,
 		});
 
 		/** @type {import('@sveltejs/kit').ActionResult} */
-		const result = await response.json();
+		const result = deserialize(await response.text());
 
 		if (result.type === 'success') {
 			// re-run all `load` functions, following the successful update
@@ -422,6 +422,8 @@ We can also implement progressive enhancement ourselves, without `use:enhance`,
 </form>
 ```
 
+Note that you need to `deserialize` the response before processing it further using the corresponding method from `$app/forms`. `JSON.parse()` isn't enough because form actions - like `load` functions - also support returning `Date` or `BigInt` objects.
+
 If you have a `+server.js` alongside your `+page.server.js`, `fetch` requests will be routed there by default. To `POST` to an action in `+page.server.js` instead, use the custom `x-sveltekit-action` header:
 
 ```diff

packages/kit/src/runtime/app/forms.js

@@ -1,5 +1,6 @@
-import { invalidateAll } from './navigation.js';
+import * as devalue from 'devalue';
 import { client } from '../client/singletons.js';
+import { invalidateAll } from './navigation.js';
 
 /**
  * @param {string} name
@@ -15,6 +16,15 @@ const ssr = import.meta.env.SSR;
 /** @type {import('$app/forms').applyAction} */
 export const applyAction = ssr ? guard('applyAction') : client.apply_action;
 
+/** @type {import('$app/forms').deserialize} */
+export function deserialize(result) {
+	const parsed = JSON.parse(result);
+	if (parsed.data) {
+		parsed.data = devalue.parse(parsed.data);
+	}
+	return parsed;
+}
+
 /** @type {import('$app/forms').enhance} */
 export function enhance(form, submit = () => {}) {
 	/**
@@ -93,7 +103,7 @@ export function enhance(form, submit = () => {}) {
 				signal: controller.signal
 			});
 
-			result = await response.json();
+			result = deserialize(await response.text());
 		} catch (error) {
 			if (/** @type {any} */ (error)?.name === 'AbortError') return;
 			result = { type: 'error', error };

packages/kit/src/runtime/server/page/actions.js

@@ -1,3 +1,4 @@
+import * as devalue from 'devalue';
 import { error, json } from '../../../exports/index.js';
 import { normalize_error } from '../../../utils/error.js';
 import { is_form_content_type, negotiate } from '../../../utils/http.js';
@@ -41,14 +42,20 @@ export async function handle_action_json_request(event, options, server) {
 		const data = await call_action(event, actions);
 
 		if (data instanceof ValidationError) {
-			check_serializability(data.data, /** @type {string} */ (event.route.id), 'data');
-			return action_json({ type: 'invalid', status: data.status, data: data.data });
+			return action_json({
+				type: 'invalid',
+				status: data.status,
+				// @ts-expect-error we assign a string to what is supposed to be an object. That's ok
+				// because we don't use the object outside, and this way we have better code navigation
+				// through knowing where the related interface is used.
+				data: stringify_action_response(data.data, /** @type {string} */ (event.route.id))
+			});
 		} else {
-			check_serializability(data, /** @type {string} */ (event.route.id), 'data');
 			return action_json({
 				type: 'success',
 				status: data ? 200 : 204,
-				data: /** @type {Record<string, any> | undefined} */ (data)
+				// @ts-expect-error see comment above
+				data: stringify_action_response(data, /** @type {string} */ (event.route.id))
 			});
 		}
 	} catch (e) {
@@ -211,46 +218,41 @@ function maybe_throw_migration_error(server) {
 }
 
 /**
- * Check that the data can safely be serialized to JSON
- * @param {any} value
- * @param {string} id
- * @param {string} path
+ * Try to `devalue.uneval` the data object, and if it fails, return a proper Error with context
+ * @param {any} data
+ * @param {string} route_id
  */
-function check_serializability(value, id, path) {
-	const type = typeof value;
+export function uneval_action_response(data, route_id) {
+	return try_deserialize(data, devalue.uneval, route_id);
+}
 
-	if (type === 'string' || type === 'boolean' || type === 'number' || type === 'undefined') {
-		// primitives are fine
-		return;
-	}
+/**
+ * Try to `devalue.stringify` the data object, and if it fails, return a proper Error with context
+ * @param {any} data
+ * @param {string} route_id
+ */
+function stringify_action_response(data, route_id) {
+	return try_deserialize(data, devalue.stringify, route_id);
+}
 
-	if (type === 'object') {
-		// nulls are fine...
-		if (!value) return;
+/**
+ * @param {any} data
+ * @param {(data: any) => string} fn
+ * @param {string} route_id
+ */
+function try_deserialize(data, fn, route_id) {
+	try {
+		return fn(data);
+	} catch (e) {
+		// If we're here, the data could not be serialized with devalue
+		const error = /** @type {any} */ (e);
 
-		// ...so are plain arrays...
-		if (Array.isArray(value)) {
-			value.forEach((child, i) => {
-				check_serializability(child, id, `${path}[${i}]`);
-			});
-			return;
+		if ('path' in error) {
+			let message = `Data returned from action inside ${route_id} is not serializable: ${error.message}`;
+			if (error.path !== '') message += ` (data.${error.path})`;
+			throw new Error(message);
 		}
 
-		// ...and objects
-		// This simple check might potentially run into some weird edge cases
-		// Refer to https://github.com/lodash/lodash/blob/2da024c3b4f9947a48517639de7560457cd4ec6c/isPlainObject.js?rgh-link-date=2022-07-20T12%3A48%3A07Z#L30
-		// if that ever happens
-		if (Object.getPrototypeOf(value) === Object.prototype) {
-			for (const key in value) {
-				check_serializability(value[key], id, `${path}.${key}`);
-			}
-			return;
-		}
+		throw error;
 	}
-
-	throw new Error(
-		`${path} returned from action in ${id} cannot be serialized as JSON without losing its original type` +
-			// probably the most common case, so let's give a hint
-			(value instanceof Date ? ' (Date objects are serialized as strings)' : '')
-	);
 }

packages/kit/src/runtime/server/page/render.js

@@ -4,6 +4,7 @@ import { hash } from '../../hash.js';
 import { serialize_data } from './serialize_data.js';
 import { s } from '../../../utils/misc.js';
 import { Csp } from './csp.js';
+import { uneval_action_response } from './actions.js';
 import { clarify_devalue_error } from '../utils.js';
 
 // TODO rename this function/module
@@ -201,8 +202,7 @@ export async function render_response({
 	}
 
 	if (form_value) {
-		// no need to check it can be serialized, we already verified that it's JSON-friendly
-		serialized.form = devalue.uneval(form_value);
+		serialized.form = uneval_action_response(form_value, /** @type {string} */ (event.route.id));
 	}
 
 	if (inline_styles.size > 0) {

packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte

@@ -1,4 +1,5 @@
 <script>
+	import { deserialize } from '$app/forms';
 	import { browser } from '$app/environment';
 
 	/** @type {import('./$types').ActionData} */
@@ -14,10 +15,9 @@
 				accept: 'application/json'
 			}
 		});
-		const {
-			data: { errors, values }
-		} = await res.json();
-		form = { errors, values };
+		// @ts-expect-error don't bother with type narrowing work here
+		const { data } = deserialize(await res.text());
+		form = data;
 	}
 </script>
 

packages/kit/test/apps/basics/src/routes/actions/success-data/+page.svelte

@@ -1,18 +1,22 @@
 <script>
+	import { deserialize } from '$app/forms';
+
 	/** @type {import('./$types').ActionData} */
 	export let form;
 
 	async function submit({ submitter }) {
 		const res = await fetch(this.action, {
 			method: 'POST',
-			body: submitter.getAttribute('formenctype') === 'multipart/form-data'
-				? new FormData(this)
-				: new URLSearchParams({ username: this['username'].value }),
+			body:
+				submitter.getAttribute('formenctype') === 'multipart/form-data'
+					? new FormData(this)
+					: new URLSearchParams({ username: this['username'].value }),
 			headers: {
 				accept: 'application/json'
 			}
 		});
-		const { data } = await res.json();
+		// @ts-expect-error don't bother with type narrowing work here
+		const { data } = deserialize(await res.text());
 		form = data;
 	}
 </script>

packages/kit/test/apps/basics/test/server.test.js

@@ -360,7 +360,7 @@ test.describe('Shadowed pages', () => {
 		});
 
 		expect(response.status()).toBe(200);
-		expect(await response.json()).toEqual({ type: 'success', status: 204 });
+		expect(await response.json()).toEqual({ data: '-1', type: 'success', status: 204 });
 	});
 });
 

packages/kit/types/ambient.d.ts

@@ -157,6 +157,19 @@ declare module '$app/forms' {
 		Success extends Record<string, unknown> | undefined = Record<string, any>,
 		Invalid extends Record<string, unknown> | undefined = Record<string, any>
 	>(result: ActionResult<Success, Invalid>): Promise<void>;
+
+	/**
+	 * Use this function to deserialize the response from a form submission.
+	 * Usage:
+	 * ```
+	 * const res = await fetch('/form?/action', { method: 'POST', body: formData });
+	 * const result = deserialize(await res.text());
+	 * ```
+	 */
+	export function deserialize<
+		Success extends Record<string, unknown> | undefined = Record<string, any>,
+		Invalid extends Record<string, unknown> | undefined = Record<string, any>
+	>(serialized: string): ActionResult<Success, Invalid>;
 }
 
 /**