From cb66630d87126b5d9baa326504a86c2d0727e196 Mon Sep 17 00:00:00 2001 From: Radoslav Karaivanov Date: Mon, 28 Sep 2026 19:13:21 +0300 Subject: [PATCH 1/3] fix(equal): match Set/Map entries one to one, allow null-prototype objects A Set or Map entry could match an entry on the other side that was already matched, so `new Set([{x:1},{x:1}])` equaled `new Set([{x:1},{x:2}])`. Each entry now takes a distinct match. Null-prototype objects threw, since `valueOf` and `toString` were called without an existence check. Ported from IgniteUI/igniteui-react#193. --- src/internals/utils/objects.spec.ts | 27 +++++++++ src/internals/utils/objects.ts | 88 ++++++++++++----------------- 2 files changed, 63 insertions(+), 52 deletions(-) diff --git a/src/internals/utils/objects.spec.ts b/src/internals/utils/objects.spec.ts index 3924c2d01..500cfc334 100644 --- a/src/internals/utils/objects.spec.ts +++ b/src/internals/utils/objects.spec.ts @@ -368,6 +368,33 @@ describe('equal', () => { expect(equal(a, mismatched)).to.be.false; }); + it('should match Set elements one to one', () => { + expect(equal(new Set([{ x: 1 }, { x: 1 }]), new Set([{ x: 1 }, { x: 2 }]))) + .to.be.false; + expect(equal(new Set([{ x: 1 }, { x: 1 }]), new Set([{ x: 1 }, { x: 1 }]))) + .to.be.true; + }); + + it('should match Map entries one to one', () => { + const left = new Map([ + [{ k: 1 }, 'v'], + [{ k: 1 }, 'v'], + ]); + const right = new Map([ + [{ k: 1 }, 'v'], + [{ k: 2 }, 'v'], + ]); + expect(equal(left, right)).to.be.false; + }); + + it('should compare null-prototype objects', () => { + const create = (value: number) => + Object.assign(Object.create(null), { a: value }); + + expect(equal(create(1), create(1))).to.be.true; + expect(equal(create(1), create(2))).to.be.false; + }); + it('should still terminate on circular references', () => { const a: Record = { name: 'a' }; const b: Record = { name: 'a' }; diff --git a/src/internals/utils/objects.ts b/src/internals/utils/objects.ts index 692a1f237..2d1f6c2b7 100644 --- a/src/internals/utils/objects.ts +++ b/src/internals/utils/objects.ts @@ -1,4 +1,4 @@ -import { isObject, isRegExp } from './types.js'; +import { isFunction, isObject, isRegExp } from './types.js'; /** The object pairs the comparison visits at this moment, to stop cycles. */ type Visited = WeakMap>; @@ -20,14 +20,9 @@ export function equal( // Record the pair, not each object on its own: the Map and Set branches // below test candidates they expect to fail, and single-object records // would make a later comparison of the same pair return `true`. - let pending = visited.get(a); - if (pending?.has(b)) return true; - - if (!pending) { - pending = new WeakSet(); - visited.set(a, pending); - } - pending.add(b); + const pending = visited.get(a) ?? new WeakSet(); + if (pending.has(b)) return true; + visited.set(a, pending.add(b)); try { return compare(a, b, visited); @@ -41,61 +36,50 @@ function compare(a: object, b: object, visited: Visited): boolean { if (isRegExp(a) && isRegExp(b)) return a.source === b.source && a.flags === b.flags; - if (a instanceof Map && b instanceof Map) { - if (a.size !== b.size) return false; - for (const [keyA, valueA] of a.entries()) { - let found = false; - for (const [keyB, valueB] of b.entries()) { - if (equal(keyA, keyB, visited) && equal(valueA, valueB, visited)) { - found = true; - break; - } - } - if (!found) return false; - } - return true; - } - - if (a instanceof Set && b instanceof Set) { - if (a.size !== b.size) return false; - for (const valueA of a) { - let found = false; - for (const valueB of b) { - if (equal(valueA, valueB, visited)) { - found = true; - break; - } - } - if (!found) return false; - } - return true; - } + // Map entries iterate as [key, value] arrays. + if ( + (a instanceof Map && b instanceof Map) || + (a instanceof Set && b instanceof Set) + ) + return a.size === b.size && matchOneToOne(a, b, visited); if (Array.isArray(a) && Array.isArray(b)) { - const length = a.length; - if (length !== b.length) return false; - for (let i = 0; i < length; i++) { + if (a.length !== b.length) return false; + for (let i = 0; i < a.length; i++) { if (!equal(a[i], b[i], visited)) return false; } return true; } - if (a.valueOf !== Object.prototype.valueOf) + // Null-prototype objects have neither method. + if (isFunction(a.valueOf) && a.valueOf !== Object.prototype.valueOf) return a.valueOf() === b.valueOf(); - if (a.toString !== Object.prototype.toString) + if (isFunction(a.toString) && a.toString !== Object.prototype.toString) return a.toString() === b.toString(); - const aKeys = Object.keys(a); - const bKeys = Object.keys(b); - if (aKeys.length !== bKeys.length) return false; + const keys = Object.keys(a) as (keyof typeof a)[]; - for (const key of aKeys) { - if (!Object.hasOwn(b, key)) return false; - } + return ( + keys.length === Object.keys(b).length && + keys.every((key) => Object.hasOwn(b, key) && equal(a[key], b[key], visited)) + ); +} + +/** + * Pairs each item of `left` with a distinct equal item of `right`. + * A greedy match suffices, since `equal` is transitive. + */ +function matchOneToOne( + left: Iterable, + right: Iterable, + visited: Visited +): boolean { + const pool = [...right]; - for (const key of aKeys) { - if (!equal(a[key as keyof typeof a], b[key as keyof typeof b], visited)) - return false; + for (const a of left) { + const index = pool.findIndex((b) => equal(a, b, visited)); + if (index < 0) return false; + pool.splice(index, 1); } return true; From a76400f78c7513c2696bb3854790b7cf9acb5465 Mon Sep 17 00:00:00 2001 From: Radoslav Karaivanov Date: Mon, 28 Sep 2026 19:19:48 +0300 Subject: [PATCH 2/3] fix(equal): guard b's valueOf/toString on null-prototype objects A null-prototype `a` with its own `valueOf` or `toString` threw when `b` lacked the method. The objects now compare unequal instead. --- src/internals/utils/objects.spec.ts | 15 +++++++++++++++ src/internals/utils/objects.ts | 6 +++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/internals/utils/objects.spec.ts b/src/internals/utils/objects.spec.ts index 500cfc334..a7658f01e 100644 --- a/src/internals/utils/objects.spec.ts +++ b/src/internals/utils/objects.spec.ts @@ -395,6 +395,21 @@ describe('equal', () => { expect(equal(create(1), create(2))).to.be.false; }); + it('should not throw when only one null-prototype object has valueOf or toString', () => { + const bare = Object.create(null); + const withValueOf = Object.assign(Object.create(null), { + valueOf: () => 1, + }); + const withToString = Object.assign(Object.create(null), { + toString: () => 'a', + }); + + expect(equal(withValueOf, bare)).to.be.false; + expect(equal(withToString, bare)).to.be.false; + expect(equal(bare, withValueOf)).to.be.false; + expect(equal(bare, withToString)).to.be.false; + }); + it('should still terminate on circular references', () => { const a: Record = { name: 'a' }; const b: Record = { name: 'a' }; diff --git a/src/internals/utils/objects.ts b/src/internals/utils/objects.ts index 2d1f6c2b7..f88a7d0d2 100644 --- a/src/internals/utils/objects.ts +++ b/src/internals/utils/objects.ts @@ -51,11 +51,11 @@ function compare(a: object, b: object, visited: Visited): boolean { return true; } - // Null-prototype objects have neither method. + // Null-prototype objects may lack either method, on either side. if (isFunction(a.valueOf) && a.valueOf !== Object.prototype.valueOf) - return a.valueOf() === b.valueOf(); + return isFunction(b.valueOf) && a.valueOf() === b.valueOf(); if (isFunction(a.toString) && a.toString !== Object.prototype.toString) - return a.toString() === b.toString(); + return isFunction(b.toString) && a.toString() === b.toString(); const keys = Object.keys(a) as (keyof typeof a)[]; From 0d194c6742415dacb922ccb9620ab227f4e293ec Mon Sep 17 00:00:00 2001 From: Radoslav Karaivanov Date: Mon, 28 Sep 2026 19:50:53 +0300 Subject: [PATCH 3/3] fix(equal): require both sides to agree on custom valueOf/toString Comparing a custom conversion with the default one made `equal` directional and non-transitive, so the greedy Set/Map pairing could miss a valid match. Objects now compare by conversion only when both override it, and are unequal when only one does. `equal` is then an equivalence relation, and greedy pairing is safe. --- src/internals/utils/objects.spec.ts | 24 ++++++++++++++ src/internals/utils/objects.ts | 51 +++++++++++++++++++---------- 2 files changed, 58 insertions(+), 17 deletions(-) diff --git a/src/internals/utils/objects.spec.ts b/src/internals/utils/objects.spec.ts index a7658f01e..d1aa52df2 100644 --- a/src/internals/utils/objects.spec.ts +++ b/src/internals/utils/objects.spec.ts @@ -387,6 +387,30 @@ describe('equal', () => { expect(equal(left, right)).to.be.false; }); + it('should give the same result in both directions', () => { + const valueOf = Object.create({ valueOf: () => 1 }); + // A `toString` that mimics the default one still differs from it. + const toString = Object.create({ toString: () => '[object Object]' }); + + expect(equal({}, valueOf)).to.be.false; + expect(equal(valueOf, {})).to.be.false; + expect(equal({}, toString)).to.be.false; + expect(equal(toString, {})).to.be.false; + }); + + it('should not treat objects pending in different pairs as equal', () => { + // `a` and `d` are both pending when they meet, but they differ. + const a: Record = {}; + const b: Record = {}; + const c: Record = { q: a }; + const d: Record = {}; + a.p = c; + b.p = d; + d.q = d; + + expect(equal(a, b)).to.be.false; + }); + it('should compare null-prototype objects', () => { const create = (value: number) => Object.assign(Object.create(null), { a: value }); diff --git a/src/internals/utils/objects.ts b/src/internals/utils/objects.ts index f88a7d0d2..f1269e5ac 100644 --- a/src/internals/utils/objects.ts +++ b/src/internals/utils/objects.ts @@ -1,15 +1,15 @@ import { isFunction, isObject, isRegExp } from './types.js'; -/** The object pairs the comparison visits at this moment, to stop cycles. */ +/** The object pairs under comparison at this moment. */ type Visited = WeakMap>; /** * Returns whether two values are deeply equal. Handles arrays, Maps, Sets, * RegExps, plain objects and circular references. */ -export function equal( +export function equal( a: unknown, - b: T, + b: unknown, visited: Visited = new WeakMap() ): boolean { if (Object.is(a, b)) return true; @@ -17,9 +17,8 @@ export function equal( if (!isObject(a) || !isObject(b)) return false; if (a.constructor !== b.constructor) return false; - // Record the pair, not each object on its own: the Map and Set branches - // below test candidates they expect to fail, and single-object records - // would make a later comparison of the same pair return `true`. + // A pending pair counts as equal, which stops cycles. Track pairs, not + // objects: `a` can meet another partner while it is still pending. const pending = visited.get(a) ?? new WeakSet(); if (pending.has(b)) return true; visited.set(a, pending.add(b)); @@ -27,7 +26,6 @@ export function equal( try { return compare(a, b, visited); } finally { - // Release the pair, including on an early return in `compare`. pending.delete(b); } } @@ -51,23 +49,42 @@ function compare(a: object, b: object, visited: Visited): boolean { return true; } - // Null-prototype objects may lack either method, on either side. - if (isFunction(a.valueOf) && a.valueOf !== Object.prototype.valueOf) - return isFunction(b.valueOf) && a.valueOf() === b.valueOf(); - if (isFunction(a.toString) && a.toString !== Object.prototype.toString) - return isFunction(b.toString) && a.toString() === b.toString(); + // A custom conversion on one side only makes the objects unequal, which + // keeps `equal` an equivalence relation. + for (const method of ['valueOf', 'toString'] as const) { + const left = customConversion(a, method); + const right = customConversion(b, method); - const keys = Object.keys(a) as (keyof typeof a)[]; + if (left || right) + return ( + !!left && + !!right && + Reflect.apply(left, a, []) === Reflect.apply(right, b, []) + ); + } + + const x = a as Record; + const y = b as Record; + const keys = Object.keys(x); return ( - keys.length === Object.keys(b).length && - keys.every((key) => Object.hasOwn(b, key) && equal(a[key], b[key], visited)) + keys.length === Object.keys(y).length && + keys.every((key) => Object.hasOwn(y, key) && equal(x[key], y[key], visited)) ); } +/** Returns `value[method]` unless it is missing or the default. */ +function customConversion( + value: object, + method: 'valueOf' | 'toString' +): CallableFunction | undefined { + const fn = (value as Record)[method]; + return isFunction(fn) && fn !== Object.prototype[method] ? fn : undefined; +} + /** - * Pairs each item of `left` with a distinct equal item of `right`. - * A greedy match suffices, since `equal` is transitive. + * Pairs each item of `left` with a distinct equal item of `right`. Greedy + * suffices, since `equal` is an equivalence relation. */ function matchOneToOne( left: Iterable,