fix: improve resolution of dashed prop names

[tbvjaos510]

Fixes an issue with unknown props causing errors when applied to Three.js objects.

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 7c6680f:

Sandbox	Source
example	Configuration

[krispya]

What is an "unknown prop" in this case?

[CodyJasonBennett]

Duplicate with #3573? If you write foo-bar and foo does not exist as a property (piercing as foo.bar), R3F will not write it as 'foo-bar': true but crash. I left a todo since this was not good behavior.

[acdzh]

Duplicate with #3573? If you write foo-bar and foo does not exist as a property (piercing as foo.bar), R3F will not write it as 'foo-bar': true but crash. I left a todo since this was not good behavior.

Is there some better way to solve this?

[tbvjaos510]

Duplicate with #3573? If you write foo-bar and foo does not exist as a property (piercing as foo.bar), R3F will not write it as 'foo-bar': true but crash. I left a todo since this was not good behavior.

Yes, you're exactly right! This PR addresses the same issue as #3573. Both PRs are trying to fix the problem where foo-bar attributes crash when foo doesn't exist as a property, instead of gracefully handling them as 'foo-bar': true.

Feel free to close this PR if #3573 is the preferred approach, or let me know if there are any changes needed to align with the solution you have in mind.

[krispya]

This is an awkward part of the codebase and so will necessitate awkward solutions. It is legal for keys to have dashes in them such as data-key and so the assumption that all prop names in snake case should be read as a path does not always hold true. We have two separate issues here:

1. Not all props with dashes should be read as an object access path.
2. We need a better way to handle when a prop is an access path and the path terminates early.

Right now my thinking is we should have the following rules:

• data-* and aria-* are exempt from being read as access paths.
• There should be some kind of explicit escape for having R3F not read snake case as a path. For example:

<mesh my--prop="value" /> // Apply to key my-prop
<mesh material-color--space /> // Apply to material['color-space']

// Or if that is too confusing to read something like this
<mesh _my-prop_="value" /> // Apply to key my-prop

• We should throw if a path does not exist with an explicit error. No silent failures.

I believe silent failures will lead to more confusion than it solves. Having a prop fail to set is breaking a contract with React and our deep path setting is brittle enough as it is. I don't want to encourage making it even more brittle! Our contract here is that we will set values on nested objects but you must guarantee React that those paths will be valid and unchanged.

I am hesitant on adding an explicit escape just because it adds more rules and exceptions where we already have too many but I still think making those exceptions implicit will not be good.

[acdzh]

My personal opinion is that it's not advisable to solve this issue by defining specific rules. A better approach would be to simply ignore any key that isn't valid (ideally, throw a warning).

1. Custom rules can sometimes be quite odd. If every library has its own set of rules, the codebase will be filled with various dialects, which can be confusing for users.

2. We can guide users to write attributes using special rules, but in many cases, that's not what users actually need. In reality, users have no reason to add attributes to R3F components that won't have any effect—these attributes are often automatically inserted by Babel plugins or other build tools. Instead of telling users how to write their code, it's better to make the system robust enough to handle these attributes gracefully, rather than failing when encountering them.

[tbvjaos510]

I agree with @acdzh's perspective here. Given React JSX's nature where any prop can be injected unexpectedly, it seems counterintuitive for a library to define its own whitelist of acceptable props.

Currently, props without dashes are silently ignored when they don't match valid Three.js properties, and users aren't confused by this behavior. Having props with dashes behave consistently with this pattern would reduce user confusion and maintain a more predictable API surface.

Rather than introducing custom rules and escape mechanisms, handling unknown props gracefully (whether they contain dashes or not) would provide a more robust and user-friendly experience.

[CodyJasonBennett]

It is unfortunate that type safety suffers with prop piercing (material-color) since runtime validation now becomes an important part of the API. TypeScript does not let us infer these props. My intention with the todo was to fall back to setting string keys if piercing fails to resolve, but this is difficult to use without some kind of visibility like a warning or error for undefined props.

Perhaps there's a way we can support snake case props in a way where we keep validation? Consider the following routine:

1. set target['a-b'] if not undefined (null okay); otherwise, continue
2. set target['a']['b'] if target['a'] is not undefined, throw if defined but unexpected shape; otherwise, continue
3. throw, prop is undefined

[tbvjaos510]

@CodyJasonBennett Thanks for the detailed feedback! I've implemented the 3-step routine as suggested:

1. Direct property check: First try target['foo-bar'] if it exists as a literal key (including undefined/null values)
2. Piercing attempt: If direct property doesn't exist, try target.foo.bar access
3. Natural error handling: When intermediate path is defined but has unexpected shape, return that value as root so applyProps naturally throws TypeError

[krispya]

Thanks for the discussion everyone. It's great when we work together for the best solution.

I think having a 3-step routine is good. It should be easy to explain and follow. However, does this solve the issue of a bundler plugin inserting data-* props? If understand the routine correctly, it will fail to set since the property does not already exist on the object. In these cases I think we would still need to make exceptions for common props like data-* and aria-*. React DOM does similar for DOM elements so it shouldn't be that strange.

Also, I think we should emit our own error when the traversal fails. This will let us also downgrade it to a warning when we need to.

[acdzh]

The R3F components inside Canvas probably don’t actually render as real DOM elements? If that’s the case, it seems like data-* and aria-* attributes not working isn’t really a problem.

[krispya]

I wouldn’t have considered it either but this PR was created to address situations where a bundler adds it for testing with dev tools: #3573

I wonder if it still works even if the property is not actually written to the object?

[tbvjaos510]

I use locatorjs in my project, but the goal of this PR isn't to enable locatorjs functionality on Three.js elements.
The primary purpose is to resolve the runtime error crashes that occur when unknown attributes are injected into R3F components.

[krispya]

Okay thanks for the clarification. Just to be extra clear, none of your use cases require data-* props to literally get written to a Three object, yes? In which case silently failing would work just fine. @tbvjaos510 @acdzh

[tbvjaos510]

@krispya Correct! data-* props don't need to be written to Three.js objects. My use case with
locatorjs is purely for DOM-level tooling that injects these attributes for development purposes - they should just be ignored by R3F without causing crashes.

[krispya]

Okay I'll clean this PR up and merge it today unless someone objects.

[acdzh]

@krispya Hello, may I ask when the next release that includes this change will be published? The current version is still from July.

[CodyJasonBennett]

I now released v9.4.0 which contains this fix.

[krispya]

Thanks Cody. Sorry folks, I went on a trip and got distracted from this!