[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>;
 }
 
 /**