fix(shallow): iterable-like insensitive keys order comparison (altern…

…ate implementation) (#2821)

* alternate implementation for shallow

* test with pure iterable

* refactor

* another refactor

---------

Co-authored-by: Danilo Britto <dbritto.dev@gmail.com>

src/vanilla/shallow.ts

@@ -1,13 +1,23 @@
 const isIterable = (obj: object): obj is Iterable<unknown> =>
   Symbol.iterator in obj
 
-const compareMapLike = (
-  iterableA: Iterable<[unknown, unknown]>,
-  iterableB: Iterable<[unknown, unknown]>,
+const hasIterableEntries = (
+  value: Iterable<unknown>,
+): value is Iterable<unknown> & {
+  entries(): Iterable<[unknown, unknown]>
+} =>
+  // HACK: avoid checking entries type
+  'entries' in value
+
+const compareEntries = (
+  valueA: { entries(): Iterable<[unknown, unknown]> },
+  valueB: { entries(): Iterable<[unknown, unknown]> },
 ) => {
-  const mapA = iterableA instanceof Map ? iterableA : new Map(iterableA)
-  const mapB = iterableB instanceof Map ? iterableB : new Map(iterableB)
-  if (mapA.size !== mapB.size) return false
+  const mapA = valueA instanceof Map ? valueA : new Map(valueA.entries())
+  const mapB = valueB instanceof Map ? valueB : new Map(valueB.entries())
+  if (mapA.size !== mapB.size) {
+    return false
+  }
   for (const [key, value] of mapA) {
     if (!Object.is(value, mapB.get(key))) {
       return false
@@ -16,60 +26,45 @@ const compareMapLike = (
   return true
 }
 
-export function shallow<T>(objA: T, objB: T): boolean {
-  if (Object.is(objA, objB)) {
+// Ordered iterables
+const compareIterables = (
+  valueA: Iterable<unknown>,
+  valueB: Iterable<unknown>,
+) => {
+  const iteratorA = valueA[Symbol.iterator]()
+  const iteratorB = valueB[Symbol.iterator]()
+  let nextA = iteratorA.next()
+  let nextB = iteratorB.next()
+  while (!nextA.done && !nextB.done) {
+    if (!Object.is(nextA.value, nextB.value)) {
+      return false
+    }
+    nextA = iteratorA.next()
+    nextB = iteratorB.next()
+  }
+  return !!nextA.done && !!nextB.done
+}
+
+export function shallow<T>(valueA: T, valueB: T): boolean {
+  if (Object.is(valueA, valueB)) {
     return true
   }
   if (
-    typeof objA !== 'object' ||
-    objA === null ||
-    typeof objB !== 'object' ||
-    objB === null
+    typeof valueA !== 'object' ||
+    valueA === null ||
+    typeof valueB !== 'object' ||
+    valueB === null
   ) {
     return false
   }
-
-  if (isIterable(objA) && isIterable(objB)) {
-    const iteratorA = objA[Symbol.iterator]()
-    const iteratorB = objB[Symbol.iterator]()
-    let nextA = iteratorA.next()
-    let nextB = iteratorB.next()
-    if (
-      Array.isArray(nextA.value) &&
-      Array.isArray(nextB.value) &&
-      nextA.value.length === 2 &&
-      nextB.value.length === 2
-    ) {
-      try {
-        return compareMapLike(
-          objA as Iterable<[unknown, unknown]>,
-          objB as Iterable<[unknown, unknown]>,
-        )
-      } catch {
-        // fallback
-      }
-    }
-    while (!nextA.done && !nextB.done) {
-      if (!Object.is(nextA.value, nextB.value)) {
-        return false
-      }
-      nextA = iteratorA.next()
-      nextB = iteratorB.next()
-    }
-    return !!nextA.done && !!nextB.done
+  if (!isIterable(valueA) || !isIterable(valueB)) {
+    return compareEntries(
+      { entries: () => Object.entries(valueA) },
+      { entries: () => Object.entries(valueB) },
+    )
   }
-
-  const keysA = Object.keys(objA)
-  if (keysA.length !== Object.keys(objB).length) {
-    return false
+  if (hasIterableEntries(valueA) && hasIterableEntries(valueB)) {
+    return compareEntries(valueA, valueB)
   }
-  for (const keyA of keysA) {
-    if (
-      !Object.hasOwn(objB, keyA as string) ||
-      !Object.is(objA[keyA as keyof T], objB[keyA as keyof T])
-    ) {
-      return false
-    }
-  }
-  return true
+  return compareIterables(valueA, valueB)
 }

tests/vanilla/shallow.test.tsx

@@ -37,43 +37,93 @@ describe('shallow', () => {
     ).toBe(false)
 
     expect(shallow([{ foo: 'bar' }], [{ foo: 'bar', asd: 123 }])).toBe(false)
+
+    expect(shallow([1, 2, 3], [2, 3, 1])).toBe(false)
   })
 
   it('compares Maps', () => {
-    function createMap<T extends object>(obj: T) {
-      return new Map(Object.entries(obj))
-    }
+    expect(
+      shallow(
+        new Map<string, unknown>([
+          ['foo', 'bar'],
+          ['asd', 123],
+        ]),
+        new Map<string, unknown>([
+          ['foo', 'bar'],
+          ['asd', 123],
+        ]),
+      ),
+    ).toBe(true)
 
     expect(
       shallow(
-        createMap({ foo: 'bar', asd: 123 }),
-        createMap({ foo: 'bar', asd: 123 }),
+        new Map<string, unknown>([
+          ['foo', 'bar'],
+          ['asd', 123],
+        ]),
+        new Map<string, unknown>([
+          ['asd', 123],
+          ['foo', 'bar'],
+        ]),
       ),
     ).toBe(true)
 
     expect(
       shallow(
-        createMap({ foo: 'bar', asd: 123 }),
-        createMap({ foo: 'bar', foobar: true }),
+        new Map<string, unknown>([
+          ['foo', 'bar'],
+          ['asd', 123],
+        ]),
+        new Map<string, unknown>([
+          ['foo', 'bar'],
+          ['foobar', true],
+        ]),
+      ),
+    ).toBe(false)
+
+    expect(
+      shallow(
+        new Map<string, unknown>([
+          ['foo', 'bar'],
+          ['asd', 123],
+        ]),
+        new Map<string, unknown>([
+          ['foo', 'bar'],
+          ['asd', 123],
+          ['foobar', true],
+        ]),
       ),
     ).toBe(false)
 
+    const obj = {}
+    const obj2 = {}
     expect(
       shallow(
-        createMap({ foo: 'bar', asd: 123 }),
-        createMap({ foo: 'bar', asd: 123, foobar: true }),
+        new Map<object, unknown>([[obj, 'foo']]),
+        new Map<object, unknown>([[obj2, 'foo']]),
       ),
     ).toBe(false)
   })
 
   it('compares Sets', () => {
     expect(shallow(new Set(['bar', 123]), new Set(['bar', 123]))).toBe(true)
 
+    expect(shallow(new Set(['bar', 123]), new Set([123, 'bar']))).toBe(true)
+
     expect(shallow(new Set(['bar', 123]), new Set(['bar', 2]))).toBe(false)
 
     expect(shallow(new Set(['bar', 123]), new Set(['bar', 123, true]))).toBe(
       false,
     )
+
+    const obj = {}
+    const obj2 = {}
+    expect(shallow(new Set([obj]), new Set([obj]))).toBe(true)
+    expect(shallow(new Set([obj]), new Set([obj2]))).toBe(false)
+    expect(shallow(new Set([obj]), new Set([obj, obj2]))).toBe(false)
+    expect(shallow(new Set([obj]), new Set([obj2, obj]))).toBe(false)
+
+    expect(shallow(['bar', 123] as never, new Set(['bar', 123]))).toBe(false)
   })
 
   it('compares functions', () => {
@@ -93,9 +143,27 @@ describe('shallow', () => {
   })
 
   it('compares URLSearchParams', () => {
-    const a = new URLSearchParams({ hello: 'world' })
-    const b = new URLSearchParams({ zustand: 'shallow' })
-    expect(shallow(a, b)).toBe(false)
+    expect(
+      shallow(new URLSearchParams({ a: 'a' }), new URLSearchParams({ a: 'a' })),
+    ).toBe(true)
+    expect(
+      shallow(new URLSearchParams({ a: 'a' }), new URLSearchParams({ a: 'b' })),
+    ).toBe(false)
+    expect(
+      shallow(new URLSearchParams({ a: 'a' }), new URLSearchParams({ b: 'b' })),
+    ).toBe(false)
+    expect(
+      shallow(
+        new URLSearchParams({ a: 'a' }),
+        new URLSearchParams({ a: 'a', b: 'b' }),
+      ),
+    ).toBe(false)
+    expect(
+      shallow(
+        new URLSearchParams({ b: 'b', a: 'a' }),
+        new URLSearchParams({ a: 'a', b: 'b' }),
+      ),
+    ).toBe(true)
   })
 
   it('should work with nested arrays (#2794)', () => {
@@ -104,6 +172,17 @@ describe('shallow', () => {
   })
 })
 
+describe('generators', () => {
+  it('pure iterable', () => {
+    function* gen() {
+      yield 1
+      yield 2
+    }
+    expect(Symbol.iterator in gen()).toBe(true)
+    expect(shallow(gen(), gen())).toBe(true)
+  })
+})
+
 describe('unsupported cases', () => {
   it('date', () => {
     expect(