fix(sandbox.cement): fix recursive cementing

Cementing wasn't applied recursively due to a combination of bad detection and bad recursion

.vscode/launch.json

@@ -49,7 +49,7 @@
             "request": "launch",
             "name": "Run Sandbox Tests",
             "runtimeExecutable": "pnpm",
-            "runtimeArgs": ["--prefix","./packages/sandbox", "test", "--timeout", "60000", "--", "--grep", "\"should return the original object if it is a sandbox proxy\""],
+            "runtimeArgs": ["--prefix","./packages/sandbox", "test", "--", "--timeout", "60000", "--grep", "\"should call transform and predicate the correct\""],
             "internalConsoleOptions": "openOnSessionStart",
             "cwd": "${workspaceFolder}",
             "outputCapture": "std"

packages/sandbox/_test/type-tests.test.ts

@@ -81,7 +81,7 @@ describe("Integration type tests", function()
 			root: false,
 			properties: {
 				all: false,
-				any: true,
+				any: false,
 				elligible: true,
 			},
 		});

packages/sandbox/lib/cement/_test/cement.test.ts

@@ -1,7 +1,10 @@
 import { expect } from "chai";
 import { cement } from "../cement";
 import { CONSTANTS } from "../../constants";
-import { isSandboxProxy, sandbox } from "../../sandbox";
+import {
+	containsSandboxProxy, isSandboxProxy, sandbox, 
+} from "../../sandbox";
+import structuredClone from "@ungap/structured-clone";
 
 describe("cement", function() 
 {
@@ -156,4 +159,34 @@ describe("cement", function()
 
 		expect(isSandboxProxy(result)).to.be.false;
 	});
+
+	it("should cement deeply nested objects, even if the parent object is not a sandbox", function()
+	{
+		const original = {
+			key1: "value1",
+			key2: {
+				key3: {
+					key4: {
+						key5: "unchanged",
+					},
+				},
+			},
+		};
+
+		const clone = structuredClone(original);
+
+		const sb = sandbox(clone.key2.key3.key4);
+		clone.key2.key3.key4 = sb;
+
+		clone.key2.key3.key4.key5 = "changed";
+
+		expect(clone.key2.key3.key4.key5).to.equal("changed");
+		expect(original.key2.key3.key4.key5).to.equal("unchanged");
+
+		const cemented = cement(clone);
+
+		expect(cemented.key2.key3.key4.key5).to.equal("changed");
+		expect(original.key2.key3.key4.key5).to.equal("unchanged");
+		expect(containsSandboxProxy(cemented)).to.be.false;
+	});
 });

packages/sandbox/lib/cement/cement.ts

@@ -1,10 +1,10 @@
 import { CONSTANTS, SANDBOX_VERIFIABLE_PROP_SYMBOL } from "../constants";
 import { frostClone, isFrostProxy } from "../frost";
 import { generateVerificationProperty, getVerificationValueFromObject } from "../frost/properties";
-import isValidObject from "../validity/is-valid-object";
-import type { SandboxMode } from "../sandbox";
-import { containsSandboxProxy, isSandboxProxy } from "../sandbox/is-sandbox-proxy";
+import { containsSandboxProxy, type SandboxMode } from "../sandbox";
 import structuredClone from "@ungap/structured-clone";
+import isElligibleForSandbox from "../validity/is-elligible-for-sandbox";
+import { sandboxTransform } from "../sandbox/sandbox-transform";
 
 /**
  * Finalizes the changes made in a sandbox proxy object and returns a new object with those changes applied.
@@ -16,39 +16,51 @@ import structuredClone from "@ungap/structured-clone";
  */
 export function cement<T extends object>(obj: T): T 
 {
-	if (isSandboxProxy(obj)) 
-	{
-		const {
-			original,
-			changes,
-			params: { mode },
-		} = obj[CONSTANTS.SANDBOX_SYMBOL];
-
-		const toModify = getModifiableObject(original, mode);
+	let finalResult = obj;
 
-		return applyChanges(toModify, changes);
+	// If the object is a sandbox proxy at the root level, apply the changes
+	const rootCemented = cementAtRoot(obj);
+	if (rootCemented !== undefined)
+	{
+		finalResult = rootCemented;
 	}
+	// If the object contains nested sandbox proxies, apply the changes recursively.
+	// We don't need to check for situations where there are non-sandbox-proxies between
+	// a sb-root and a sb-nested, because sb-root objects are recursively sandboxed, so
+	// any changes to nested objects will also be sandboxed if the root is sandboxed.
 	else if (containsSandboxProxy(obj))
 	{
-		// Loop through the object's properties and cement any nested sandbox proxies recursively
-		for (const key in obj) 
+		finalResult = sandboxTransform(obj, (val) => 
 		{
-			if (Object.prototype.hasOwnProperty.call(obj, key)) 
-			{
-				const val = obj[key];
-				if (Array.isArray(val) || isValidObject(val) && isSandboxProxy(val))
-				{
-					const cemented = cement(val);
-					obj[key] = cemented;
-				}
-			}
-		}
+			return cement(val);
+		}, isElligibleForSandbox);
+	}
+
+	return finalResult;
+}
 
