Skip to content

Commit 5f7e945

Browse files
authored
Merge pull request #3576 from tbvjaos510/fix/unknown-props
fix: improve resolution of dashed prop names
2 parents 500e051 + 7c6680f commit 5f7e945

File tree

2 files changed

+96
-8
lines changed

2 files changed

+96
-8
lines changed

packages/fiber/src/core/utils.tsx

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -255,19 +255,32 @@ export function prepare<T = any>(target: T, root: RootStore, type: string, props
255255
}
256256

257257
export function resolve(root: any, key: string): { root: any; key: string; target: any } {
258-
let target: unknown = root[key]
259-
if (!key.includes('-')) return { root, key, target }
258+
if (!key.includes('-')) return { root, key, target: root[key] }
260259

261-
// Resolve pierced target
262-
target = root
263-
for (const part of key.split('-')) {
260+
// First try the entire key as a single property (e.g., 'foo-bar')
261+
if (key in root) {
262+
return { root, key, target: root[key] }
263+
}
264+
265+
// Try piercing (e.g., 'material-color' -> material.color)
266+
let target = root
267+
const parts = key.split('-')
268+
269+
for (const part of parts) {
270+
if (typeof target !== 'object' || target === null) {
271+
if (target !== undefined) {
272+
// Property exists but has unexpected shape
273+
const remaining = parts.slice(parts.indexOf(part)).join('-')
274+
return { root: target, key: remaining, target: undefined }
275+
}
276+
// Property doesn't exist - fallback to original key
277+
return { root, key, target: undefined }
278+
}
264279
key = part
265280
root = target
266-
target = (target as any)?.[key]
281+
target = target[key]
267282
}
268283

269-
// TODO: change key to 'foo-bar' if target is undefined?
270-
271284
return { root, key, target }
272285
}
273286

@@ -411,6 +424,11 @@ export function applyProps<T = any>(object: Instance<T>['object'], props: Instan
411424

412425
let { root, key, target } = resolve(object, prop)
413426

427+
// Throw an error if we attempted to set a pierced prop to a non-object
428+
if (target === undefined && (typeof root !== 'object' || root === null)) {
429+
throw Error(`R3F: Cannot set "${prop}". Ensure it is an object before setting "${key}".`)
430+
}
431+
414432
// Layers must be written to the mask property
415433
if (target instanceof THREE.Layers && value instanceof THREE.Layers) {
416434
target.mask = value.mask

packages/fiber/tests/utils.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,63 @@ describe('resolve', () => {
205205
expect(key).toBe('bar')
206206
expect(target).toBe(root[key])
207207
})
208+
209+
it('should prioritize direct property over piercing', () => {
210+
const object = {
211+
'foo-bar': 'direct',
212+
foo: { bar: 'pierced' },
213+
}
214+
const { root, key, target } = resolve(object, 'foo-bar')
215+
216+
expect(root).toBe(object)
217+
expect(key).toBe('foo-bar')
218+
expect(target).toBe('direct')
219+
})
220+
221+
it('should handle undefined direct property values', () => {
222+
const object = { 'foo-bar': undefined }
223+
const { root, key, target } = resolve(object, 'foo-bar')
224+
225+
expect(root).toBe(object)
226+
expect(key).toBe('foo-bar')
227+
expect(target).toBe(undefined)
228+
})
229+
230+
it('should handle null direct property values', () => {
231+
const object = { 'foo-bar': null }
232+
const { root, key, target } = resolve(object, 'foo-bar')
233+
234+
expect(root).toBe(object)
235+
expect(key).toBe('foo-bar')
236+
expect(target).toBe(null)
237+
})
238+
239+
it('should return non-object as root when piercing fails due to non-object intermediate', () => {
240+
const object = { foo: 'not-an-object' }
241+
const { root, key, target } = resolve(object, 'foo-bar')
242+
243+
expect(root).toBe('not-an-object')
244+
expect(key).toBe('bar')
245+
expect(target).toBe(undefined)
246+
})
247+
248+
it('should return null as root when piercing fails due to null intermediate', () => {
249+
const object = { foo: null }
250+
const { root, key, target } = resolve(object, 'foo-bar')
251+
252+
expect(root).toBe(null)
253+
expect(key).toBe('bar')
254+
expect(target).toBe(undefined)
255+
})
256+
257+
it('should handle non-dashed keys normally', () => {
258+
const object = { foo: 'value' }
259+
const { root, key, target } = resolve(object, 'foo')
260+
261+
expect(root).toBe(object)
262+
expect(key).toBe('foo')
263+
expect(target).toBe('value')
264+
})
208265
})
209266

210267
describe('attach / detach', () => {
@@ -311,6 +368,19 @@ describe('applyProps', () => {
311368
expect(() => applyProps(target, {})).not.toThrow()
312369
})
313370

371+
it('should not throw when applying unknown props', () => {
372+
const target = new THREE.Object3D()
373+
applyProps(target, {})
374+
expect(() => applyProps(target, { ['foo-bar']: 1 })).not.toThrow()
375+
expect((target as any)['foo-bar']).toBe(undefined)
376+
})
377+
378+
it('should throw when applying unknown props due to non-object intermediate', () => {
379+
const target = new THREE.Object3D()
380+
applyProps(target, { foo: 1 })
381+
expect(() => applyProps(target, { ['foo-bar']: 1 })).toThrow()
382+
})
383+
314384
it('should filter reserved props without accessing them', () => {
315385
const get = jest.fn()
316386
const set = jest.fn()

0 commit comments

Comments
 (0)