Repository: pmndrs/valtio
Pull request #330 (2022-01-19 06:48:10)

feat(utils): add proxyMap 
WIP on adding the `proxyMap` util. For the implementation I thought about 2 solutions.
Using arrays vs using a Map and an Object.

I went for the latter. This allows us to lookup value with an O(1) (O(2) in that case as we have 2 lookups to do) complexity no matter the size of our state.
With arrays, we would have to iterate to find values and the complexity would increase with the size of our state.

Edit: went with array as Map was breaking snapshot behavior


vercel[bot]: This pull request is being automatically deployed with Vercel ([learn more](https://vercel.link/github-learn-more)).  
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: [https://vercel.com/pmndrs/valtio/5eu4cNBov99XruVss1LegiqEyzP3](https://vercel.com/pmndrs/valtio/5eu4cNBov99XruVss1LegiqEyzP3)  
✅ Preview: *In Progress*


codesandbox[bot]: This pull request is automatically built and testable in [CodeSandbox](https://codesandbox.io).

  To see build info of the built libraries, click [here](https://ci.codesandbox.io/status/pmndrs/valtio/pr/330/builds/209283) or the icon next to each commit SHA.

Latest deployment of this branch, based on commit d3df4cb0e9d097ae6341d6816a4f4951663cdd4f:

|Sandbox| Source |
|--|--|
|[React](https://codesandbox.io/s/react-95mc9)| Configuration |
|[React Typescript](https://codesandbox.io/s/react-typescript-2gcwq)| Configuration |
|[React Browserify](https://codesandbox.io/s/react-browserify-20t2f)| Configuration |
|[React Snowpack](https://codesandbox.io/s/react-snowpack-7lqg6)| Configuration |
|[React Parcel](https://codesandbox.io/s/react-parcel-60m27)| Configuration |

fkhadra: > I agree O(1) lookup would be desirable, but at least this doesn't seem to work.
> 
> ```js
> const p = proxyMap([[1,'a']])
> const s = snapshot(p)
> p.delete(1)
> s.has(1) // should be true
> ```

With the current implementation, only values are proxies. `keys` remain the way they are. Do we want to proxy keys as well? If yes what would be the benefit?
dai-shi: > what would be the benefit?

It's not about the benefit. It should work correctly.
Basically, `proxyMap(new Map())` should behave like `proxy({})`, and it also supports any keys.
fkhadra: > > what would be the benefit?
> 
> It's not about the benefit. It should work correctly. Basically, `proxyMap(new Map())` should behave like `proxy({})`, and it also supports any keys.

Idd, misunderstanding from my side ^^
fkhadra: I'll add more tests, notably around the snapshotting behavior to ensure that I'm not breaking it.

Regarding our discussion around `ref`, I was wondering if it's a good idea to add something about it in the example block, doing so it would be visible in the IDE. WDYT?

![Screenshot 2022-01-19 at 21 49 15](https://user-images.githubusercontent.com/5574267/150211438-b10521c5-3a3d-45e6-bd9f-adc3d750808f.png)


dai-shi: > Regarding our discussion around `ref`, I was wondering if it's a good idea to add something about it in the example block, doing so it would be visible in the IDE. WDYT?

I think it's a good idea. I'm not totally sure about various use cases though. (that's why I avoid changing the library code, so the example block idea works for me.)
fkhadra: As soon as it's merged I'll open a PR with the example then 

Patch:
diff --git a/readme.md b/readme.md
index 1a9ed43..a34d14e 100644
--- a/readme.md
+++ b/readme.md
@@ -368,6 +368,21 @@ state.delete(1)
 state.forEach(v => console.log(v)) // 2,3,4
 ```
 
+#### `proxyMap` util
+
+This is to create a proxy which emulate the native Map behavior. The API is the same as Map API
+
+```js
+
+import { proxyMap } from 'valtio/utils'
+ 
+const state = proxyMap([["key", "value"], ["key2", "value2"]])
+state.set("key", "value")
+state.delete("key")
+state.get("key") // ---> value
+state.forEach((value, key) => console.log(key, value)) // ---> "key", "value", "key2", "value2"
+```
+
 #### Plugins
 
 - [eslint-plugin-valtio](https://github.com/pmndrs/eslint-plugin-valtio)
diff --git a/src/utils.ts b/src/utils.ts
index f7c1f3a..14c8614 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -10,3 +10,4 @@ export { addComputed_DEPRECATED as addComputed } from './utils/addComputed'
 export { proxyWithComputed } from './utils/proxyWithComputed'
 export { proxyWithHistory } from './utils/proxyWithHistory'
 export { proxySet } from './utils/proxySet'
+export { proxyMap } from './utils/proxyMap'
diff --git a/src/utils/proxyMap.ts b/src/utils/proxyMap.ts
new file mode 100644
index 0000000..75f6ea7
--- /dev/null
+++ b/src/utils/proxyMap.ts
@@ -0,0 +1,111 @@
+import { proxy } from 'valtio'
+
+type KeyValRecord<K, V> = [key: K, value: V]
+
+type InternalProxyMap<K, V> = Map<K, V> & {
+  data: KeyValRecord<K, V>[]
+  toJSON: object
+}
+
+/**
+ * proxyMap
+ *
+ * This is to create a proxy which mimic the native Map behavior.
+ * The API is the same as Map API
+ *
+ * @example
+ * import { proxyMap } from 'valtio/utils'
+ * const state = proxyMap([["key", "value"]])
+ *
+ * //can be used inside a proxy as well
+ * const state = proxy({
+ *   count: 1,
+ *   map: proxyMap()
+ * })
+ *
+ * // When using an object as a key, you can wrap it with `ref` so it's not proxied
+ * // this is useful if you want to preserve the key equality
+ * import { ref } from 'valtio'
+ *
+ * const key = ref({})
+ * state.set(key, "value")
+ * state.get(key) //value
+ *
+ * const key = {}
+ * state.set(key, "value")
+ * state.get(key) //undefined
+ */
+export const proxyMap = <K, V>(
+  entries?: Iterable<readonly [K, V]> | null
+): Map<K, V> => {
+  const map: InternalProxyMap<K, V> = proxy({
+    data: Array.from(entries || []) as KeyValRecord<K, V>[],
+    has(key) {
+      return this.data.some((p) => p[0] === key)
+    },
+    set(key, value) {
+      const record = this.data.find((p) => p[0] === key)
+      if (record) {
+        record[1] = value
+      } else {
+        this.data.push([key, value])
+      }
+      return this
+    },
+    get(key) {
+      return this.data.find((p) => p[0] === key)?.[1]
+    },
+    delete(key) {
+      const index = this.data.findIndex((p) => p[0] === key)
+      if (index === -1) {
+        return false
+      }
+      this.data.splice(index, 1)
+      return true
+    },
+    clear() {
+      this.data.splice(0)
+    },
+    get size() {
+      return this.data.length
+    },
+    toJSON() {
+      return {}
+    },
+    forEach(cb) {
+      this.data.forEach((p) => {
+        cb(p[1], p[0], this)
+      })
+    },
+    keys() {
+      return this.data.map((p) => p[0]).values()
+    },
+    values() {
+      return this.data.map((p) => p[1]).values()
+    },
+    entries() {
+      return new Map(this.data).entries()
+    },
+    get [Symbol.toStringTag]() {
+      return 'Map'
+    },
+    [Symbol.iterator]() {
+      return this.entries()
+    },
+  })
+
+  Object.defineProperties(map, {
+    data: {
+      enumerable: false,
+    },
+    size: {
+      enumerable: false,
+    },
+    toJSON: {
+      enumerable: false,
+    },
+  })
+  Object.seal(map)
+
+  return map as Map<K, V>
+}
diff --git a/src/utils/proxySet.ts b/src/utils/proxySet.ts
index c45eae6..2f0ac17 100644
--- a/src/utils/proxySet.ts
+++ b/src/utils/proxySet.ts
@@ -21,7 +21,7 @@ type InternalProxySet<T> = Set<T> & {
  *   set: proxySet()
  * })
  */
-export const proxySet = <T>(initialValues: Iterable<T> | null = []): Set<T> => {
+export const proxySet = <T>(initialValues?: Iterable<T> | null): Set<T> => {
   const set: InternalProxySet<T> = proxy({
     data: Array.from(new Set(initialValues)),
     has(value) {


Test patch:
diff --git a/tests/proxyMap.test.tsx b/tests/proxyMap.test.tsx
new file mode 100644
index 0000000..23717f4
--- /dev/null
+++ b/tests/proxyMap.test.tsx
@@ -0,0 +1,331 @@
+import { StrictMode } from 'react'
+import { fireEvent, render, waitFor } from '@testing-library/react'
+import { proxy, useSnapshot } from 'valtio'
+import { proxyMap, proxySet } from 'valtio/utils'
+
+const consoleError = console.error
+beforeEach(() => {
+  console.error = jest.fn((message) => {
+    if (
+      process.env.NODE_ENV === 'production' &&
+      message.startsWith('act(...) is not supported in production')
+    ) {
+      return
+    }
+
+    consoleError(message)
+  })
+})
+afterEach(() => {
+  console.error = consoleError
+})
+
+const initialValues = [
+  {
+    name: 'number',
+    value: [[-10, 1]],
+  },
+  {
+    name: 'string',
+    value: [['hello', 'world']],
+  },
+  {
+    name: 'Symbol',
+    value: [[Symbol(), Symbol()]],
+  },
+  {
+    name: 'boolean',
+    value: [[false, true]],
+  },
+  {
+    name: 'array',
+    value: [
+      [
+        [10, 'hello'],
+        [1, 2, 3, 'x', 'w'],
+      ],
+    ],
+  },
+  {
+    name: 'object',
+    value: [[{}, { id: 'somehting', field: null }]],
+  },
+  {
+    name: 'null',
+    value: [[null, [null, 1]]],
+  },
+  {
+    name: 'function',
+    value: [[() => {}, () => {}]],
+  },
+  {
+    name: 'array buffer',
+    value: [[new ArrayBuffer(8), new ArrayBuffer(8)]],
+  },
+  {
+    name: 'Set',
+    value: [[new Set(), new Set(['x', 'y', 'z'])]],
+  },
+  {
+    name: 'Map',
+    value: [[new Map(), new Map([['key', 'value']])]],
+  },
+  {
+    name: 'proxySet',
+    value: [[proxySet([{}]), proxySet([{}, Symbol()])]],
+  },
+]
+
+const inputValues = [
+  {
+    name: 'array',
+    value: [1, 'hello'],
+  },
+  {
+    name: 'nested array',
+    value: [[1, 'hello']],
+  },
+  {
+    name: 'Map',
+    value: new Map<any, any>([
+      ['key1', 'value1'],
+      [{}, 'value2'],
+    ]),
+  },
+  {
+    name: 'boolean',
+    value: false,
+  },
+  {
+    name: 'number',
+    value: 123,
+  },
+  {
+    name: 'string',
+    value: 'hello',
+  },
+  {
+    name: 'Set',
+    value: new Set([1, 2, 3]),
+  },
+  {
+    name: 'proxySet',
+    value: proxySet([1, {}, null, 'xyz', Symbol()]),
+  },
+  {
+    name: 'object',
+    value: { id: Symbol(), field: 'field', bool: true, null: null },
+  },
+]
+
+describe('features parity with native Map', () => {
+  initialValues.forEach(({ name, value }) => {
+    it(`Support Map operations on ${name}`, () => {
+      const map = proxyMap(value as any)
+      const nativeMap = new Map(value as any)
+
+      // check for Symbol.toStringTag / toString
+      expect(`${map}`).toBe(`${nativeMap}`)
+
+      const expectOutputToMatch = () => {
+        expect(map.size).toStrictEqual(nativeMap.size)
+        expect(Array.from(map.values())).toStrictEqual(
+          Array.from(nativeMap.values())
+        )
+        expect(Array.from(map.keys())).toStrictEqual(
+          Array.from(nativeMap.keys())
+        )
+        expect(Array.from(map.entries())).toStrictEqual(
+          Array.from(nativeMap.entries())
+        )
+        expect(JSON.stringify(map)).toStrictEqual(JSON.stringify(nativeMap))
+
+        // cover loops
+        const handleForEach = jest.fn()
+        const handleForOf = jest.fn()
+
+        map.forEach(handleForEach)
+        expect(handleForEach).toHaveBeenCalledTimes(map.size)
+
+        for (const _ of map) {
+          handleForOf()
+        }
+
+        expect(handleForOf).toHaveBeenCalledTimes(map.size)
+      }
+
+      expectOutputToMatch()
+
+      const [firstElementFromMap] = map
+      const [firstElementFromNativeMap] = nativeMap
+
+      // Bypass Forbidden non-null assertion
+      const keyFromMap = firstElementFromMap && firstElementFromMap[0]
+      const keyFromNativeMap =
+        firstElementFromNativeMap && firstElementFromNativeMap[0]
+
+      expect(map.has(keyFromMap)).toBe(nativeMap.has(keyFromNativeMap))
+
+      map.delete(keyFromMap)
+      nativeMap.delete(keyFromNativeMap)
+      expectOutputToMatch()
+
+      map.set('newKey', {})
+      nativeMap.set('newKey', {})
+      expectOutputToMatch()
+
+      // test value replacement
+      map.set('newKey', 'newValue')
+      nativeMap.set('newKey', 'newValue')
+      expectOutputToMatch()
+
+      // test getter
+      expect(map.get('newKey')).toBe(nativeMap.get('newKey'))
+    })
+  })
+
+  it('support initialization with null', () => {
+    const map = proxyMap(null)
+    const nativeMap = new Map(null)
+
+    expect(map.size).toStrictEqual(nativeMap.size)
+    expect(Array.from(map.values())).toStrictEqual(
+      Array.from(nativeMap.values())
+    )
+    expect(Array.from(map.keys())).toStrictEqual(Array.from(nativeMap.keys()))
+    expect(Array.from(map.entries())).toStrictEqual(
+      Array.from(nativeMap.entries())
+    )
+  })
+})
+
+describe('clear map', () => {
+  initialValues.forEach(({ name, value }) => {
+    it(`clear proxyMap of ${name}`, async () => {
+      const state = proxyMap(value as any)
+
+      const TestComponent = () => {
+        const snap = useSnapshot(state)
+
+        return (
+          <>
+            <div>size: {snap.size}</div>
+            <button onClick={() => state.clear()}>button</button>
+          </>
+        )
+      }
+
+      const { getByText } = render(
+        <StrictMode>
+          <TestComponent />
+        </StrictMode>
+      )
+
+      expect(state.size).toBeGreaterThan(0)
+      getByText(`size: ${state.size}`)
+
+      fireEvent.click(getByText('button'))
+      await waitFor(() => {
+        getByText('size: 0')
+      })
+    })
+  })
+})
+
+describe('add value', () => {
+  inputValues.forEach(({ name, value }) => {
+    it(`update size when adding ${name}`, async () => {
+      const state = proxy({
+        map: proxyMap(),
+      })
+
+      const TestComponent = () => {
+        const snap = useSnapshot(state)
+
+        return (
+          <>
+            <div>size: {snap.map.size}</div>
+            <button onClick={() => state.map.set(value, value)}>button</button>
+          </>
+        )
+      }
+
+      const { getByText } = render(
+        <StrictMode>
+          <TestComponent />
+        </StrictMode>
+      )
+
+      getByText('size: 0')
+      fireEvent.click(getByText('button'))
+
+      await waitFor(() => {
+        getByText('size: 1')
+      })
+    })
+  })
+})
+
+describe('delete', () => {
+  inputValues.forEach(({ name, value }) => {
+    it(`return false when trying to delete non-existing value of type ${name}`, () => {
+      const set = proxyMap()
+
+      expect(set.delete(value)).toBe(false)
+    })
+  })
+
+  initialValues.forEach(({ name, value }) => {
+    it(`support delete on key of type ${name}`, async () => {
+      const state = proxy({
+        map: proxyMap(value as any),
+      })
+
+      // pick a random value from the set
+      const [firstValue] = state.map
+
+      // Bypass Forbidden non-null assertion
+      const firstKey = firstValue && firstValue[0]
+
+      const TestComponent = () => {
+        const snap = useSnapshot(state)
+
+        return (
+          <>
+            <div>size: {snap.map.size}</div>
+            <button onClick={() => state.map.delete(firstKey)}>button</button>
+          </>
+        )
+      }
+
+      const { getByText } = render(
+        <StrictMode>
+          <TestComponent />
+        </StrictMode>
+      )
+
+      getByText(`size: ${state.map.size}`)
+
+      const expectedSizeAfterDelete =
+        state.map.size > 1 ? state.map.size - 1 : 0
+
+      fireEvent.click(getByText('button'))
+      await waitFor(() => {
+        getByText(`size: ${expectedSizeAfterDelete}`)
+      })
+    })
+  })
+})
+
+describe('proxyMap internal', () => {
+  it('should be sealed', () => {
+    expect(Object.isSealed(proxySet())).toBe(true)
+  })
+
+  it('should list only enumerable properties', () => {
+    const notEnumerableProps = ['data', 'size', 'toJSON']
+    expect(
+      Object.keys(proxyMap()).some((k) => notEnumerableProps.includes(k))
+    ).toBe(false)
+  })
+})