Enhance README with badges and improve circular reference handling

README.md

@@ -1,4 +1,6 @@
 # spotdiff
+[![npm](https://img.shields.io/npm/v/spotdiff)](https://www.npmjs.com/package/spotdiff)
+[![Socket Badge](https://badge.socket.dev/npm/package/spotdiff/0.1.0)](https://badge.socket.dev/npm/package/spotdiff/0.1.0)
 
 Compare two JS objects, find what changed, patch back, humanize the diff.
 

src/diff.ts

@@ -1,78 +1,80 @@
 import type { Change, SpotdiffOptions } from './types.js'
-import { isObject, isPrimitive, areEqual } from './utils.js'
+import { isObject, areEqual } from './utils.js'
 
 interface WalkContext {
   changes: Change[]
-  options: Required<SpotdiffOptions>
-  visited: WeakSet<object>
+  options: {
+    maxDepth: number
+    ignoreKeysSet: Set<string>
+    arrays: 'index' | 'smart'
+  }
+  stackA: WeakSet<object>
+  stackB: WeakSet<object>
 }
 
-function walk(
-  a: unknown,
-  b: unknown,
-  path: string,
-  depth: number,
-  ctx: WalkContext
-): void {
+function isTraversable(val: unknown): val is object {
+  return (
+    val !== null &&
+    typeof val === 'object' &&
+    !(val instanceof Date) &&
+    !(val instanceof Map) &&
+    !(val instanceof Set)
+  )
+}
+
+function checkCircular(val: unknown, path: string, stack: WeakSet<object>): void {
+  if (isTraversable(val) && stack.has(val as object)) {
+    throw new Error(`Circular reference detected at path: ${path || '(root)'}`)
+  }
+}
+
+function walk(a: unknown, b: unknown, path: string, depth: number, ctx: WalkContext): void {
   if (areEqual(a, b)) return
 
-  // Both primitives or incompatible types — emit change directly
-  const aIsObj = isObject(a) || Array.isArray(a)
-  const bIsObj = isObject(b) || Array.isArray(b)
+  const aTraversable = isTraversable(a)
+  const bTraversable = isTraversable(b)
 
-  if (!aIsObj && !bIsObj) {
+  if (!aTraversable || !bTraversable) {
     ctx.changes.push({ path, op: 'changed', from: a, to: b })
     return
   }
 
-  // Type mismatch (one is object/array, other is not)
-  if (!aIsObj || !bIsObj) {
+  if (depth >= ctx.options.maxDepth) {
     ctx.changes.push({ path, op: 'changed', from: a, to: b })
     return
   }
 
-  if (depth >= ctx.options.maxDepth) {
-    if (!areEqual(a, b)) ctx.changes.push({ path, op: 'changed', from: a, to: b })
-    return
-  }
+  const aObj = a as object
+  const bObj = b as object
 
-  // Circular reference detection
-  if (typeof a === 'object' && a !== null) {
-    if (ctx.visited.has(a as object)) throw new Error(`Circular reference detected at path: ${path || '(root)'}`)
-    ctx.visited.add(a as object)
-  }
-  if (typeof b === 'object' && b !== null) {
-    if (ctx.visited.has(b as object)) throw new Error(`Circular reference detected at path: ${path || '(root)'}`)
-    ctx.visited.add(b as object)
-  }
+  if (ctx.stackA.has(aObj)) throw new Error(`Circular reference detected at path: ${path || '(root)'}`)
+  if (ctx.stackB.has(bObj)) throw new Error(`Circular reference detected at path: ${path || '(root)'}`)
+
+  ctx.stackA.add(aObj)
+  ctx.stackB.add(bObj)
 
   if (Array.isArray(a) && Array.isArray(b)) {
     diffArrays(a, b, path, depth, ctx)
   } else if (Array.isArray(a) !== Array.isArray(b)) {
     ctx.changes.push({ path, op: 'changed', from: a, to: b })
   } else {
-    diffObjects(
-      a as Record<string, unknown>,
-      b as Record<string, unknown>,
-      path,
-      depth,
-      ctx
-    )
+    diffObjects(a as Record<string, unknown>, b as Record<string, unknown>, path, depth, ctx)
   }
+
+  // Pop from stack so shared (non-circular) references don't trigger false positives
+  ctx.stackA.delete(aObj)
+  ctx.stackB.delete(bObj)
 }
 
-function diffArrays(
-  a: unknown[],
-  b: unknown[],
-  path: string,
-  depth: number,
-  ctx: WalkContext
-): void {
+function diffArrays(a: unknown[], b: unknown[], path: string, depth: number, ctx: WalkContext): void {
   if (ctx.options.arrays === 'smart') {
     diffArraysSmart(a, b, path, depth, ctx)
     return
   }
+  diffArraysIndex(a, b, path, depth, ctx)
+}
 
+function diffArraysIndex(a: unknown[], b: unknown[], path: string, depth: number, ctx: WalkContext): void {
   const len = Math.max(a.length, b.length)
   for (let i = 0; i < len; i++) {
     const childPath = `${path}[${i}]`
@@ -86,15 +88,7 @@ function diffArrays(
   }
 }
 
-function diffArraysSmart(
-  a: unknown[],
-  b: unknown[],
-  path: string,
-  depth: number,
-  ctx: WalkContext
-): void {
-  // For object arrays: try to match by identity keys (id, key, name)
-  // Fall back to index-based for primitive arrays
+function diffArraysSmart(a: unknown[], b: unknown[], path: string, depth: number, ctx: WalkContext): void {
   const allObjects = a.every(isObject) && b.every(isObject)
 
   if (!allObjects) {
@@ -143,34 +137,6 @@ function diffArraysSmart(
   }
 }
 
-function diffArraysIndex(
-  a: unknown[],
-  b: unknown[],
-  path: string,
-  depth: number,
-  ctx: WalkContext
-): void {
-  const len = Math.max(a.length, b.length)
-  for (let i = 0; i < len; i++) {
-    const childPath = `${path}[${i}]`
-    if (i >= a.length) {
-      ctx.changes.push({ path: childPath, op: 'added', from: undefined, to: b[i] })
-    } else if (i >= b.length) {
-      ctx.changes.push({ path: childPath, op: 'removed', from: a[i], to: undefined })
-    } else {
-      walk(a[i], b[i], childPath, depth + 1, ctx)
-    }
-  }
-}
-
-function checkCircular(val: unknown, path: string, visited: WeakSet<object>): void {
-  if (typeof val === 'object' && val !== null && !(val instanceof Date) && !(val instanceof Map) && !(val instanceof Set)) {
-    if (visited.has(val as object)) {
-      throw new Error(`Circular reference detected at path: ${path || '(root)'}`)
-    }
-  }
-}
-
 function diffObjects(
   a: Record<string, unknown>,
   b: Record<string, unknown>,
@@ -181,17 +147,17 @@ function diffObjects(
   const keys = new Set([...Object.keys(a), ...Object.keys(b)])
 
   for (const key of keys) {
-    if (ctx.options.ignoreKeys.includes(key)) continue
+    if (ctx.options.ignoreKeysSet.has(key)) continue
 
     const childPath = path ? `${path}.${key}` : key
     const inA = Object.prototype.hasOwnProperty.call(a, key)
     const inB = Object.prototype.hasOwnProperty.call(b, key)
 
     if (!inA) {
-      checkCircular(b[key], childPath, ctx.visited)
+      checkCircular(b[key], childPath, ctx.stackB)
       ctx.changes.push({ path: childPath, op: 'added', from: undefined, to: b[key] })
     } else if (!inB) {
-      checkCircular(a[key], childPath, ctx.visited)
+      checkCircular(a[key], childPath, ctx.stackA)
       ctx.changes.push({ path: childPath, op: 'removed', from: a[key], to: undefined })
     } else {
       walk(a[key], b[key], childPath, depth + 1, ctx)
@@ -204,10 +170,11 @@ export function spotdiff(a: unknown, b: unknown, options: SpotdiffOptions = {}):
     changes: [],
     options: {
       maxDepth: options.maxDepth ?? Infinity,
-      ignoreKeys: options.ignoreKeys ?? [],
+      ignoreKeysSet: new Set(options.ignoreKeys ?? []),
       arrays: options.arrays ?? 'index',
     },
-    visited: new WeakSet(),
+    stackA: new WeakSet(),
+    stackB: new WeakSet(),
   }
 
   walk(a, b, '', 0, ctx)

src/patch.ts

@@ -100,25 +100,41 @@ export function deletePath(obj: unknown, path: string): void {
   }
 }
 
+function arrayRemoveKey(path: string): { parent: string; index: number } | null {
+  const m = path.match(/^(.*)\[(\d+)\]$/)
+  if (!m) return null
+  return { parent: m[1], index: parseInt(m[2], 10) }
+}
+
 export function patch(obj: unknown, changes: Change[], reverse = false): unknown {
   const cloned = cloneDeep(obj)
 
-  for (const change of changes) {
+  // Array element deletions must be applied highest-index-first to prevent
+  // splice() from shifting subsequent indices in the same array.
+  const sorted = [...changes].sort((ca, cb) => {
+    const aIsDelete = reverse ? ca.op === 'added' : ca.op === 'removed'
+    const bIsDelete = reverse ? cb.op === 'added' : cb.op === 'removed'
+    if (aIsDelete && bIsDelete) {
+      const aArr = arrayRemoveKey(ca.path)
+      const bArr = arrayRemoveKey(cb.path)
+      if (aArr && bArr && aArr.parent === bArr.parent) {
+        return bArr.index - aArr.index
+      }
+    }
+    return 0
+  })
+
+  for (const change of sorted) {
     const { path, op, from, to } = change
 
     if (reverse) {
-      // Invert: added→remove, removed→add, changed→swap from/to
       if (op === 'added') {
         deletePath(cloned, path)
-      } else if (op === 'removed') {
-        setPath(cloned, path, from)
       } else {
         setPath(cloned, path, from)
       }
     } else {
-      if (op === 'added') {
-        setPath(cloned, path, to)
-      } else if (op === 'removed') {
+      if (op === 'removed') {
         deletePath(cloned, path)
       } else {
         setPath(cloned, path, to)

src/utils.ts

@@ -1,7 +1,14 @@
 import type { Op } from './types.js'
 
 export function isObject(val: unknown): val is Record<string, unknown> {
-  return val !== null && typeof val === 'object' && !(val instanceof Date) && !Array.isArray(val)
+  return (
+    val !== null &&
+    typeof val === 'object' &&
+    !(val instanceof Date) &&
+    !(val instanceof Map) &&
+    !(val instanceof Set) &&
+    !Array.isArray(val)
+  )
 }
 
 export function isPrimitive(val: unknown): boolean {

tests/spotdiff.test.ts

@@ -202,6 +202,15 @@ test('circular: throws descriptive error', () => {
   expect(() => spotdiff(a, { x: 1 })).toThrow(/[Cc]ircular/)
 })
 
+test('circular: shared reference (non-circular) does not throw', () => {
+  const shared = { val: 1 }
+  const a = { x: shared, y: shared }
+  const b = { x: { val: 1 }, y: { val: 2 } }
+  expect(() => spotdiff(a, b)).not.toThrow()
+  const changes = spotdiff(a, b)
+  expect(changes).toEqual([{ path: 'y.val', op: 'changed', from: 1, to: 2 }])
+})
+
 // ── Map and Set ───────────────────────────────────────────────────────────
 
 test('Map: treated as opaque (no crash)', () => {
@@ -210,12 +219,26 @@ test('Map: treated as opaque (no crash)', () => {
   expect(() => spotdiff({ m: m1 }, { m: m2 })).not.toThrow()
 })
 
+test('Map: different Maps reported as changed', () => {
+  const changes = spotdiff({ m: new Map([['k', 1]]) }, { m: new Map([['k', 2]]) })
+  expect(changes[0]?.op).toBe('changed')
+})
+
 test('Set: treated as opaque (no crash)', () => {
   const s1 = new Set([1, 2])
   const s2 = new Set([1, 2])
   expect(() => spotdiff({ s: s1 }, { s: s2 })).not.toThrow()
 })
 
+// ── patch: multiple array deletions ──────────────────────────────────────
+
+test('patch: multiple array removals apply without index shift', () => {
+  const v1 = { items: ['a', 'b', 'c'] }
+  const v2 = { items: ['a'] }
+  const changes = spotdiff(v1, v2)
+  expect(patch(v1, changes)).toEqual(v2)
+})
+
 // ── arrays smart mode ─────────────────────────────────────────────────────
 
 test('arrays smart: matches objects by id', () => {