iway1 / react-ts-form

https://react-ts-form.com
MIT License
2.01k stars 35 forks source link

Recursive generation for object types #64

Closed scamden closed 1 year ago

scamden commented 1 year ago

Please tell me if I've missed this but. I think it would be really great to be able to represent a nested schema and have zod automatically walk the objects and render any primitive types if i don't have an z.object() mapping setup.

z.object({
  nested : z.object({
    numField : z.number()
  })
})

would just render whatever is mapped to the number editor and supply default values from the object path and recreate the object on edit automatically so values would be {nested : {numField : 9}}

P.S. We are considering using this very heavily and i'd be happy to contribute if you're open to some of these ideas. (I wrote a custom form gen library at my last job, so I think i have some relevant background that could be useful)

iway1 commented 1 year ago

yeah this might be pretty cool, not sure how complex the typescript would get but I don't think it'd be too terrible.

Wondering what is the advantage of this over the non nested version?

scamden commented 1 year ago

Just being able to edit complex / nested json types like say from a GQL api and directly send the formvalues back without having to reconstruct the object on the way out (and flatten the object on the way in)

iway1 commented 1 year ago

ah okay so just allowing skipping the step of having to remap the form data object to something else, that'd be really nice.

I think to implement this we'd need to update the name that we pass to the react-hook-form use controller to take into account the nesting and it should just work for the validation part of it. So would check if a field is nested and recursively create the name like:

const FormSchema = z.object({
  nested: z.object({
    text: z.string()
  })
)

Then in the library code would need to make sure that this gets passed to useController

const controller = useController({name: 'nested.text'})

Luckily RHF supports that dot syntax which seems nice here.

Also - would have to check for collisions I think:

z.object({
  text: z.string(),
  nested: z.object({
    text: z.string() // not allowed because it collides with the upper level "text" property
  })
)

TBH sounds pretty hard to implement but would be a really nice quality of life feature

scamden commented 1 year ago

ah okay so just allowing skipping the step of having to remap the form data object to something else

ya in particular i think arrays of objects also make this really crucial cause there's no way to manually flatten them.

are the collisions really a problem though?

thanks for the pointers on implementation path! i'm gonna take a stab at this today. let you know how it goes :)

iway1 commented 1 year ago

are the collisions really a problem though?

Not sure if it'd matter in practice really, but would just break in cases where there was a collision 🤷 not a big deal if we don't handle them but would be nice to throw an error if there's a convenient place to do it to make it a little more dev friendly

ya in particular i think arrays of objects also make this really crucial cause there's no way to manually flatten them.

Not sure what you mean by this, is this a graphql specific issue or something?

scamden commented 1 year ago

Not sure if it'd matter in practice really

i think what i meant is: isn't it ok to have both? like they will have different field names to hook form and technically there's nothing keeping a data structure from having the same prop name at multiple levels

Not sure what you mean by this, is this a graphql specific issue or something?

i meant if i have schema

const schema = z.object({
      arrayField: z.object({
        text: z.string(),
        age: z.number(),
      }).array()
    });

i have to build a custom editor in the mapping for this or else i would have to generate a new schema each time because flattening the array would require me to name each key in the schema with an index.

i have this basically working on my fork! the types part was a DOOZY! i'm mildly concerned about performance but let's discuss on the pr.

the main thing i came here to ask is what formatter you use? @iway1 would you be open to using prettier or something? i have a lot of changes and manually formatting them sounds hard and i'm not sure how to emulate your settings

iway1 commented 1 year ago

@scamden yeah prettier! IDK why i never set it up for this project I use it on all my other ones. Sorry about that. TBH Idk what's going on with the current formatting.

I just added prettier with a small config and ran it on the files, it only changed a few lines it looks like so shouldn't be any conflicts with your fork I don't think. probably will want to add some more rules to it and run it on everything once your fork is merged I think.

i have this basically working on my fork! the types part was a DOOZY!

NICE WORK figuring out those types I figured they'd be really tough 🔥

i think what i meant is: isn't it ok to have both? like they will have different field names to hook form and technically there's nothing keeping a data structure from having the same prop name at multiple levels

I think I misunderstood the API you were going for here then, I assumed that the props would get flattened like:

const Schema = z.object({
  text: z.string(),
  nested: z.object({
    nestedText
  })
})

<Form schema={Schema} props={{
  text: {},
  nestedText: {}
}}/>

Your implementation works just as well though and probably simpler, so yeah totally unnecessary to check for collisions.

i have to build a custom editor in the mapping for this or else i would have to generate a new schema each time because flattening the array would require me to name each key in the schema with an index.

I'm still confused about how you're thinking it'll work if changed

So right now the library will want a custom schema per array field IE:

const NameAgeArraySchema = z.object({
        text: z.string(),
        age: z.number(),
      }).array()

const mapping = [
  [NameAgeArraySchema, NameAndAgesArrayField]
] as const;

const FormSchema = z.object({
  nameAndAges: NameAgeArraySchema
})

what would be the difference once nested schemas are implemented?

scamden commented 1 year ago

@iway1 oh also what package manager are you using? there's no lock file so i've been doing pnpm i --shamefully-hoist && rm pnpm-lock.yaml lol

iway1 commented 1 year ago

@scamden pnpm now, I just added it to the repo