final-form / react-final-form

🏁 High performance subscription-based form state management for React
https://final-form.org/react
MIT License
7.39k stars 481 forks source link

Why do we have to clone mutators? #704

Open fzaninotto opened 4 years ago

fzaninotto commented 4 years ago

What is the current behavior?

The documentation shows that external form mutators should be cloned before passing them to a Form component:

<Form
    onSubmit={onSubmit}
    mutators={{
        ...arrayMutators
    }}
    render={({

In the case of final-form-arrays, arrayMutators is a set of functions. Cloning this set has absolutely no effect. I checked that, if I remove the clone, it still works fine in the FieldArrays example.

Since the clone is useless in that case and it works without it, my question is: Why does the documentation emphasize the need for a clone?

Sandbox Link

https://codesandbox.io/s/kx8qv67nk5

Thanks for a great lib.

fzaninotto commented 4 years ago

Additional comment: If I remove the spread operator:

<Form
    onSubmit={onSubmit}
    mutators={arrayMutators}
    render={({

Then the TypeScript compilation fails with

error TS2322: Type 'DefaultType' is not assignable to type '{ [key: string]: Mutator<object>; }'.
Index signature is missing in type 'DefaultType'.

DefaultType is defined as follows in final-form-arrays:

export interface DefaultType {
  insert: Mutator
  concat: Mutator
  move: Mutator
  pop: Mutator
  push: Mutator
  removeBatch: Mutator
  remove: Mutator
  shift: Mutator
  swap: Mutator
  update: Mutator
  unshift: Mutator
}

while final-form expects mutators of the type { [key: string]: Mutator<object>; }.

I don't understand why TypeScript fails on this one. But if it's the reason why we have to use the spread operator, I think it'd be better to improve the types rather than force people who use JS to write an ugly syntax.

I fixed the TypeScript error with an even uglier

    mutators={(arrayMutators as unknown) as { [key: string]: Mutator<object> }}
thefat32 commented 4 years ago

Y think { [key: string]: Mutator<object>; } should be satisfied by DefaultType definition. But clearly TS is failing. Did you find out why is necessary to destructure the object?

pgross41 commented 2 years ago

I was wondering the same thing last night looking at final-form-arrays. The light bulb went on when I added the mutator from final-form-set-field-data. When you spread arrayMutators you are actually adding 11 named mutators (insert, concat, etc.). If you didn't want to spread you'd have to pick which ones you want, eg:

<Form
    onSubmit={onSubmit}
    mutators={{
        pop: arrayMutators.pop, 
        insert: arrayMutators.insert, 
        setFieldData
    }}
    render={({