Identity de-dupe with replacers in uneval

.changeset/fix-uneval-replacer-dedupe.md

@@ -0,0 +1,9 @@
+---
+'devalue': patch
+---
+
+fix: preserve shared-reference deduping when a custom `uneval` replacer calls
+the callback for nested values
+
+This can cause replacers to run twice for the same value, if the replacer calls
+the uneval callback passed to it.

src/uneval.js

@@ -26,6 +26,7 @@ export function uneval(value, replacer) {
 	/** @type {string[]} */
 	const keys = [];
 
+	/** @type {Map<any, string | undefined>} */
 	const custom = new Map();
 
 	/** @param {any} thing */
@@ -39,10 +40,18 @@ export function uneval(value, replacer) {
 			counts.set(thing, 1);
 
 			if (replacer) {
-				const str = replacer(thing, (value) => uneval(value, replacer));
+				let calledWalk = false;
+				const str = replacer(thing, (value) => {
+					calledWalk = true;
+					walk(value);
+					return '<placeholder>';
+				});
 
 				if (typeof str === 'string') {
-					custom.set(thing, str);
+					// If the replacer called walk, then we need to handle the possiblilty that inner values
+					// are seen multiple times, and may need to be deduped with a name. In that case run the
+					// replacer again during stringify.
+					custom.set(thing, calledWalk ? undefined : str);
 					return;
 				}
 			}
@@ -167,7 +176,18 @@ export function uneval(value, replacer) {
 		}
 
 		if (custom.has(thing)) {
-			return custom.get(thing);
+			const str = custom.get(thing);
+			if (str !== undefined) {
+				return str;
+			}
+			const finalStr = replacer(thing, (value) => stringify(value));
+			if (typeof finalStr === 'string') {
+				custom.set(thing, finalStr);
+				return finalStr;
+			}
+			throw new TypeError(
+				'Replacer must return a string when called on a value it previously returned a string for'
+			);
 		}
 
 		const type = get_type(thing);
@@ -367,8 +387,19 @@ export function uneval(value, replacer) {
 			params.push(name);
 
 			if (custom.has(thing)) {
-				values.push(/** @type {string} */ (custom.get(thing)));
-				return;
+				const str = custom.get(thing);
+				if (str !== undefined) {
+					values.push(str);
+					return;
+				}
+				const finalStr = replacer(thing, (value) => stringify(value));
+				if (typeof finalStr === 'string') {
+					values.push(finalStr);
+					return;
+				}
+				throw new TypeError(
+					'Replacer must return a string when called on a value it previously returned a string for'
+				);
 			}
 
 			if (is_primitive(thing)) {

test/index.test.js

@@ -1350,6 +1350,23 @@ uvu.test('does not create duplicate parameter names', () => {
 	eval(serialized);
 });
 
+uvu.test('replacer callback preserves shared references', () => {
+	const shared = { answer: 42 };
+	const value = [new Foo(shared), shared];
+
+	const serialized = uneval(value, (value, uneval) => {
+		if (value instanceof Foo) {
+			return `new Foo(${uneval(value.value)})`;
+		}
+	});
+
+	const roundtripped = eval(serialized);
+
+	assert.ok(roundtripped[0] instanceof Foo);
+	assert.is(roundtripped[0].value, roundtripped[1]);
+	assert.is(roundtripped[1].answer, 42);
+});
+
 uvu.test('rejects sparse array __proto__ pollution via parse', () => {
 	// Attempt to set __proto__ on an array via the sparse array encoding
 	const payload = JSON.stringify([[consts.SPARSE, 1, '__proto__', { polluted: true }]]);