Don't mutate deepMap values (#305)

Previously, calling setKey on a $deepMap would mutate the value
in-place. This can cause various problems, for example #250 and #290.
Those were both solved with workarounds, however, this solves it in a
better way, by not mutating the deepMap in the first place.

Some advantages of this approach:
* No need for structuredClone
* The oldValue passed to listeners is the same object reference as the
value that was previously passed. This is how immutable stores usually
work, so it will align better with expectations, cause fewer issues, and
allow for new use cases, like the upcoming batching PR which needs that.
* Copies were already being made at each nesting level of the key being
set, they just weren't being used (so maybe this was the original
intent?). Either way, that means there's no new work being done, and
perf will actually improve since we no longer need to call
structuredClone on oldValue.

As part of this change, when a key path extends past the end of an array
the result will be a sparse array rather than filling it with undefined.
It would have required extra code (and slower perf) to keep the old
behavior, and I don't see a good reason for it. The new behavior aligns
with what JS does natively, and with other stores like SolidJS. But let
me know if that's a problem.

deep-map/index.d.ts

@@ -32,7 +32,9 @@ export type DeepMapStore<T extends BaseDeepMap> = {
   notify(oldValue?: T, changedKey?: AllPaths<T>): void
 
   /**
-   * Change key in store value.
+   * Change key in store value. Copies are made at each level of `key` so that
+   * the old value is not mutated (but it does not do a full deep copy --
+   * references to objects will still be shared between the old and new value).
    *
    * ```js
    * $settings.setKey('visuals.theme', 'dark')

deep-map/index.js

@@ -6,14 +6,9 @@ export { getPath, setByKey, setPath } from './path.js'
 export function deepMap(initial = {}) {
   let $deepMap = atom(initial)
   $deepMap.setKey = (key, value) => {
-    let oldValue
-    try {
-      oldValue = structuredClone($deepMap.value)
-    } catch {
-      oldValue = { ...$deepMap.value }
-    }
     if (getPath($deepMap.value, key) !== value) {
-      $deepMap.value = { ...setPath($deepMap.value, key, value) }
+      let oldValue = $deepMap.value
+      $deepMap.value = setPath($deepMap.value, key, value)
       $deepMap.notify(oldValue, key)
     }
   }

deep-map/index.test.ts

@@ -324,7 +324,7 @@ test('does not run queued listeners after they are unsubscribed', () => {
   deepStrictEqual(events, ['a1', 'b1', 'a2', 'c2'])
 })
 
-test('notifies correct previous value from deep store', t => {
+test('notifies correct previous value from deep store', () => {
   type DeepValue = { a: number; b: { nested: { deep: number } } }
 
   let events: DeepValue[] = []
@@ -345,16 +345,6 @@ test('notifies correct previous value from deep store', t => {
     { a: 1, b: { nested: { deep: 0 } } },
     { a: 1, b: { nested: { deep: 1 } } }
   ])
-
-  t.mock.method(global, 'structuredClone', () => {
-    throw new Error('structuredClone is not supported')
-  })
-
-  events = []
-  $store.setKey('b.nested.deep', 3)
-  deepStrictEqual(events, [{ a: 1, b: { nested: { deep: 3 } } }])
-
-  t.mock.reset()
   unbind()
 })
 
@@ -384,6 +374,26 @@ test('passes previous value to subscribers', () => {
   unbind()
 })
 
+test('oldValue references the same object as the previous value', () => {
+  let events: ({ a: number } | undefined)[] = []
+  let $store = deepMap({ a: 0 })
+  let unbind = $store.subscribe((value, oldValue) => {
+    events.push(oldValue)
+  })
+
+  let oldValue1 = $store.value
+  $store.setKey('a', 1)
+  let oldValue2 = $store.value
+  $store.setKey('a', 2)
+
+  // Intentionally not using deepStrictEqual here - we're testing object reference equality
+  equal(events.length, 3)
+  equal(events[0], undefined)
+  equal(events[1], oldValue1)
+  equal(events[2], oldValue2)
+  unbind()
+})
+
 test('keys starting with numbers do not create unnecessary arrays', () => {
   let $store = deepMap<{ key?: { '100key': string } }>({})
 

deep-map/path.d.ts

@@ -103,14 +103,16 @@ export function getPath<T extends BaseDeepMap, K extends AllPaths<T>>(
 ): FromPath<T, K>
 
 /**
- * Set a deep value by key. Initialized arrays with `undefined`
- * if you set arbitrary length.
+ * Set a deep value by key. Makes a copy at each level of `path` so the input
+ * object is not mutated (but does not do a full deep copy -- references to
+ * objects will still be shared between input and output). Sparse arrays will
+ * be created if you set arbitrary length.
  *
  * ```
  * import { setPath } from 'nanostores'
  *
  * setPath({ a: { b: { c: [] } } }, 'a.b.c[1]', 'hey')
- * // Returns `{ a: { b: { c: [undefined, 'hey'] } } }`
+ * // Returns `{ a: { b: { c: [<empty>, 'hey'] } } }`
  * ```
  *
  * @param obj Any object.
@@ -124,14 +126,16 @@ export function setPath<T extends BaseDeepMap, K extends AllPaths<T>>(
 ): T
 
 /**
- * Set a deep value by path. Initialized arrays with `undefined`
- * if you set arbitrary length.
+ * Set a deep value by path. Makes a copy at each level of `path` so the input
+ * object is not mutated (but does not do a full deep copy -- references to
+ * objects will still be shared between input and output). Sparse arrays will
+ * be created if you set arbitrary length.
  *
  * ```
  * import { setByKey } from 'nanostores'
  *
  * setByKey({ a: { b: { c: [] } } }, ['a', 'b', 'c', 1], 'hey')
- * // Returns `{ a: { b: { c: [undefined, 'hey'] } } }`
+ * // Returns `{ a: { b: { c: [<empty>, 'hey'] } } }`
  * ```
  *
  * @param obj Any object.

deep-map/path.js

@@ -16,11 +16,10 @@ export function setPath(obj, path, value) {
 
 export function setByKey(obj, splittedKeys, value) {
   let key = splittedKeys[0]
-  ensureKey(obj, key, splittedKeys[1])
   let copy = Array.isArray(obj) ? [...obj] : { ...obj }
   if (splittedKeys.length === 1) {
     if (value === undefined) {
-      if (Array.isArray(obj)) {
+      if (Array.isArray(copy)) {
         copy.splice(key, 1)
       } else {
         delete copy[key]
@@ -30,9 +29,9 @@ export function setByKey(obj, splittedKeys, value) {
     }
     return copy
   }
-  let newVal = setByKey(obj[key], splittedKeys.slice(1), value)
-  obj[key] = newVal
-  return obj
+  ensureKey(copy, key, splittedKeys[1])
+  copy[key] = setByKey(copy[key], splittedKeys.slice(1), value)
+  return copy
 }
 
 const ARRAY_INDEX = /(.*)\[(\d+)\]/
@@ -58,7 +57,7 @@ function ensureKey(obj, key, nextKey) {
   let isNum = IS_NUMBER.test(nextKey)
 
   if (isNum) {
-    obj[key] = Array(parseInt(nextKey, 10) + 1).fill(undefined)
+    obj[key] = Array(parseInt(nextKey, 10) + 1)
   } else {
     obj[key] = {}
   }

deep-map/path.test.ts

@@ -58,57 +58,61 @@ test('creating objects', () => {
   type TestObj = { a?: { b?: { c?: { d: string } } } }
   let initial: TestObj = {}
 
-  setPath(initial, 'a.b.c.d', 'val')
+  initial = setPath(initial, 'a.b.c.d', 'val')
   equal(initial.a?.b?.c?.d, 'val')
 })
 
 test('creating arrays', () => {
   type TestObj = { a?: string[] }
   let initial: TestObj = {}
 
-  setPath(initial, 'a[0]', 'val')
+  initial = setPath(initial, 'a[0]', 'val')
   deepStrictEqual(initial, { a: ['val'] })
-  setPath(initial, 'a[3]', 'val3')
-  deepStrictEqual(initial, { a: ['val', undefined, undefined, 'val3'] })
+  initial = setPath(initial, 'a[3]', 'val3')
+  // The expected value is a sparse array
+  let expectedA = ['val']
+  expectedA[3] = 'val3'
+  deepStrictEqual(initial, { a: expectedA })
 })
 
 test('removes arrays', () => {
   type TestObj = { a?: string[] }
   let initial: TestObj = { a: ['a', 'b'] }
 
   // @ts-expect-error: incorrect key here
-  setPath(initial, 'a[1]', undefined)
+  initial = setPath(initial, 'a[1]', undefined)
   deepStrictEqual(initial, { a: ['a'] })
 
   // @ts-expect-error: incorrect key here
-  setPath(initial, 'a[0]', undefined)
+  initial = setPath(initial, 'a[0]', undefined)
   deepStrictEqual(initial, { a: [] })
 })
 
-test('changes object reference, when this level key is changed', () => {
+test('changes object reference at this level and earlier levels when key is changed', () => {
   type Obj = { a: { b: { c: number; d: string }; e: number } }
   let b = { c: 1, d: '1' }
   let a = { b, e: 1 }
 
   let initial: Obj = { a }
 
-  setPath(initial, 'a.b.c', 2)
-  equal(initial.a, a)
+  initial = setPath(initial, 'a.b.c', 2)
+  notEqual(initial.a, a)
   notEqual(initial.a.b, b)
 
-  setPath(initial, 'a.e', 2)
+  initial = setPath(initial, 'a.e', 2)
   notEqual(initial.a, a)
 })
 
-test('array items mutation changes identity on the same level', () => {
+test('array items mutation changes identity on the same and earlier levels', () => {
   let arr1 = { a: 1 }
   let arr2 = { a: 2 }
   let d = [arr1, arr2]
   let c = { d }
 
   let initial = { a: { b: { c } } }
   let newInitial = setPath(initial, 'a.b.c.d[1].a', 3)
-  equal(newInitial.a.b.c.d, d)
+  notEqual(newInitial, initial)
+  notEqual(newInitial.a.b.c.d, d)
   equal(newInitial.a.b.c.d[0], d[0])
   notEqual(newInitial.a.b.c.d[1], arr2)
   deepStrictEqual(newInitial.a.b.c.d[1], { a: 3 })