Repository: pmndrs/valtio
Pull request #24 (2020-11-23 00:11:46)

unsupported map/set, improve delete property
Map/Set are not supported, but they should be at least usable as non tracking objects.

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/24/builds/75888) or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 32fcc537d94a71435259d4ed645bdf55f1436855:

|Sandbox| Source |
|--|--|
|[React](https://codesandbox.io/s/react-8vy99)| Configuration |
|[React Typescript](https://codesandbox.io/s/react-typescript-i213s)| Configuration |

dai-shi: This is not perfect (as `WeakMap` is not detected), but we have a larger issue (proxy-compare only supports plain objects). Meanwhile, this could be an intermediate solution.

Patch:
diff --git a/.size-snapshot.json b/.size-snapshot.json
index 29dcc86..ee388db 100644
--- a/.size-snapshot.json
+++ b/.size-snapshot.json
@@ -1,8 +1,8 @@
 {
   "index.js": {
-    "bundled": 5558,
-    "minified": 2455,
-    "gzipped": 1049,
+    "bundled": 6043,
+    "minified": 2711,
+    "gzipped": 1123,
     "treeshaked": {
       "rollup": {
         "code": 61,
@@ -14,19 +14,19 @@
     }
   },
   "index.cjs.js": {
-    "bundled": 6288,
-    "minified": 2881,
-    "gzipped": 1153
+    "bundled": 6779,
+    "minified": 3129,
+    "gzipped": 1229
   },
   "index.iife.js": {
-    "bundled": 6688,
-    "minified": 2208,
-    "gzipped": 1011
+    "bundled": 7195,
+    "minified": 2395,
+    "gzipped": 1076
   },
   "vanilla.js": {
-    "bundled": 3785,
-    "minified": 1646,
-    "gzipped": 763,
+    "bundled": 4270,
+    "minified": 1902,
+    "gzipped": 835,
     "treeshaked": {
       "rollup": {
         "code": 22,
@@ -38,8 +38,8 @@
     }
   },
   "vanilla.cjs.js": {
-    "bundled": 4265,
-    "minified": 1936,
-    "gzipped": 853
+    "bundled": 4756,
+    "minified": 2184,
+    "gzipped": 934
   }
 }
diff --git a/src/vanilla.ts b/src/vanilla.ts
index 5995c85..3743c23 100644
--- a/src/vanilla.ts
+++ b/src/vanilla.ts
@@ -4,8 +4,10 @@ const VERSION = Symbol()
 const LISTENERS = Symbol()
 const SNAPSHOT = Symbol()
 
-const isObject = (x: unknown): x is object =>
-  typeof x === 'object' && x !== null
+const isSupportedObject = (x: unknown): x is object =>
+  typeof x === 'object' &&
+  x !== null &&
+  (Array.isArray(x) || !(x as any)[Symbol.iterator])
 
 const proxyCache = new WeakMap<object, object>()
 let globalVersion = 0
@@ -18,6 +20,9 @@ const snapshotCache = new WeakMap<
 >()
 
 export const proxy = <T extends object>(initialObject: T = {} as T): T => {
+  if (!isSupportedObject(initialObject)) {
+    throw new Error('unsupported object type')
+  }
   let version = globalVersion
   const listeners = new Set<(nextVersion: number) => void>()
   const notifyUpdate = (nextVersion?: number) => {
@@ -45,11 +50,13 @@ export const proxy = <T extends object>(initialObject: T = {} as T): T => {
         if (cache && cache.version === version) {
           return cache.snapshot
         }
-        const snapshot = Object.create(target.constructor.prototype)
+        const snapshot = Array.isArray(target)
+          ? []
+          : Object.create(target.constructor?.prototype || null)
         snapshotCache.set(receiver, { version, snapshot })
         Reflect.ownKeys(target).forEach((key) => {
           const value = target[key]
-          if (!isObject(value)) {
+          if (!isSupportedObject(value)) {
             snapshot[key] = value
           } else if (value instanceof Promise) {
             Object.defineProperty(snapshot, key, {
@@ -72,24 +79,29 @@ export const proxy = <T extends object>(initialObject: T = {} as T): T => {
       return target[prop]
     },
     deleteProperty(target, prop) {
-      const value = target[prop]
-      const childListeners = isObject(value) && (value as any)[LISTENERS]
+      const prevValue = target[prop]
+      const childListeners =
+        isSupportedObject(prevValue) && (prevValue as any)[LISTENERS]
       if (childListeners) {
         childListeners.delete(notifyUpdate)
       }
-      delete target[prop]
-      notifyUpdate()
-      return true
+      const deleted = Reflect.deleteProperty(target, prop)
+      if (deleted) {
+        notifyUpdate()
+      }
+      return deleted
     },
     set(target, prop, value, receiver) {
-      if (Object.is(target[prop], value)) {
+      const prevValue = target[prop]
+      if (Object.is(prevValue, value)) {
         return true
       }
-      const childListeners = isObject(target[prop]) && target[prop][LISTENERS]
+      const childListeners =
+        isSupportedObject(prevValue) && (prevValue as any)[LISTENERS]
       if (childListeners) {
         childListeners.delete(notifyUpdate)
       }
-      if (!isObject(value)) {
+      if (!isSupportedObject(value)) {
         target[prop] = value
       } else if (value instanceof Promise) {
         target[prop] = value.then((v) => {


Test patch:
diff --git a/tests/basic.test.tsx b/tests/basic.test.tsx
index 9f8f068..0d956d8 100644
--- a/tests/basic.test.tsx
+++ b/tests/basic.test.tsx
@@ -27,7 +27,7 @@ it('simple counter', async () => {
   await findByText('count: 1')
 })
 
-it('no extra re-renders', async () => {
+it.only('no extra re-renders', async () => {
   const obj = proxy({ count: 0, count2: 0 })
 
   const Counter: React.FC = () => {
@@ -177,6 +177,31 @@ it('array length after direct assignment', async () => {
   await findByText('counts: 0,1,2,3,,,,,,9')
 })
 
+it('deleting property', async () => {
+  const obj = proxy<{ count?: number }>({ count: 1 })
+
+  const Counter: React.FC = () => {
+    const snapshot = useProxy(obj)
+    return (
+      <>
+        <div>count: {snapshot.count ?? 'none'}</div>
+        <button onClick={() => delete obj.count}>button</button>
+      </>
+    )
+  }
+
+  const { getByText, findByText } = render(
+    <StrictMode>
+      <Counter />
+    </StrictMode>
+  )
+
+  await findByText('count: 1')
+
+  fireEvent.click(getByText('button'))
+  await findByText('count: none')
+})
+
 it('circular object', async () => {
   const obj = proxy<any>({ object: {} })
   obj.object = obj
diff --git a/tests/mapset.test.tsx b/tests/mapset.test.tsx
new file mode 100644
index 0000000..4a0a09e
--- /dev/null
+++ b/tests/mapset.test.tsx
@@ -0,0 +1,55 @@
+import React, { StrictMode } from 'react'
+import { fireEvent, render } from '@testing-library/react'
+import { proxy, useProxy } from '../src/index'
+
+it('unsupported map', async () => {
+  const obj = proxy({ map: new Map([['count', 0]]) })
+
+  const Counter: React.FC = () => {
+    const snapshot = useProxy(obj)
+    return (
+      <>
+        <div>count: {snapshot.map.get('count')}</div>
+        <button onClick={() => obj.map.set('count', 1)}>button</button>
+      </>
+    )
+  }
+
+  const { getByText, findByText } = render(
+    <StrictMode>
+      <Counter />
+    </StrictMode>
+  )
+
+  await findByText('count: 0')
+
+  fireEvent.click(getByText('button'))
+  await new Promise((resolve) => setTimeout(resolve, 10)) // FIXME better way?
+  expect(() => getByText('count: 1')).toThrow()
+})
+
+it('unsupported set', async () => {
+  const obj = proxy({ set: new Set([1, 2, 3]) })
+
+  const Counter: React.FC = () => {
+    const snapshot = useProxy(obj)
+    return (
+      <>
+        <div>count: {[...snapshot.set].join(',')}</div>
+        <button onClick={() => obj.set.add(4)}>button</button>
+      </>
+    )
+  }
+
+  const { getByText, findByText } = render(
+    <StrictMode>
+      <Counter />
+    </StrictMode>
+  )
+
+  await findByText('count: 1,2,3')
+
+  fireEvent.click(getByText('button'))
+  await new Promise((resolve) => setTimeout(resolve, 10)) // FIXME better way?
+  expect(() => getByText('count: 1,2,3,4')).toThrow()
+})