-		return obj;
+function cementAtRoot<T extends object>(obj: T): T 
+{
+	let original: T, changes: Record<string | symbol, any>, mode: SandboxMode, isSandboxProxy = false;
+	try 
+	{
+		// If this errors, the object is not a sandbox proxy
+		const sbMetadata = obj[CONSTANTS.SANDBOX_SYMBOL];
+		original = sbMetadata.original;
+		changes = sbMetadata.changes;
+		mode = sbMetadata.params.mode;
+		isSandboxProxy = true;
 	}
-	else 
+	catch 
+	{
+		// Not really an error case, just means the object is not a sandbox proxy
+	}
+
+	if (isSandboxProxy) 
 	{
-		throw new Error("Cannot cement an object that is not a sandbox.");
+		const toModify = getModifiableObject(original, mode);
+
+		return applyChanges(toModify, changes);
 	}
 }
 
@@ -95,7 +107,7 @@ function applyChanges<T extends object>(target: any, changes: Record<string | sy
 			handleSet(target, key, newValue, targetIsFrost);
 		}
 		// If the value is a nested object, apply changes recursively
-		else if (isSandboxProxy(newValue)) 
+		else if (isElligibleForSandbox(newValue)) 
 		{
 			const cemented = cement(newValue);
 			handleSet(target, key, cemented, targetIsFrost);

packages/sandbox/lib/frost/frost.ts

@@ -80,7 +80,7 @@ export function defrost<T extends object>(obj: T): T
 	for (const key in root)
 	{
 		const innerValue = root[key];
-		if (isElligibleForSandbox<object>(innerValue))
+		if (isElligibleForSandbox(innerValue))
 		{
 			root[key] = defrost(innerValue);
 		}

packages/sandbox/lib/sandbox/_test/sandbox-transform.test.ts

@@ -0,0 +1,111 @@
+import { expect } from "chai";
+import sinon from "sinon";
+import { sandboxTransform } from "../sandbox-transform";
+
+describe("deepSandboxTransform", function() 
+{
+    interface TestObject {
+        [key: string]: any;
+    }
+
+    it("should apply transform to values that match the predicate", function() 
+    {
+    	const obj: TestObject = {
+    		a: 1,
+    		b: 2,
+    		c: 3, 
+    	};
+    	const transform = sinon.spy((val: any) => val * 2);
+    	const predicate = (val: any) => val > 1;
+
+    	const result = sandboxTransform(obj, transform, predicate);
+
+    	expect(result).to.deep.equal({
+    		a: 1,
+    		b: 4,
+    		c: 6, 
+    	});
+    	expect(transform.calledTwice).to.be.true;
+    	expect(transform.firstCall.calledWith(2)).to.be.true;
+    	expect(transform.secondCall.calledWith(3)).to.be.true;
+    });
+
+    it("should not apply transform to values that do not match the predicate", function() 
+    {
+    	const obj: TestObject = {
+    		a: 1,
+    		b: 2,
+    		c: 3, 
+    	};
+    	const transform = sinon.spy((val: any) => val * 2);
+    	const predicate = (val: any) => val > 3;
+
+    	const result = sandboxTransform(obj, transform, predicate);
+
+    	expect(result).to.deep.equal({
+    		a: 1,
+    		b: 2,
+    		c: 3, 
+    	});
+    	expect(transform.notCalled).to.be.true;
+    });
+
+    it("should apply transform to all values if no predicate is provided", function() 
+    {
+    	const obj: TestObject = {
+    		a: 1,
+    		b: 2,
+    		c: 3, 
+    	};
+    	const transform = sinon.spy((val: any) => val * 2);
+
+    	const result = sandboxTransform(obj, transform);
+
+    	expect(result).to.deep.equal({
+    		a: 2,
+    		b: 4,
+    		c: 6, 
+    	});
+    	expect(transform.calledThrice).to.be.true;
+    	expect(transform.firstCall.calledWith(1)).to.be.true;
+    	expect(transform.secondCall.calledWith(2)).to.be.true;
+    	expect(transform.thirdCall.calledWith(3)).to.be.true;
+    });
+
+    it("should not handle nested objects", function() 
+    {
+    	const obj: TestObject = {
+    		a: {
+    			d: 1, 
+    		},
+    		b: 2,
+    		c: 3, 
+    	};
+    	const transform = sinon.spy((val: any) => (typeof val === "number" ? val * 2 : val));
+    	const predicate = (val: any) => typeof val === "number";
+
+    	const result = sandboxTransform(obj, transform, predicate);
+
+    	expect(result).to.deep.equal({
+    		a: {
+    			d: 1, 
+    		},
+    		b: 4,
+    		c: 6, 
+    	});
+    	expect(transform.calledTwice).to.be.true;
+    	expect(transform.firstCall.calledWith(2)).to.be.true;
+    	expect(transform.secondCall.calledWith(3)).to.be.true;
+    });
+
+    it("should handle empty objects", function() 
+    {
+    	const obj: TestObject = {};
+    	const transform = sinon.spy((val: any) => val * 2);
+
+    	const result = sandboxTransform(obj, transform);
+
+    	expect(result).to.deep.equal({});
+    	expect(transform.notCalled).to.be.true;
+    });
+});

packages/sandbox/lib/sandbox/is-sandbox-proxy.ts

@@ -9,12 +9,12 @@ export function objectRootIsSandboxProxy(o?: any): boolean
 
 function objectPropertiesAreSandboxProxy(o?: any): SandboxProxyStatus<true>["properties"]
 {
-	const elligibleWithKeys = (obj: any) => isElligibleForSandbox<object>(obj) && Object.keys(obj).length > 0;
+	const elligibleWithKeys = (obj: any) => isElligibleForSandbox(obj) && Object.keys(obj).length > 0;
 
 	const rootElligibleWithKeys = elligibleWithKeys(o);
 
 	let allropertiesAreSandboxProxy = rootElligibleWithKeys;
-	let anyPropertiesAreSandboxProxy = rootElligibleWithKeys;
+	let anyPropertiesAreSandboxProxy = false;
 
 	if (rootElligibleWithKeys)
 	{

packages/sandbox/lib/sandbox/sandbox-transform.ts

@@ -0,0 +1,28 @@
+export function sandboxTransform<
+    T extends object, 
+>(obj: T, transform: (obj: any) => any, predicate?: (val: any) => boolean): T
+{
+	const transformValue = (val: any): any => 
+	{
+		if (!predicate || predicate?.(val)) 
+		{
+			return transform(val);
+		}
+		else 
+		{
+			return val;
+		}
+	};
+
+	for (const key in obj) 
+	{
+		if (Object.prototype.hasOwnProperty.call(obj, key)) 
+		{
+			const val = obj[key];
+			const transformed = transformValue(val);
+			obj[key] = transformed;
+		}
+	}
+
+	return obj;
+}

packages/sandbox/lib/sandbox/sandbox.ts

@@ -69,7 +69,7 @@ function createProxy<T extends object>(obj: T, proxyMap: WeakMap<T, T>, params:
 	for (const [key, value] of Object.entries(obj)) 
 	{
 		// Only proxify values which are objects, and only those which are plain objects (not special objects like Date, etc.)
-		if (isElligibleForSandbox<object>(value))
+		if (isElligibleForSandbox(value))
 		{
 			proxy[key] = createProxy(value, proxyMap, params);
 		}
@@ -101,7 +101,7 @@ function handleGet<T extends object>(target: T, prop: string | symbol, receiver:
 	}
 
 	const value = Reflect.get(target, prop, receiver);
-	if (isElligibleForSandbox<object>(value) && !isSandboxProxy(value)) 
+	if (isElligibleForSandbox(value) && !isSandboxProxy(value)) 
 	{
 		if (!proxyMap.has(value)) 
 		{

packages/sandbox/lib/validity/is-elligible-for-sandbox.ts

@@ -1,6 +1,9 @@
 import isValidObject from "./is-valid-object";
+import type { AtLeastOneItemOfType } from "./types";
 
-export default function isElligibleForSandbox<T>(value: any): value is T 
+type ElligibleForSandbox = object | AtLeastOneItemOfType<object>;
+
+export default function isElligibleForSandbox(value: any): value is ElligibleForSandbox
 {
 	return Array.isArray(value) || isValidObject(value);
 }

packages/sandbox/lib/validity/types.ts

@@ -0,0 +1,5 @@
+export type AtLeastOnePropertyOfType<T, K extends keyof T, V> = {
+    [P in K]: T[P] extends V ? P : never;
+}[K];
+
+export type AtLeastOneItemOfType<T> = [T, ...any[]] | [...any[], T];