Skip to content

Commit 6cdf28b

Browse files
fix(patchProp): harden props inference (#1006)
* fix(patchProp): harden props inference * chore: remove as any casting --------- Co-authored-by: Alvaro Saburido <[email protected]>
1 parent 1aaa85a commit 6cdf28b

File tree

3 files changed

+78
-15
lines changed

3 files changed

+78
-15
lines changed

src/core/nodeOps.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1343,7 +1343,7 @@ describe('nodeOps', () => {
13431343
const s = v => JSON.stringify(v)
13441344
const camera = nodeOps.createElement('TresPerspectiveCamera', undefined, undefined, {})
13451345
const result = []
1346-
camera.position.set = (x, y, z) => result.push({ x, y, z })
1346+
camera.position.fromArray = ([x, y, z]: THREE.Vector3Tuple) => result.push({ x, y, z })
13471347
nodeOps.patchProp(camera, 'position', undefined, [0, 0, 0])
13481348
nodeOps.patchProp(camera, 'position', undefined, [1, 2, 3])
13491349
nodeOps.patchProp(camera, 'position', undefined, [4, 5, 6])

src/core/nodeOps.ts

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ export const nodeOps: (context: TresContext) => RendererOptions<TresObject, Tres
242242
function patchProp(node: TresObject, prop: string, prevValue: any, nextValue: any) {
243243
if (!node) { return }
244244

245-
let root = node
245+
let root: Record<string, unknown> = node
246246
let key = prop
247247

248248
// NOTE: Update memoizedProps with the new value
@@ -277,7 +277,7 @@ export const nodeOps: (context: TresContext) => RendererOptions<TresObject, Tres
277277
node.__tres.eventCount += 1
278278
}
279279
let finalKey = kebabToCamel(key)
280-
let target = root?.[finalKey]
280+
let target = root?.[finalKey] as Record<string, unknown>
281281

282282
if (key === 'args') {
283283
const prevNode = node as TresObject3D
@@ -300,7 +300,7 @@ export const nodeOps: (context: TresContext) => RendererOptions<TresObject, Tres
300300

301301
if (root.type === 'BufferGeometry') {
302302
if (key === 'args') { return }
303-
root.setAttribute(
303+
(root as TresObject).setAttribute(
304304
kebabToCamel(key),
305305
new BufferAttribute(...(nextValue as ConstructorParameters<typeof BufferAttribute>)),
306306
)
@@ -312,11 +312,12 @@ export const nodeOps: (context: TresContext) => RendererOptions<TresObject, Tres
312312
// TODO: A standalone function called `resolve` is
313313
// available in /src/utils/index.ts. It's covered by tests.
314314
// Refactor below to DRY.
315-
const chain = key.split('-')
316-
target = chain.reduce((acc, key) => acc[kebabToCamel(key)], root)
317-
key = chain.pop() as string
318-
finalKey = key
319-
if (!target?.set) { root = chain.reduce((acc, key) => acc[kebabToCamel(key)], root) }
315+
target = root
316+
for (const part of key.split('-')) {
317+
finalKey = key = kebabToCamel(part)
318+
root = target
319+
target = target?.[key] as Record<string, unknown>
320+
}
320321
}
321322
let value = nextValue
322323
if (value === '') { value = true }
@@ -335,11 +336,45 @@ export const nodeOps: (context: TresContext) => RendererOptions<TresObject, Tres
335336
}
336337
return
337338
}
338-
if (!target?.set && !is.fun(target)) { root[finalKey] = value }
339-
else if (target.constructor === value.constructor && target?.copy) { target?.copy(value) }
340-
else if (is.arr(value)) { target.set(...value) }
341-
else if (!target.isColor && target.setScalar) { target.setScalar(value) }
342-
else { target.set(value) }
339+
340+
// Layers must be written to the mask property
341+
if (is.layers(target) && is.layers(value)) {
342+
target.mask = value.mask
343+
}
344+
// Set colors if valid color representation for automatic conversion (copy)
345+
else if (is.color(target) && is.colorRepresentation(value)) {
346+
target.set(value)
347+
}
348+
// Copy if properties match signatures and implement math interface (likely read-only)
349+
else if (
350+
is.copyable(target) && is.classInstance(value) && target.constructor === value.constructor
351+
) {
352+
target.copy(value)
353+
}
354+
// Set array types
355+
else if (is.vectorLike(target) && Array.isArray(value)) {
356+
if ('fromArray' in target && typeof target.fromArray === 'function') {
357+
target.fromArray(value)
358+
}
359+
else {
360+
target.set(...value)
361+
}
362+
}
363+
// Set literal types
364+
else if (is.vectorLike(target) && typeof value === 'number') {
365+
// Allow setting array scalars
366+
if ('setScalar' in target && typeof target.setScalar === 'function') {
367+
target.setScalar(value)
368+
}
369+
// Otherwise just set single value
370+
else {
371+
target.set(value)
372+
}
373+
}
374+
// Else, just overwrite the value
375+
else {
376+
root[key] = value
377+
}
343378

344379
invalidateInstance(node as TresObject)
345380
}

src/utils/is.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { TresObject, TresPrimitive } from 'src/types'
2-
import type { BufferGeometry, Camera, Fog, Light, Material, Object3D, Scene } from 'three'
2+
import type { BufferGeometry, Camera, Color, ColorRepresentation, Fog, Light, Material, Object3D, Scene } from 'three'
3+
import { Layers } from 'three'
34

45
export function und(u: unknown): u is undefined {
56
return typeof u === 'undefined'
@@ -33,6 +34,33 @@ export function object3D(u: unknown): u is Object3D {
3334
return obj(u) && !!(u.isObject3D)
3435
}
3536

37+
export function color(u: unknown): u is Color {
38+
return obj(u) && !!(u.isColor)
39+
}
40+
41+
export function colorRepresentation(u: unknown): u is ColorRepresentation {
42+
return u != null && (typeof u === 'string' || typeof u === 'number' || color(u))
43+
}
44+
45+
interface VectorLike { set: (...args: any[]) => void, constructor?: (...args: any[]) => any }
46+
export function vectorLike(u: unknown): u is VectorLike {
47+
return u !== null && typeof u === 'object' && 'set' in u && typeof u.set === 'function'
48+
}
49+
50+
interface Copyable { copy: (...args: any[]) => void, constructor?: (...args: any[]) => any }
51+
export function copyable(u: unknown): u is Copyable {
52+
return vectorLike(u) && 'copy' in u && typeof u.copy === 'function'
53+
}
54+
55+
interface ClassInstance { constructor?: (...args: any[]) => any }
56+
export function classInstance(object: unknown): object is ClassInstance {
57+
return !!(object as any)?.constructor
58+
}
59+
60+
export function layers(u: unknown): u is Layers {
61+
return u instanceof Layers // three does not implement .isLayers
62+
}
63+
3664
export function camera(u: unknown): u is Camera {
3765
return obj(u) && !!(u.isCamera)
3866
}

0 commit comments

Comments
 (0)