Open scamden opened 11 months ago
The latest updates on your projects. Learn more about Vercel for Git ↗︎
Name | Status | Preview | Comments | Updated (UTC) |
---|---|---|---|---|
react-ts-form | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Aug 2, 2023 10:57pm |
@scamden Cool! Checking it out.
Just curious what the main motivation for this is, was there a concrete use case you're wanting it for or just a general idea for improvement?
I have a bunch of concrete use cases! On phone but I'll write out more later! I can add tests that show case them
@iway1 alright added a few more test cases! This allows us to write array components without duplicating any of the Fields. It also allows us to handle recursive schemas (with a lot of frustrating Zod type writing but these should be a power user feature and at least this proves it possible).
This also provides IMO a better way to handle nested schemas than the automatic recursive thing I added previously. That solution is a little bit less work to use if you don't have any customizations but it's performance impact on TS is still significant. And also in real life I find that you end up needing to customize at different levels of nesting pretty often. We could leave it, but I'd love to discuss defaulting the recursion depth to only a single level or maybe removing the feature entirely and publishing our first breaking change.
One question I still have that I'd love your feedback on:
I needed to allow you to pass "name" for array based components so they could insert the necessary [index]
into the field path. I'll comment inline where i did this, but i wasn't sure whether it's better to force the user to prefix the name themselves or automatically supply the prefix from context and let the user assume they are only providing the name for their "level" of the data (eg just the [1]
instead of previousKeys[1]
). I chose the former, but maybe we could make name
something more clear? something like nameAtThisLevel
? idk naming is hard..
@iway1 ok iterated quite a bit and found a nice API for this i think! can you TAL now?
Maybe we can add a section to the docs under complex-fields as well if we like this. (Can do as a follow on as I'm hoping to make use of this feature for a project this week)
@iway1 i'm feeling pretty solid about this contract and it's a non breaking change but I don't really want to merge without your go ahead. i could merge in our fork and test it out that way for a bit, or if you're feeling comfortable I can merge here myself. lemme know what you're thinking.
i'll probably need to merge one of the two ways by tomorrow so we can make use of this for a project.
@iway1 One more ping on this. I may go ahead and merge to keep us in sync. This is working well for us.
One add on I'll probably do eventually is a wrapper around useFieldArray similar to the usTsController as we end up using that hook a lot with these.
@iway1 you kinda done with this lib? Totally ok if so just want to set my expectations
@iway1 I think this approach might be more versatile (and better performance) than the recursive component walking I added. This lets you render whatever remaining piece of the form your component is responsible for reducing duplication but giving full control.
This is close but still WIP needs at least:
name
prop to be an intermediate name, and should it be renamed?I think these are not necessary to merge
type
but isn't strongly typed). i'm not sure this is important since we will have to cast it's type regardless. It does bring up a point though. That these custom components don't "know' their own schema. They only are coupled to the schema in the mapping. That's kinda fine but it would be cool to think of a way to more strongly couple themThoughts?