fixed issue with .has() when new value is added and then deleted or c… (

#981) * fixed issue with .has() when new value is added and then deleted or cleared * removed unneeded variable assignment * added tests for adding and removing data in proxySet and proxyMap to make sure it updates with useSnapshot * fixed typo in test * added epoch to proxySet and proxyMap to keep track of versioning * added tests for using .clear on proxyMap and proxySet * added more tests for useSnapshot * added comment for epoch in proxyMap
pmndrs · Oct 27, 2024 · 13a59d8 · 13a59d8
1 parent cd361f7
commit 13a59d8
Show file tree

Hide file tree

Showing 4 changed files with 520 additions and 12 deletions.
diff --git a/src/vanilla/utils/proxyMap.ts b/src/vanilla/utils/proxyMap.ts
@@ -6,6 +6,7 @@ const isProxy = (x: any) => proxyStateMap.has(x)
 type InternalProxyObject<K, V> = Map<K, V> & {
   data: Array<V>
   index: number
+  epoch: number
   toJSON: () => Map<K, V>
 }
 
@@ -68,6 +69,7 @@ export function proxyMap<K, V>(entries?: Iterable<[K, V]> | undefined | null) {
   const vObject: InternalProxyObject<K, V> = {
     data: initialData,
     index: initialIndex,
+    epoch: 0,
     get size() {
       if (!isProxy(this)) {
         registerSnapMap()
@@ -80,19 +82,17 @@ export function proxyMap<K, V>(entries?: Iterable<[K, V]> | undefined | null) {
       const index = map.get(key)
       if (index === undefined) {
         // eslint-disable-next-line @typescript-eslint/no-unused-expressions
-        this.index // touch property for tracking
+        this.epoch // touch property for tracking
         return undefined
       }
       return this.data[index]
     },
     has(key: K) {
       const map = getMapForThis(this)
       const exists = map.has(key)
-      if (!exists) {
-        // eslint-disable-next-line @typescript-eslint/no-unused-expressions
-        this.index // touch property for tracking
-      }
-      return exists
+      // eslint-disable-next-line @typescript-eslint/no-unused-expressions
+      this.epoch // touch property for tracking
+      return map.has(key)
     },
     set(key: K, value: V) {
       if (!isProxy(this)) {
@@ -105,6 +105,7 @@ export function proxyMap<K, V>(entries?: Iterable<[K, V]> | undefined | null) {
       } else {
         this.data[index] = value
       }
+      this.epoch++
       return this
     },
     delete(key: K) {
@@ -117,6 +118,7 @@ export function proxyMap<K, V>(entries?: Iterable<[K, V]> | undefined | null) {
       }
       delete this.data[index]
       indexMap.delete(key)
+      this.epoch++
       return true
     },
     clear() {
@@ -125,6 +127,7 @@ export function proxyMap<K, V>(entries?: Iterable<[K, V]> | undefined | null) {
       }
       this.data.length = 0 // empty array
       this.index = 0
+      this.epoch++
       indexMap.clear()
     },
     forEach(cb: (value: V, key: K, map: Map<K, V>) => void) {
@@ -166,6 +169,7 @@ export function proxyMap<K, V>(entries?: Iterable<[K, V]> | undefined | null) {
   Object.defineProperties(proxiedObject, {
     size: { enumerable: false },
     index: { enumerable: false },
+    epoch: { enumerable: false },
     data: { enumerable: false },
     toJSON: { enumerable: false },
   })

diff --git a/src/vanilla/utils/proxySet.ts b/src/vanilla/utils/proxySet.ts
@@ -8,6 +8,7 @@ type InternalProxySet<T> = Set<T> & {
   data: T[]
   toJSON: object
   index: number
+  epoch: number
   intersection: (other: Set<T>) => Set<T>
   isDisjointFrom: (other: Set<T>) => boolean
   isSubsetOf: (other: Set<T>) => boolean
@@ -63,6 +64,7 @@ export function proxySet<T>(initialValues?: Iterable<T> | null) {
   const vObject: InternalProxySet<T> = {
     data: initialData,
     index: initialIndex,
+    epoch: 0,
     get size() {
       if (!isProxy(this)) {
         registerSnapMap()
@@ -72,12 +74,9 @@ export function proxySet<T>(initialValues?: Iterable<T> | null) {
     has(value: T) {
       const map = getMapForThis(this)
       const v = maybeProxify(value)
-      const exists = map.has(v)
-      if (!exists) {
-        // eslint-disable-next-line @typescript-eslint/no-unused-expressions
-        this.index // touch property for tracking
-      }
-      return exists
+      // eslint-disable-next-line @typescript-eslint/no-unused-expressions
+      this.epoch // touch property for tracking
+      return map.has(v)
     },
     add(value: T) {
       if (!isProxy(this)) {
@@ -87,6 +86,7 @@ export function proxySet<T>(initialValues?: Iterable<T> | null) {
       if (!indexMap.has(v)) {
         indexMap.set(v, this.index)
         this.data[this.index++] = v
+        this.epoch++
       }
       return this
     },
@@ -101,6 +101,7 @@ export function proxySet<T>(initialValues?: Iterable<T> | null) {
       }
       delete this.data[index]
       indexMap.delete(v)
+      this.epoch++
       return true
     },
     clear() {
@@ -109,6 +110,7 @@ export function proxySet<T>(initialValues?: Iterable<T> | null) {
       }
       this.data.length = 0 // empty array
       this.index = 0
+      this.epoch++
       indexMap.clear()
     },
     forEach(cb) {
@@ -200,6 +202,8 @@ export function proxySet<T>(initialValues?: Iterable<T> | null) {
   Object.defineProperties(proxiedObject, {
     size: { enumerable: false },
     data: { enumerable: false },
+    index: { enumerable: false },
+    epoch: { enumerable: false },
     toJSON: { enumerable: false },
   })
   Object.seal(proxiedObject)

diff --git a/tests/proxyMap.test.tsx b/tests/proxyMap.test.tsx
@@ -319,6 +319,7 @@ describe('proxyMap internal', () => {
     ).toBe(false)
   })
 })
+
 describe('snapshot', () => {
   it('should error when trying to mutate a snapshot', () => {
     const state = proxyMap()
@@ -375,3 +376,249 @@ describe('snapshot', () => {
     expect(snap2.get('key1')).toBe('val1modified')
   })
 })
+
+describe('ui updates - useSnapshot', async () => {
+  it('should update ui when calling has before and after setting and deleting a key', async () => {
+    const state = proxyMap()
+    const TestComponent = () => {
+      const snap = useSnapshot(state)
+
+      return (
+        <>
+          <p>has key: {`${snap.has('key')}`}</p>
+          <button onClick={() => state.set('key', 'value')}>set key</button>
+          <button onClick={() => state.delete('key')}>delete key</button>
+        </>
+      )
+    }
+
+    const { getByText } = render(
+      <StrictMode>
+        <TestComponent />
+      </StrictMode>,
+    )
+
+    await waitFor(() => {
+      getByText('has key: false')
+    })
+
+    fireEvent.click(getByText('set key'))
+    await waitFor(() => {
+      getByText('has key: true')
+    })
+
+    fireEvent.click(getByText('delete key'))
+    await waitFor(() => {
+      getByText('has key: false')
+    })
+  })
+
+  it('should update ui when calling has before and after settiing and deleting multiple keys', async () => {
+    const state = proxyMap()
+    const TestComponent = () => {
+      const snap = useSnapshot(state)
+
+      return (
+        <>
+          <p>has key: {`${snap.has('key')}`}</p>
+          <p>has key2: {`${snap.has('key2')}`}</p>
+          <button
+            onClick={() => {
+              state.set('key', 'value')
+              state.set('key2', 'value')
+            }}
+          >
+            set keys
+          </button>
+          <button
+            onClick={() => {
+              state.delete('key')
+              state.delete('key2')
+            }}
+          >
+            delete keys
+          </button>
+        </>
+      )
+    }
+
+    const { getByText } = render(
+      <StrictMode>
+        <TestComponent />
+      </StrictMode>,
+    )
+
+    await waitFor(() => {
+      getByText('has key: false')
+      getByText('has key2: false')
+    })
+
+    fireEvent.click(getByText('set keys'))
+    await waitFor(() => {
+      getByText('has key: true')
+      getByText('has key2: true')
+    })
+
+    fireEvent.click(getByText('delete keys'))
+    await waitFor(() => {
+      getByText('has key: false')
+      getByText('has key2: false')
+    })
+  })
+
+  it('should update ui when calling has before and after settiing multile keys and deleting a single one (first item)', async () => {
+    const state = proxyMap()
+    const TestComponent = () => {
+      const snap = useSnapshot(state)
+
+      return (
+        <>
+          <p>has key: {`${snap.has('key')}`}</p>
+          <p>has key2: {`${snap.has('key2')}`}</p>
+          <button
+            onClick={() => {
+              state.set('key', 'value')
+              state.set('key2', 'value')
+            }}
+          >
+            set keys
+          </button>
+          <button
+            onClick={() => {
+              state.delete('key')
+            }}
+          >
+            delete keys
+          </button>
+        </>
+      )
+    }
+
+    const { getByText } = render(
+      <StrictMode>
+        <TestComponent />
+      </StrictMode>,
+    )
+
+    await waitFor(() => {
+      getByText('has key: false')
+      getByText('has key2: false')
+    })
+
+    fireEvent.click(getByText('set keys'))
+    await waitFor(() => {
+      getByText('has key: true')
+      getByText('has key2: true')
+    })
+
+    fireEvent.click(getByText('delete keys'))
+    await waitFor(() => {
+      getByText('has key: false')
+      getByText('has key2: true')
+    })
+  })
+
+  it('should update ui when calling has before and after settiing multile keys and deleting a single one (first item)', async () => {
+    const state = proxyMap()
+    const TestComponent = () => {
+      const snap = useSnapshot(state)
+
+      return (
+        <>
+          <p>has key: {`${snap.has('key')}`}</p>
+          <p>has key2: {`${snap.has('key2')}`}</p>
+          <button
+            onClick={() => {
+              state.set('key', 'value')
+              state.set('key2', 'value')
+            }}
+          >
+            set keys
+          </button>
+          <button
+            onClick={() => {
+              state.delete('key2')
+            }}
+          >
+            delete keys
+          </button>
+        </>
+      )
+    }
+
+    const { getByText } = render(
+      <StrictMode>
+        <TestComponent />
+      </StrictMode>,
+    )
+
+    await waitFor(() => {
+      getByText('has key: false')
+      getByText('has key2: false')
+    })
+
+    fireEvent.click(getByText('set keys'))
+    await waitFor(() => {
+      getByText('has key: true')
+      getByText('has key2: true')
+    })
+
+    fireEvent.click(getByText('delete keys'))
+    await waitFor(() => {
+      getByText('has key: true')
+      getByText('has key2: false')
+    })
+  })
+
+  it('should update ui when clearing the map', async () => {
+    const state = proxyMap()
+    const TestComponent = () => {
+      const snap = useSnapshot(state)
+
+      return (
+        <>
+          <p>has key: {`${snap.has('key')}`}</p>
+          <p>has key2: {`${snap.has('key2')}`}</p>
+          <button
+            onClick={() => {
+              state.set('key', 'value')
+              state.set('key2', 'value')
+            }}
+          >
+            set keys
+          </button>
+          <button
+            onClick={() => {
+              state.clear()
+            }}
+          >
+            clear map
+          </button>
+        </>
+      )
+    }
+
+    const { getByText } = render(
+      <StrictMode>
+        <TestComponent />
+      </StrictMode>,
+    )
+
+    await waitFor(() => {
+      getByText('has key: false')
+      getByText('has key2: false')
+    })
+
+    fireEvent.click(getByText('set keys'))
+    await waitFor(() => {
+      getByText('has key: true')
+      getByText('has key2: true')
+    })
+
+    fireEvent.click(getByText('clear map'))
+    await waitFor(() => {
+      getByText('has key: false')
+      getByText('has key2: false')
+    })
+  })
+})