Jelajahi Sumber

fix(patchProp): harden props inference (#1006)

* fix(patchProp): harden props inference

* chore: remove as any casting

---------

Co-authored-by: Alvaro Saburido <alvaro.saburido@gmail.com>
Cody Bennett 1 hari lalu
induk
melakukan
6cdf28b73a
3 mengubah file dengan 78 tambahan dan 15 penghapusan
  1. 1 1
      src/core/nodeOps.test.ts
  2. 48 13
      src/core/nodeOps.ts
  3. 29 1
      src/utils/is.ts

+ 1 - 1
src/core/nodeOps.test.ts

@@ -1343,7 +1343,7 @@ describe('nodeOps', () => {
         const s = v => JSON.stringify(v)
         const camera = nodeOps.createElement('TresPerspectiveCamera', undefined, undefined, {})
         const result = []
-        camera.position.set = (x, y, z) => result.push({ x, y, z })
+        camera.position.fromArray = ([x, y, z]: THREE.Vector3Tuple) => result.push({ x, y, z })
         nodeOps.patchProp(camera, 'position', undefined, [0, 0, 0])
         nodeOps.patchProp(camera, 'position', undefined, [1, 2, 3])
         nodeOps.patchProp(camera, 'position', undefined, [4, 5, 6])

+ 48 - 13
src/core/nodeOps.ts

@@ -242,7 +242,7 @@ export const nodeOps: (context: TresContext) => RendererOptions<TresObject, Tres
   function patchProp(node: TresObject, prop: string, prevValue: any, nextValue: any) {
     if (!node) { return }
 
-    let root = node
+    let root: Record<string, unknown> = node
     let key = prop
 
     // NOTE: Update memoizedProps with the new value
@@ -277,7 +277,7 @@ export const nodeOps: (context: TresContext) => RendererOptions<TresObject, Tres
       node.__tres.eventCount += 1
     }
     let finalKey = kebabToCamel(key)
-    let target = root?.[finalKey]
+    let target = root?.[finalKey] as Record<string, unknown>
 
     if (key === 'args') {
       const prevNode = node as TresObject3D
@@ -300,7 +300,7 @@ export const nodeOps: (context: TresContext) => RendererOptions<TresObject, Tres
 
     if (root.type === 'BufferGeometry') {
       if (key === 'args') { return }
-      root.setAttribute(
+      (root as TresObject).setAttribute(
         kebabToCamel(key),
         new BufferAttribute(...(nextValue as ConstructorParameters<typeof BufferAttribute>)),
       )
@@ -312,11 +312,12 @@ export const nodeOps: (context: TresContext) => RendererOptions<TresObject, Tres
       // TODO: A standalone function called `resolve` is
       // available in /src/utils/index.ts. It's covered by tests.
       // Refactor below to DRY.
-      const chain = key.split('-')
-      target = chain.reduce((acc, key) => acc[kebabToCamel(key)], root)
-      key = chain.pop() as string
-      finalKey = key
-      if (!target?.set) { root = chain.reduce((acc, key) => acc[kebabToCamel(key)], root) }
+      target = root
+      for (const part of key.split('-')) {
+        finalKey = key = kebabToCamel(part)
+        root = target
+        target = target?.[key] as Record<string, unknown>
+      }
     }
     let value = nextValue
     if (value === '') { value = true }
@@ -335,11 +336,45 @@ export const nodeOps: (context: TresContext) => RendererOptions<TresObject, Tres
       }
       return
     }
-    if (!target?.set && !is.fun(target)) { root[finalKey] = value }
-    else if (target.constructor === value.constructor && target?.copy) { target?.copy(value) }
-    else if (is.arr(value)) { target.set(...value) }
-    else if (!target.isColor && target.setScalar) { target.setScalar(value) }
-    else { target.set(value) }
+
+    // Layers must be written to the mask property
+    if (is.layers(target) && is.layers(value)) {
+      target.mask = value.mask
+    }
+    // Set colors if valid color representation for automatic conversion (copy)
+    else if (is.color(target) && is.colorRepresentation(value)) {
+      target.set(value)
+    }
+    // Copy if properties match signatures and implement math interface (likely read-only)
+    else if (
+      is.copyable(target) && is.classInstance(value) && target.constructor === value.constructor
+    ) {
+      target.copy(value)
+    }
+    // Set array types
+    else if (is.vectorLike(target) && Array.isArray(value)) {
+      if ('fromArray' in target && typeof target.fromArray === 'function') {
+        target.fromArray(value)
+      }
+      else {
+        target.set(...value)
+      }
+    }
+    // Set literal types
+    else if (is.vectorLike(target) && typeof value === 'number') {
+      // Allow setting array scalars
+      if ('setScalar' in target && typeof target.setScalar === 'function') {
+        target.setScalar(value)
+      }
+      // Otherwise just set single value
+      else {
+        target.set(value)
+      }
+    }
+    // Else, just overwrite the value
+    else {
+      root[key] = value
+    }
 
     invalidateInstance(node as TresObject)
   }

+ 29 - 1
src/utils/is.ts

@@ -1,5 +1,6 @@
 import type { TresObject, TresPrimitive } from 'src/types'
-import type { BufferGeometry, Camera, Fog, Light, Material, Object3D, Scene } from 'three'
+import type { BufferGeometry, Camera, Color, ColorRepresentation, Fog, Light, Material, Object3D, Scene } from 'three'
+import { Layers } from 'three'
 
 export function und(u: unknown): u is undefined {
   return typeof u === 'undefined'
@@ -33,6 +34,33 @@ export function object3D(u: unknown): u is Object3D {
   return obj(u) && !!(u.isObject3D)
 }
 
+export function color(u: unknown): u is Color {
+  return obj(u) && !!(u.isColor)
+}
+
+export function colorRepresentation(u: unknown): u is ColorRepresentation {
+  return u != null && (typeof u === 'string' || typeof u === 'number' || color(u))
+}
+
+interface VectorLike { set: (...args: any[]) => void, constructor?: (...args: any[]) => any }
+export function vectorLike(u: unknown): u is VectorLike {
+  return u !== null && typeof u === 'object' && 'set' in u && typeof u.set === 'function'
+}
+
+interface Copyable { copy: (...args: any[]) => void, constructor?: (...args: any[]) => any }
+export function copyable(u: unknown): u is Copyable {
+  return vectorLike(u) && 'copy' in u && typeof u.copy === 'function'
+}
+
+interface ClassInstance { constructor?: (...args: any[]) => any }
+export function classInstance(object: unknown): object is ClassInstance {
+  return !!(object as any)?.constructor
+}
+
+export function layers(u: unknown): u is Layers {
+  return u instanceof Layers // three does not implement .isLayers
+}
+
 export function camera(u: unknown): u is Camera {
   return obj(u) && !!(u.isCamera)
 }