fix: ensure $state.snapshot never errors (#12445)

* fix: ensure `$state.snapshot` never errors

Snapshotting can error on un-cloneable objects. It's not practical to error in this case; often there's no way out of this for users, so it makes sense to return the original value in that case, and warn in dev mode about it.

closes #12438

* lint

* single warning per snapshot call, list affected properties

* lint

* lint

---------

Co-authored-by: Rich Harris <[email protected]>

Author: dummdidumm

.changeset/brave-gorillas-fold.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: ensure `$state.snapshot` never errors

packages/svelte/messages/shared-warnings/warnings.md

@@ -1,3 +1,11 @@
 ## dynamic_void_element_content
 
 > `<svelte:element this="%tag%">` is a void element — it cannot have content
+
+## state_snapshot_uncloneable
+
+> Value cannot be cloned with `$state.snapshot` — the original value was returned
+
+> The following properties cannot be cloned with `$state.snapshot` — the return value contains the originals:
+>
+> %properties%

packages/svelte/src/internal/shared/clone.js

@@ -1,22 +1,56 @@
 /** @import { Snapshot } from './types' */
+import { DEV } from 'esm-env';
+import * as w from './warnings.js';
 import { get_prototype_of, is_array, object_prototype } from './utils.js';
 
+/**
+ * In dev, we keep track of which properties could not be cloned. In prod
+ * we don't bother, but we keep a dummy array around so that the
+ * signature stays the same
+ * @type {string[]}
+ */
+const empty = [];
+
 /**
  * @template T
  * @param {T} value
  * @returns {Snapshot<T>}
  */
 export function snapshot(value) {
-	return clone(value, new Map());
+	if (DEV) {
+		/** @type {string[]} */
+		const paths = [];
+
+		const copy = clone(value, new Map(), '', paths);
+		if (paths.length === 1 && paths[0] === '') {
+			// value could not be cloned
+			w.state_snapshot_uncloneable();
+		} else if (paths.length > 0) {
+			// some properties could not be cloned
+			const slice = paths.length > 10 ? paths.slice(0, 7) : paths.slice(0, 10);
+			const excess = paths.length - slice.length;
+
+			let uncloned = slice.map((path) => `- <value>${path}`).join('\n');
+			if (excess > 0) uncloned += `\n- ...and ${excess} more`;
+
+			w.state_snapshot_uncloneable(uncloned);
+		}
+
+		return copy;
+	}
+
+	return clone(value, new Map(), '', empty);
 }
 
 /**
  * @template T
  * @param {T} value
  * @param {Map<T, Snapshot<T>>} cloned
+ * @param {string} path
+ * @param {string[]} paths
  * @returns {Snapshot<T>}
  */
-function clone(value, cloned) {
+function clone(value, cloned, path, paths) {
 	if (typeof value === 'object' && value !== null) {
 		const unwrapped = cloned.get(value);
 		if (unwrapped !== undefined) return unwrapped;
@@ -25,8 +59,8 @@ function clone(value, cloned) {
 			const copy = /** @type {Snapshot<any>} */ ([]);
 			cloned.set(value, copy);
 
-			for (const element of value) {
-				copy.push(clone(element, cloned));
+			for (let i = 0; i < value.length; i += 1) {
+				copy.push(clone(value[i], cloned, DEV ? `${path}[${i}]` : path, paths));
 			}
 
 			return copy;
@@ -39,16 +73,34 @@ function clone(value, cloned) {
 
 			for (var key in value) {
 				// @ts-expect-error
-				copy[key] = clone(value[key], cloned);
+				copy[key] = clone(value[key], cloned, DEV ? `${path}.${key}` : path, paths);
 			}
 
 			return copy;
 		}
 
 		if (typeof (/** @type {T & { toJSON?: any } } */ (value).toJSON) === 'function') {
-			return clone(/** @type {T & { toJSON(): any } } */ (value).toJSON(), cloned);
+			return clone(
+				/** @type {T & { toJSON(): any } } */ (value).toJSON(),
+				cloned,
+				DEV ? `${path}.toJSON()` : path,
+				paths
+			);
 		}
 	}
 
-	return /** @type {Snapshot<T>} */ (structuredClone(value));
+	if (value instanceof EventTarget) {
+		// can't be cloned
+		return /** @type {Snapshot<T>} */ (value);
+	}
+
+	try {
+		return /** @type {Snapshot<T>} */ (structuredClone(value));
+	} catch (e) {
+		if (DEV) {
+			paths.push(path);
+		}
+
+		return /** @type {Snapshot<T>} */ (value);
+	}
 }

packages/svelte/src/internal/shared/clone.test.ts

@@ -2,6 +2,22 @@ import { snapshot } from './clone';
 import { assert, test } from 'vitest';
 import { proxy } from '../client/proxy';
 
+function capture_warnings() {
+	const warnings: string[] = [];
+
+	// eslint-disable-next-line no-console
+	const warn = console.warn;
+
+	// eslint-disable-next-line no-console
+	console.warn = (message) => warnings.push(message);
+
+	return () => {
+		// eslint-disable-next-line no-console
+		console.warn = warn;
+		return warnings;
+	};
+}
+
 test('primitive', () => {
 	assert.equal(42, snapshot(42));
 });
@@ -101,3 +117,70 @@ test('reactive class', () => {
 
 	assert.equal(copy.get(1), 2);
 });
+
+test('uncloneable value', () => {
+	const fn = () => {};
+
+	const warnings = capture_warnings();
+	const copy = snapshot(fn);
+
+	assert.equal(fn, copy);
+	assert.deepEqual(warnings(), [
+		'%c[svelte] state_snapshot_uncloneable\n%cValue cannot be cloned with `$state.snapshot` — the original value was returned'
+	]);
+});
+
+test('uncloneable properties', () => {
+	const object = {
+		a: () => {},
+		b: () => {},
+		c: [() => {}, () => {}, () => {}, () => {}, () => {}, () => {}, () => {}, () => {}]
+	};
+
+	const warnings = capture_warnings();
+	const copy = snapshot(object);
+
+	assert.notEqual(object, copy);
+	assert.equal(object.a, copy.a);
+	assert.equal(object.b, copy.b);
+
+	assert.notEqual(object.c, copy.c);
+	assert.equal(object.c[0], copy.c[0]);
+
+	assert.deepEqual(warnings(), [
+		`%c[svelte] state_snapshot_uncloneable
+%cThe following properties cannot be cloned with \`$state.snapshot\` — the return value contains the originals:
+
+- <value>.a
+- <value>.b
+- <value>.c[0]
+- <value>.c[1]
+- <value>.c[2]
+- <value>.c[3]
+- <value>.c[4]
+- <value>.c[5]
+- <value>.c[6]
+- <value>.c[7]`
+	]);
+});
+
+test('many uncloneable properties', () => {
+	const array = Array.from({ length: 100 }, () => () => {});
+
+	const warnings = capture_warnings();
+	snapshot(array);
+
+	assert.deepEqual(warnings(), [
+		`%c[svelte] state_snapshot_uncloneable
+%cThe following properties cannot be cloned with \`$state.snapshot\` — the return value contains the originals:
+
+- <value>[0]
+- <value>[1]
+- <value>[2]
+- <value>[3]
+- <value>[4]
+- <value>[5]
+- <value>[6]
+- ...and 93 more`
+	]);
+});

packages/svelte/src/internal/shared/warnings.js

@@ -16,4 +16,23 @@ export function dynamic_void_element_content(tag) {
 		// TODO print a link to the documentation
 		console.warn("dynamic_void_element_content");
 	}
+}
+
+/**
+ * The following properties cannot be cloned with `$state.snapshot` — the return value contains the originals:
+ * 
+ * %properties%
+ * @param {string | undefined | null} [properties]
+ */
+export function state_snapshot_uncloneable(properties) {
+	if (DEV) {
+		console.warn(`%c[svelte] state_snapshot_uncloneable\n%c${properties
+			? `The following properties cannot be cloned with \`$state.snapshot\` — the return value contains the originals:
+
+${properties}`
+			: "Value cannot be cloned with `$state.snapshot` — the original value was returned"}`, bold, normal);
+	} else {
+		// TODO print a link to the documentation
+		console.warn("state_snapshot_uncloneable");
+	}
 }