Fix recursive form generation typescript perf

[scamden]

Reverted the revert. Got it working again and figured out the few reasons for the typescript perf problems.

Closes https://github.com/iway1/react-ts-form/issues/64 (again)

1. RenderedFieldMap also needs the recursion level limit (this was the big one)
2. All the types used by exported types need to be exported as well or the compiler tries to inline them resulting in The inferred type of this node exceeds the maximum length the compiler will serialize. An explicit type annotation is needed.
3. Don't force TS to infer the return type of createTSform. make it explicit since we can.

I tested editor perf locally and I found that depth 3 is the farthest TS will go without becoming extremely slow so I made that the maximum possible in the Prev tuple. 3 is also quite a bit slower than 2. I can feel a very slight difference between 2 and 1 but both are reasonable. I left it at depth 1 because this still allows arrays of objects, and one level of object nesting which covers our main use case, but we could probably bump it to 2 safely if we end up wanting it.

Hopefully this solves it this time! Let me know what you think! @iway1

[vercel[bot]]

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	Mar 31, 2023 11:34pm

[iway1]

Niceee work fixing the TS stuff. That's awesome.

One as the default sounds good I think, big fan of maintaining good TS performance and if someone needs more depth they can set it.

My one worry with this is that since it changes the behavior of how arrays are rendered it'd be a breaking change, would need to bump to v2.0.0 to do it this way...

Any thoughts about making the changes to how arrays are rendered opt-in? For some use cases I think it's actually preferable to have the previous behavior, for things like dynamically sized array values. For example like:

function DynamicArray() {
  const {field: {value, onChange}} = useTsController<string[]>();

  return (
    <div>
      <button onClick={()=>{
          onChange(
            value.concat([''])
          )
        }}
      >
        Add one element to array
      </button>
      {value.map((val, i)=>{
        return <input value={val} onChange={()=>onChange(/*update the string in array*/)}/>
      })}
    </div>
  )
}

Something like this could add a new field on press of the button, allowing rendering of dynamically sized arrays into their own components... I don't think this type of thing is possible when rendering with the new approach is it?

opt-in could be global:

const Form = createTsForm(mapping, {arrayBehavior: 'recursive' | 'default'})

Or could make it more granular

I can take a crack at making it opt-in, let me know what you think though

[scamden]

it changes the behavior of how arrays are rendered

i don't think it does? the code will use any mapping it finds before recursing so if you mapped .z.string().array() to Dynamic array it should work fine. I can write a test to ensure this.

Actually, I've been thinking more about arrays in general and I'm not sure we have a great story as is with or without this change. Right now I'd have to create a separate mapped component for every unique z.object().array() schema i have right? I'd love to be able to create a mapping from z.array() to a generic array component that this code could use when it finds an array type without a mapping and somehow it would be passed the child component list or something. That component could do drag and drop, dynamic addition, etc depending on props. My thinking isn't formed on this last bit but I'm wondering if you already had ideas on arrays.

[scamden]

@iway1 just rebased and added a test. you can see you can actually use an explicitly mapped array right along side an unmapped one.

I think the only ~breaking change here, is that now rather than throwing an error we will render UI for arrays and objects that are unmapped. I don't think that's really breaking in that users couldn't have had functioning code that would hit this code path since it would have thrown No matching zod schema for type before.

[scamden]

All that said, I'm happy to make it opt in (or out) as well but I don't see it as necessary, and actually feel like this is good default behavior. (Also it will complicate the code and types, which doesn't feel worth it)

[iway1]

@scamden ah okay I think I misunderstood the behavior, that makes sense.

So it's going to map the array schemas to matching components as it did previously, but in cases where it there isn't a matching schema it will map an array of components.

Thanks for explaining, and for all the work on this PR 😃

[scamden]

@iway1 thanks for merging! I'm eager to figure out the array story a bit more as well!

Btw invite me to discord if you get a sec! Username: sterling#8220