CLOSED: RFC useForm defaultValuesStrategy: shallow & deep

[bluebill1049]

Context

React Hook Form embraces uncontrolled form and focuses on performance and user experience. This is one of the core design principles which we have followed. However, there are cases where developer experience gets sacrificed due to defaultValues is not the local state and often users expect defaultValues to be part of the form values, and hence we have introduced shallow merge to encounter that without sacrifice performance.

defaultValues will be shallowly merged with form submission data.

This approach solves most of the use cases but leaves deeply nested defaultValues get overwrite due to shallow merge.

For example the following use case:

const App = () => {
  const { register, handleSubmit } = useForm({
    defaultValues: {
      test: { firstName: 'bill', lastName: 'bill' }
    }
  })

  const onSubmit = (data) => {
     console.log(data) // { test: { firstName: 'bill' } } while last name get lost due to shallow merge
  }
  
  return (
    <form onSubmit={handleSubmit(onSubmit)}>
      <input {...register("test.firstName")} />
      <button />
    </form>
  )
}

Proposal

We are considering exposing a new config at useForm which allows users to choose the merge strategy.

Here is the proposed syntax

const App = () => {
  const { register, handleSubmit } = useForm({
    defaultValuesStrategy: 'deep', // 'shallow` by default and `deep` as optional
    defaultValues: {
      test: { firstName: 'bill', lastName: 'bill' }
    }
  })

  const onSubmit = (data) => {
     // deep merge with defaultValues solves the value lost
     console.log(data) // { test: { firstName: 'bill', lastName: 'luo' } } 
  }
  
  return (
    <form onSubmit={handleSubmit(onSubmit)}>
      <input {...register("test.firstName")} />
      <button />
    </form>
  )
}

Considerations

• this will not be a break changing.
• this feature will only be applied to shouldUnregister: false which is by default.

API coverage

• handleSubmit
• getValues
• watch / useWatch

Props

• flexible with defaultValues behavior
• confident on defaultValues without getting overwrite

Cons

• select deep will have a relative performance impact for large and complex form

Limitation

• Binary file will not get deep merged (input file).
• Class Object with methods

Related issues/feature requests

#5003
#5056

Draft PR

#5069

[bluebill1049]

cc @kotarella1110 @jorisre @leapful (team)

[mogelbrod]

Nice! If this is a feature that others would like I totally see it as a great addition to the library 👍

For more complex form models this may incur a noticeable perf cost - so it would be interesting to do some benchmarking.
I'm curious as to how often the deep merging happens:

1. How many times per useForm() render?
2. How many times per field update (onChange/setValue)?
3. How many times per (controlled/uncontrolled) field render?

Perhaps using a "lazy" deep merge/clone is possible? Eg. a deep clone that only shallowly copies the objects that are part of the path to the value being edited? They should be cheaper than a full deep merge. See this SO post for context.

[bluebill1049]

How many times per useForm() render?

Doesn't happen on userForm level.

How many times per field update (onChange/setValue)?

onChange with schema validation (schema needs to know the form values) and none for the build-in validator.

How many times per (controlled/uncontrolled) field render?

same as above.

This operation consumes the most during watch action, especially when you watch() all.

Perhaps using a "lazy" deep merge/clone is possible? Eg. a deep clone that only shallowly copies the objects that are part of the path to the value being edited? They should be cheaper than a full deep merge. See this SO post for context.

Thank you for all the awesome feedback. I was thinking the same for partial watch. I guess the most expensive part is when the user decided to watch everything on the form, which results in deepMerge on each change and every field.

[wdfinch]

I have one comment. We introduced shouldUnregister which had the following note:

By setting shouldUnregister to true at useForm level, defaultValues will not be merged against submission result.

If we have the option above, do we need this feature anymore? It seems like shouldUnregister should just focus on controlling whether the form input will unregister on unmount rather than add this extra gotcha. I get that it was added to achieve the V6 behavior, but it seems redundant now. We could add 'none' as an option above.

Additionally what would happen if the user adds shouldUnregister: false and defaultValuesStrategy: 'none'?

I think the cleanest solution is to remove the shallow merging modification from shouldUnregister and just have defaultValuesStrategy control this behavior.

[bluebill1049]

this feature will only be applied to shouldUnregister: false which is by default.

I do have some concern over large and complex app when we set deep merge as default.

[wdfinch]

I see. Thanks.

[bluebill1049]

we may no longer this config.

[wdfinch]

If it works by registering inputs then I would expect not because field array inputs need to be mounted in order to be registered.

Hopefully it could work as this is main reason I would want to use the feature. I want to only render a part of a field array for performance reasons.

We would need the ability to register field array inputs without mounting them which is probably another feature request and change.

[bluebill1049]

I believe this new feature should cover the above usage. Can u try out the beta version?

[wdfinch]

Works for me in 7.4.3-beta.4. Nice work!

Does it seem like it's not working on 7.5 onward at the moment? Is this yet to be merged in? I'm a little confused since it seems we've already moved on in versioning.

working sample
https://codesandbox.io/s/spring-cdn-ywdmb?file=/src/App.js

[bluebill1049]

not merged.

[wdfinch]

Gotcha. No problem. Thanks!

[bluebill1049]

[email protected] :)

[bluebill1049]

[email protected]

[andrwo]

I just like to say a very big thank you, for the latest update 7.6.0 that has auto register of values (simulating auto deep merge)!

I had tried to raise this issue early on the big V7 discussion thread, but was not making any headway, and I am glad now there is enough community support to get this implemented. Due to our very dynamic & very nested forms, it was very difficult for us to register every nested value we want to submit in the form.

To be honest, we had already started migrating away from RHF to React-Final-Form due to this (it was that big a dealbreaker to us), but thankfully I wrote this as a separate own API layer that allowed me to switch back from RFF to RHF, and things started working beautifully with the latest update. We would very much have preferred to remain in RHF and this allowed us to do so.

Thanks once again for all the great and speedy work put in, we really appreciate it. 👍👍

[wdfinch]

Does this get disabled if we set shouldUnregister to false?