Manage circular references in objects

.github/workflows/main.yml

@@ -10,12 +10,11 @@ jobs:
       fail-fast: false
       matrix:
         node-version:
-          - 16
-          - 14
-          - 12
+          - 20
+          - 18
     steps:
-      - uses: actions/checkout@v2
-      - uses: actions/setup-node@v2
+      - uses: actions/checkout@v4
+      - uses: actions/setup-node@v4
         with:
           node-version: ${{ matrix.node-version }}
       - run: npm install

index.d.ts

@@ -1,4 +1,4 @@
-export interface Options {
+export type Options = {
 	/**
 	Deep changes will not trigger the callback. Only changes to the immediate properties of the original object.
 
@@ -123,9 +123,9 @@ export interface Options {
 		previousValue: unknown,
 		applyData: ApplyData
 	) => boolean;
-}
+};
 
-export interface ApplyData {
+export type ApplyData = {
 	/**
 	The name of the method that produced the change.
 	*/
@@ -140,7 +140,7 @@ export interface ApplyData {
 	The result returned from the method that produced the change.
 	*/
 	readonly result: unknown;
-}
+};
 
 declare const onChange: {
 	/**

index.js

@@ -1,7 +1,8 @@
 /* eslint-disable unicorn/prefer-spread */
-import {TARGET, UNSUBSCRIBE} from './lib/constants.js';
+import {TARGET, UNSUBSCRIBE, PATH_SEPARATOR} from './lib/constants.js';
 import {isBuiltinWithMutableMethods, isBuiltinWithoutMutableMethods} from './lib/is-builtin.js';
 import path from './lib/path.js';
+import isArray from './lib/is-array.js';
 import isSymbol from './lib/is-symbol.js';
 import isIterator from './lib/is-iterator.js';
 import wrapIterator from './lib/wrap-iterator.js';
@@ -55,7 +56,7 @@ const onChange = (object, onChange, options = {}) => {
 	};
 
 	const getProxyTarget = value => value
-		? (value[proxyTarget] || value)
+		? (value[proxyTarget] ?? value)
 		: value;
 
 	const prepareValue = (value, target, property, basePath) => {
@@ -74,7 +75,52 @@ const onChange = (object, onChange, options = {}) => {
 			basePath = cache.getPath(target);
 		}
 
-		return cache.getProxy(value, path.concat(basePath, property), handler, proxyTarget);
+		/*
+  		Check for circular references.
+
+  		If the value already has a corresponding path/proxy,
+		and if the path corresponds to one of the parents,
+		then we are on a circular case, where the child is pointing to their parent.
+		In this case we return the proxy object with the shortest path.
+  		*/
+		const childPath = path.concat(basePath, property);
+		const existingPath = cache.getPath(value);
+
+		if (existingPath && isSameObjectTree(childPath, existingPath)) {
+			// We are on the same object tree but deeper, so we use the parent path.
+			return cache.getProxy(value, existingPath, handler, proxyTarget);
+		}
+
+		return cache.getProxy(value, childPath, handler, proxyTarget);
+	};
+
+	/*
+	Returns true if `childPath` is a subpath of `existingPath`
+	(if childPath starts with existingPath). Otherwise, it returns false.
+
+ 	It also returns false if the 2 paths are identical.
+
+ 	For example:
+	- childPath    = group.layers.0.parent.layers.0.value
+	- existingPath = group.layers.0.parent
+	*/
+	const isSameObjectTree = (childPath, existingPath) => {
+		if (isSymbol(childPath) || childPath.length <= existingPath.length) {
+			return false;
+		}
+
+		if (isArray(existingPath) && existingPath.length === 0) {
+			return false;
+		}
+
+		const childParts = isArray(childPath) ? childPath : childPath.split(PATH_SEPARATOR);
+		const existingParts = isArray(existingPath) ? existingPath : existingPath.split(PATH_SEPARATOR);
+
+		if (childParts.length <= existingParts.length) {
+			return false;
+		}
+
+		return !(existingParts.some((part, index) => part !== childParts[index]));
 	};
 
 	const handler = {
@@ -104,7 +150,7 @@ const onChange = (object, onChange, options = {}) => {
 		set(target, property, value, receiver) {
 			value = getProxyTarget(value);
 
-			const reflectTarget = target[proxyTarget] || target;
+			const reflectTarget = target[proxyTarget] ?? target;
 			const previous = reflectTarget[property];
 
 			if (equals(previous, value) && property in target) {
@@ -161,7 +207,7 @@ const onChange = (object, onChange, options = {}) => {
 		},
 
 		apply(target, thisArg, argumentsList) {
-			const thisProxyTarget = thisArg[proxyTarget] || thisArg;
+			const thisProxyTarget = thisArg[proxyTarget] ?? thisArg;
 
 			if (cache.isUnsubscribed) {
 				return Reflect.apply(target, thisProxyTarget, argumentsList);
@@ -240,7 +286,7 @@ const onChange = (object, onChange, options = {}) => {
 	return proxy;
 };
 
-onChange.target = proxy => (proxy && proxy[TARGET]) || proxy;
-onChange.unsubscribe = proxy => proxy[UNSUBSCRIBE] || proxy;
+onChange.target = proxy => proxy?.[TARGET] ?? proxy;
+onChange.unsubscribe = proxy => proxy?.[UNSUBSCRIBE] ?? proxy;
 
 export default onChange;

lib/cache.js

@@ -54,7 +54,7 @@ export default class Cache {
 		}
 
 		const reflectTarget = target[proxyTarget];
-		const source = reflectTarget || target;
+		const source = reflectTarget ?? target;
 
 		this._pathCache.set(source, path);
 

lib/path.js

@@ -3,7 +3,7 @@ import isArray from './is-array.js';
 import isSymbol from './is-symbol.js';
 
 const path = {
-	after: (path, subPath) => {
+	after(path, subPath) {
 		if (isArray(path)) {
 			return path.slice(subPath.length);
 		}
@@ -14,7 +14,7 @@ const path = {
 
 		return path.slice(subPath.length + 1);
 	},
-	concat: (path, key) => {
+	concat(path, key) {
 		if (isArray(path)) {
 			path = [...path];
 
@@ -39,7 +39,7 @@ const path = {
 
 		return path;
 	},
-	initial: path => {
+	initial(path) {
 		if (isArray(path)) {
 			return path.slice(0, -1);
 		}
@@ -56,9 +56,9 @@ const path = {
 
 		return path.slice(0, index);
 	},
-	last: path => {
+	last(path) {
 		if (isArray(path)) {
-			return path[path.length - 1] || '';
+			return path.at(-1) ?? '';
 		}
 
 		if (path === '') {
@@ -73,7 +73,7 @@ const path = {
 
 		return path.slice(index + 1);
 	},
-	walk: (path, callback) => {
+	walk(path, callback) {
 		if (isArray(path)) {
 			for (const key of path) {
 				callback(key);

lib/smart-clone/clone/clone-object.js

@@ -45,7 +45,7 @@ export default class CloneObject {
 		return clone;
 	}
 
-	preferredThisArg(isHandledMethod, name, thisArg, thisProxyTarget) {
+	preferredThisArg(isHandledMethod, name, thisArgument, thisProxyTarget) {
 		if (isHandledMethod) {
 			if (isArray(thisProxyTarget)) {
 				this._onIsChanged = MUTABLE_ARRAY_METHODS[name];
@@ -58,7 +58,7 @@ export default class CloneObject {
 			return thisProxyTarget;
 		}
 
-		return thisArg;
+		return thisArgument;
 	}
 
 	update(fullPath, property, value) {
@@ -68,7 +68,7 @@ export default class CloneObject {
 			let object = this.clone;
 
 			path.walk(changePath, key => {
-				if (object && object[key]) {
+				if (object?.[key]) {
 					if (!this._clonedCache.has(object[key])) {
 						object[key] = this._shallowClone(object[key]);
 					}
@@ -85,7 +85,7 @@ export default class CloneObject {
 				});
 			}
 
-			if (object && object[property]) {
+			if (object?.[property]) {
 				object[property] = value;
 			}
 		}

lib/smart-clone/clone/clone-set.js

@@ -18,4 +18,3 @@ export default class CloneSet extends CloneObject {
 		}
 	}
 }
-

lib/smart-clone/clone/clone-weakset.js

@@ -4,20 +4,19 @@ export default class CloneWeakSet extends CloneObject {
 	constructor(value, path, argumentsList, hasOnValidate) {
 		super(undefined, path, argumentsList, hasOnValidate);
 
-		this._arg1 = argumentsList[0];
-		this._weakValue = value.has(this._arg1);
+		this._argument1 = argumentsList[0];
+		this._weakValue = value.has(this._argument1);
 	}
 
 	isChanged(value) {
-		return this._weakValue !== value.has(this._arg1);
+		return this._weakValue !== value.has(this._argument1);
 	}
 
 	undo(object) {
-		if (this._weakValue && !object.has(this._arg1)) {
-			object.add(this._arg1);
+		if (this._weakValue && !object.has(this._argument1)) {
+			object.add(this._argument1);
 		} else {
-			object.delete(this._arg1);
+			object.delete(this._argument1);
 		}
 	}
 }
-

lib/smart-clone/smart-clone.js

@@ -66,19 +66,19 @@ export default class SmartClone {
 	}
 
 	update(fullPath, property, value) {
-		this._stack[this._stack.length - 1].update(fullPath, property, value);
+		this._stack.at(-1).update(fullPath, property, value);
 	}
 
-	preferredThisArg(target, thisArg, thisProxyTarget) {
+	preferredThisArg(target, thisArgument, thisProxyTarget) {
 		const {name} = target;
 		const isHandledMethod = SmartClone.isHandledMethod(thisProxyTarget, name);
 
-		return this._stack[this._stack.length - 1]
-			.preferredThisArg(isHandledMethod, name, thisArg, thisProxyTarget);
+		return this._stack.at(-1)
+			.preferredThisArg(isHandledMethod, name, thisArgument, thisProxyTarget);
 	}
 
 	isChanged(isMutable, value, equals) {
-		return this._stack[this._stack.length - 1].isChanged(isMutable, value, equals);
+		return this._stack.at(-1).isChanged(isMutable, value, equals);
 	}
 
 	undo(object) {

lib/wrap-iterator.js

@@ -1,7 +1,7 @@
 import {TARGET} from './constants.js';
 
 // eslint-disable-next-line max-params
-export default function wrapIterator(iterator, target, thisArg, applyPath, prepareValue) {
+export default function wrapIterator(iterator, target, thisArgument, applyPath, prepareValue) {
 	const originalNext = iterator.next;
 
 	if (target.name === 'entries') {
@@ -26,7 +26,7 @@ export default function wrapIterator(iterator, target, thisArg, applyPath, prepa
 			return result;
 		};
 	} else if (target.name === 'values') {
-		const keyIterator = thisArg[TARGET].keys();
+		const keyIterator = thisArgument[TARGET].keys();
 
 		iterator.next = function () {
 			const result = originalNext.call(this);

package.json

@@ -11,9 +11,13 @@
 		"url": "https://sindresorhus.com"
 	},
 	"type": "module",
-	"exports": "./index.js",
+	"exports": {
+		"types": "./index.d.ts",
+		"default": "./index.js"
+	},
+	"sideEffects": false,
 	"engines": {
-		"node": "^12.20.0 || ^14.13.1 || >=16.0.0"
+		"node": ">=18"
 	},
 	"scripts": {
 		"test": "xo && ava && tsd",
@@ -41,13 +45,11 @@
 		"listener"
 	],
 	"devDependencies": {
-		"@typescript-eslint/parser": "^5.10.0",
-		"ava": "^4.0.1",
-		"display-value": "^2.0.0",
-		"karma-webpack-bundle": "^1.3.1",
+		"ava": "^6.0.1",
+		"display-value": "^2.2.0",
+		"karma-webpack-bundle": "^1.3.3",
 		"powerset": "0.0.1",
-		"tsd": "^0.19.1",
-		"typescript": "^4.5.5",
-		"xo": "^0.47.0"
+		"tsd": "^0.29.0",
+		"xo": "^0.56.0"
 	}
 }

readme.md

@@ -198,8 +198,6 @@ Type: `Function`
 
 The function receives the same arguments and context as the [onChange callback](#onchange). The function is called whenever a change is attempted. Returning true will allow the change to be made and the onChange callback to execute, returning anything else will prevent the change from being made and the onChange callback will not trigger.
 
-<br>
-
 ### onChange.target(object)
 
 Returns the original unwatched object.

tests/on-change.accessors.test.js

@@ -15,7 +15,7 @@ test('invariants', t => {
 	// eslint-disable-next-line accessor-pairs
 	Object.defineProperty(object, 'nonReadable', {
 		configurable: false,
-		set: () => {}, // No-Op setter
+		set() {}, // No-Op setter
 	});
 	Object.defineProperty(object, 'useAccessor', {
 		configurable: false,