-
Notifications
You must be signed in to change notification settings - Fork 364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable directly using custom editable elements through the editable object #240
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@donmccurdy could you please take a look at this and see if it solves your problem re: custom editables without having to mess with the schema? |
@AndrewPrifer thanks! Nothing here I disagree with, but it's possibly more API surface than is necessary for our use case if you would rather pare that back. We need to support custom three.js objects, but not necessarily custom R3F elements. Using Ideally something like this should work — regardless of what function UserlandObject ({name, userlandObject}) {
return <e.primitive uniqueName={name} object={userlandObject} editableType={'mesh'} />;
} Another issue I'm wondering about ... the snapshot window clones the scene, does that have implications for these userland/custom objects? Do we just need to guarantee the objects implement /cc @juniorxsound does this all sound correct? |
@donmccurdy there are indeed a couple easy ways already to accommodate custom threejs objects. The rationale for this PR was that from our talks it seemed like the requirement was not to just make this possible, but specifically to make it seamless. @juniorxsound @donmccurdy can you think of something that could better accommodate your use-case in this sense? I do like this implementation because while it does slightly extend the API surface, it is fully backwards-compatible, would be opt-in, and for users who are fully committed to r3f and would use Unless you think there could be other solutions here with better ergonomics (which I'm specifically targeting as my goal), I'd be inclined to merge this. As for coining the scene, yes, the only requirement should be that you implement |
I'll defer to @juniorxsound on the remaining questions of API surface. As background, our use cases involve writing a limited amount of React / R3F code up front for tooling that is reused by many more end-users who will create three.js objects without interacting with the React codebase at all. So IMHO we can afford to declare the theatre/studio sheet object properties exhaustively ourselves, seamlessness there is not critical. Whereas doing something reasonable with current and future 'new' three.js objects (even if it's just exposing pos/rot/scale) without breaking is crucial.
I have to admit I'd prefer that theatre/r3f append a managed |
Hi 👋 sorry for jumping into the discussion late. Echoing @donmccurdy I think smaller API surface might be a beneficial thing overall (and for what we need, the
This is a little bit of a tangent to what's proposed above but might be worth considering? |
@juniorxsound all of what you mention is going to be enabled by #166. This PR here is only for convenience. From our discussions I gathered that the idea was to enable seamless use of custom objects without any upfront configuration, hence this PR, but since you don't seem to need it right now, I'm going to sit on it a bit more. :) @donmccurdy one of the first things on my plate is going to be a real-time architecture, I completely agree that it would bring massive value. |
Solution 3 for the need to create ad-hoc editable r3f components outside what's defined in the schema.
This solution allows you to directly use r3f elements not defined in the schema. Also works with TypeScript.
Example:
This solution works by checking if the referenced property is defined in the schema. If not, TypeScript will warn you to add the
editableType
prop, which must be a valid editable type defined in the schema. Theatre will treat your custom element as if it were the provided type.What is the
ThreeElements
interface? In order to be able to provide auto-complete for valid r3f elements, it is necessary to have a catalogue of just the r3f elements. Currently, r3f doesn't provide this catalogue, instead extendingJSX.IntrinsicElements
directly. This interface represents what r3f should itself expose in the future to accommodate such use cases.Blockers/caveats for this PR
While this solution seems to work without issues both in terms of functionality and in terms of DX, there are a couple things I'd prefer to wait for before merging it:
ThreeElements
interface should be something exposed by r3f. Manually replicating this interface couples us too tight to specific versions, and also introduces quite a maintenance burden.editableType
, while for some you do. How this opt in should happen in the future is through the editable schema (the thing that defines all editable types and how theatre should handle them, which I keep referring to in this PR). Since it is not clear yet what API exactly this schema should provide when we expose it to users, we might need to look for an alternative way to configure this for now.