Revert "feat(vanilla): Support class fields (defineProperty) (#760)" (#…

…766) This reverts commit c0bda5a. Due to what I presume is a bug in the Hermes engine, React Native projects cannot use Valtio 1.11.0. Let's revert #760 to support existing React Native users.
pmndrs · Jul 27, 2023 · 6eac2fc · 6eac2fc · vercel · Jul 27, 2023
1 parent 6f0eb3a
commit 6eac2fc
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 103 deletions.
diff --git a/src/vanilla.ts b/src/vanilla.ts
@@ -86,9 +86,6 @@ const buildProxyFunction = (
     !(x instanceof RegExp) &&
     !(x instanceof ArrayBuffer),
 
-  shouldTrapDefineProperty = (desc: PropertyDescriptor) =>
-    desc.configurable && desc.enumerable && desc.writable,
-
   defaultHandlePromise = <P extends Promise<any>>(
     promise: P & {
       status?: 'pending' | 'fulfilled' | 'rejected'
@@ -247,50 +244,6 @@ const buildProxyFunction = (
     const baseObject = Array.isArray(initialObject)
       ? []
       : Object.create(Object.getPrototypeOf(initialObject))
-    const trapSet = (
-      hasPrevValue: boolean,
-      prevValue: any,
-      prop: string | symbol,
-      value: any,
-      setValue: (nextValue: any) => void
-    ) => {
-      if (
-        hasPrevValue &&
-        (objectIs(prevValue, value) ||
-          (proxyCache.has(value) && objectIs(prevValue, proxyCache.get(value))))
-      ) {
-        return
-      }
-      removePropListener(prop)
-      if (isObject(value)) {
-        value = getUntracked(value) || value
-      }
-      let nextValue = value
-      if (value instanceof Promise) {
-        value
-          .then((v) => {
-            value.status = 'fulfilled'
-            value.value = v
-            notifyUpdate(['resolve', [prop], v])
-          })
-          .catch((e) => {
-            value.status = 'rejected'
-            value.reason = e
-            notifyUpdate(['reject', [prop], e])
-          })
-      } else {
-        if (!proxyStateMap.has(value) && canProxy(value)) {
-          nextValue = proxyFunction(value)
-        }
-        const childProxyState =
-          !refSet.has(nextValue) && proxyStateMap.get(nextValue)
-        if (childProxyState) {
-          addPropListener(prop, childProxyState)
-        }
-      }
-      setValue(nextValue)
-      notifyUpdate(['set', [prop], value, prevValue])
-    }
     const handler: ProxyHandler<T> = {
       deleteProperty(target: T, prop: string | symbol) {
         const prevValue = Reflect.get(target, prop)
@@ -304,35 +257,44 @@ const buildProxyFunction = (
       set(target: T, prop: string | symbol, value: any, receiver: object) {
         const hasPrevValue = Reflect.has(target, prop)
         const prevValue = Reflect.get(target, prop, receiver)
-        trapSet(hasPrevValue, prevValue, prop, value, (nextValue) => {
-          Reflect.set(target, prop, nextValue, receiver)
-        })
-        return true
-      },
-      defineProperty(
-        target: T,
-        prop: string | symbol,
-        desc: PropertyDescriptor
-      ) {
-        if (shouldTrapDefineProperty(desc)) {
-          const prevDesc = Reflect.getOwnPropertyDescriptor(target, prop)
-          if (!prevDesc || shouldTrapDefineProperty(prevDesc)) {
-            trapSet(
-              !!prevDesc && 'value' in prevDesc,
-              prevDesc?.value,
-              prop,
-              desc.value,
-              (nextValue) => {
-                Reflect.defineProperty(target, prop, {
-                  ...desc,
-                  value: nextValue,
-                })
-              }
-            )
-            return true
+        if (
+          hasPrevValue &&
+          (objectIs(prevValue, value) ||
+            (proxyCache.has(value) &&
+              objectIs(prevValue, proxyCache.get(value))))
+        ) {
+          return true
+        }
+        removePropListener(prop)
+        if (isObject(value)) {
+          value = getUntracked(value) || value
+        }
+        let nextValue = value
+        if (value instanceof Promise) {
+          value
+            .then((v) => {
+              value.status = 'fulfilled'
+              value.value = v
+              notifyUpdate(['resolve', [prop], v])
+            })
+            .catch((e) => {
+              value.status = 'rejected'
+              value.reason = e
+              notifyUpdate(['reject', [prop], e])
+            })
+        } else {
+          if (!proxyStateMap.has(value) && canProxy(value)) {
+            nextValue = proxyFunction(value)
+          }
+          const childProxyState =
+            !refSet.has(nextValue) && proxyStateMap.get(nextValue)
+          if (childProxyState) {
+            addPropListener(prop, childProxyState)
           }
         }
-        return Reflect.defineProperty(target, prop, desc)
+        Reflect.set(target, prop, nextValue, receiver)
+        notifyUpdate(['set', [prop], value, prevValue])
+        return true
       },
     }
     const proxyObject = newProxy(baseObject, handler)
@@ -371,7 +333,6 @@ const buildProxyFunction = (
     objectIs,
     newProxy,
     canProxy,
-    shouldTrapDefineProperty,
     defaultHandlePromise,
     snapCache,
     createSnapshot,

diff --git a/tests/class.test.tsx b/tests/class.test.tsx
@@ -355,31 +355,3 @@ it('no extra re-renders with getters', async () => {
     getByText('sum: 2 (2)')
   })
 })
-
-it('support class fields (defineProperty semantics)', async () => {
-  class Base {
-    constructor() {
-      return proxy(this)
-    }
-  }
-  class CountClass extends Base {
-    counter = { count: 0 }
-  }
-  const obj = new CountClass()
-
-  const Counter = () => {
-    const snap = useSnapshot(obj)
-    return <div>count: {snap.counter.count}</div>
-  }
-
-  const { findByText } = render(
-    <StrictMode>
-      <Counter />
-    </StrictMode>
-  )
-
-  await findByText('count: 0')
-
-  obj.counter.count = 1
-  await findByText('count: 1')
-})