Feedback (AppForm; unmount; proxy; context)

[Sam-Kruglov]

Amazing library, thank you!

I do see some boilderplate that from the first glance seems to be like it could be removed somehow:

export default function CreateItem() {
  const form = useAppForm({
    defaultValues: {
      name: '',
      type: ItemType.TypeA,
      special: {
        value: 0
      }
    } satisfies ItemCreateInput,
    onSubmit: ({ value }) => {
      // send request, handle error
    }
  })
  const type = useStore(form.store, state => state.values.type)
  useEffect(() => {
    if (!isSpecial(type)) form.resetField('special')
  }, [form, type])
  return (
    <form
      onSubmit={e => {
        e.preventDefault()
        void form.handleSubmit()
      }}
    >
      <form.AppForm>
        <form.AppField name="name" children={field => <field.Input />} />
        <form.AppField name="type" children={field => <field.SelectInput options={Object.values(ItemType)} />} />
        {isSpecial(type) && <SpecialFieldGroup form={form} fields="special" />}
        <form.SubmitButton />
      </form.AppForm>
    </form>
  )
}

Here's how I'd like to see it:

export default function CreateItem() {
  const form = useAppForm({
    defaultValues: {
      name: '',
      type: ItemType.TypeA,
      special: {
        value: 0
      }
    } satisfies ItemCreateInput
  })
  // no useEffect here, automatically track if some fields unmount and delete them
  return (
    // looks more like a regular form that way
    <form.AppForm onSubmit={({ value }) => {
      // send request, handle error
    }}> 
      // since we're assigning our component to the "form", couldn't we also create a proxy to add "field" attribute? 
      // Or at least mandate that we accept the field param. Also reads nice
      <form.Input field="name" />
      <form.SelectInput field="type" options={Object.values(ItemType)} />
      // I don't like the inconsistency here. I think we should either somehow do "form.SpecialFieldGroup" 
      //     or call the above like that too e.g. <Input form={form} field="name"/>
      {isSpecial(type) && <SpecialFieldGroup form={form} fields="special" />}
      <form.SubmitButton className="mt-1" />
    </form.AppForm>
  )
}

Also, when I select a non-special type so that the special group is not rendered but then I select it again so that it is rendered again, I get this error, should I do something differently?

A component is changing an uncontrolled input to be controlled. This is likely caused by the value changing from undefined to a defined value, which should not happen. Decide between using a controlled or uncontrolled input element for the lifetime of the component. More info: https://react.dev/link/controlled-components

Side note with very low confidence: since we're wrapping the whole thing in form.AppForm, couldn't the children resolve the form instance automatically via useFormContext or something? I don't know if type inference will work though.

[LeCarbonator]

Hey! Thanks a lot for the feedback. I'll note them down for future fixes and features.

I can share some insight into why the decisions were made and why the API is the way it is now. They're in no particular order and don't serve as criticism of the feedback here.

no useEffect here, automatically track if some fields unmount and delete them

We don't currently have an onUnmount listener, but users have requested it for this reason you mentioned. It will likely not be automatically handled, since field-specific errors may get lost if they're bound to the render.

<form.AppForm> looks more like a regular form that way

AppForm is merely a context provider. Making it bound to an HTML element can prove to be problematic if your structure depends on the form's location (modals, cards etc.)
You can however get quite close to it using form components:

// Allows us to now use any form components within
return <form.AppForm>
  <form.BlockNavigationIfDirty /> {/* ranging from general logic features */}
  <form.Form> {/* to mimicking form elements (and populating its `onSubmit` handler automatically */}
  </form.Form>
</form.AppForm>

since we're assigning our component to the "form", couldn't we also create a proxy to add "field" attribute?
Or at least mandate that we accept the field param. Also reads nice

Yes, the current API will be more wordy. However, consider that the UI logic and form logic are separated by their concerns this way:

// Logic for validation, linked fields, name. Related to the form
<form.AppField
   name="name"
   validators={{}}
   listeners={{}}
>
  {field => {
    // Logic for UI, rendering.
    return (<field.Input label="Name" />)
    
    // If we rarely have different fields, we can go for large components:
    return (<field.Input label="Name" />)
    
    // Or if you prefer granular control, you can make small field components sharing one field
    return (
      <field.Item>
          <field.Label>Name</field.Label>
          <field.Control />
          <field.Description>Your name</field.Description>
          <field.Message />
      </field.Item>
     )
   }}
</form.AppField>

I don't like the inconsistency between field groups and withForm vs. field components here.

This discussion has been brought up before, and I frankly agree that it's inconsistent. We'll see if this changes in the future (or perhaps gets overhauled for a v2), but know you're not alone in thinking this.

One issue that results from this is that <form.Component /> doesn't actually share information between Component and the form. Visually, it appears to do so, but Component currently does not know what form called it. That's why withForm and withFieldGroup rely on using a parameter instead, so that type safety can be guaranteed.

Keep in mind that this is related to <form.Component/> vs. <Component form={form} />. As far as field vs. fields go, that's because the properties don't behave the same. You can't use remapping for normal fields, while you can for field groups.

<MyFieldGroup form={form} fields={{
     foo: 'actual.location',
     bar: 'differently.named.in.this.form'
}} 
/>

// So if remapping is allowed, then what is this?
<MyFieldGroup form={form} fields="location" />

// Simple, the above string is a shortcut for the following object:
fields={{
   foo: 'location.foo',
   bar: 'location.bar'
}} 

I hope this can help clear up some questions you may have had. I'll also consider the feedback here for #1606 , since I wasn't sure how to approach this properly.