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

<Field> initialValue / defaultValue prop #387

Open crobinson42 opened 5 years ago

crobinson42 commented 5 years ago

Are you submitting a bug report or a feature request?

feature request

What is the current behavior?

A <Form> is required to be provided initialValues to set default values for fields.

What is the expected behavior?

<Field> can accept a prop initialValue or defaultValue that is registered with the parent <Form>'s initial values.

This would be oober convenient in many cases where fields are dynamically generated or conditionally rendered in the <Form>.

I'm happy to work up a PR if you're accept/review the effort @erikras

crobinson42 commented 5 years ago

213

borm commented 5 years ago

Up

rflmyk commented 5 years ago

up

cloneali commented 5 years ago

up

crobinson42 commented 5 years ago

@erikras, any thoughts here?

crobinson42 commented 5 years ago

Currently, my implementation is a bit hacky.

I created a HOC that I wrap my Field components and on componentDidMount triggers the fields input.onChange handler if there is a prop initialValue passed from the Field.

erikras commented 5 years ago

@wmertens has also long requested this. I'd accept a PR implementing this. 👍

The idea is to immediately call change on mount, like @crobinson42's hack? Or some deeper mechanism to actually set the initial value?

crobinson42 commented 5 years ago

The case where I need this is in a stepper/wizard form where fields are added dynamically depending on previous form values. Because of this, there isn't a way (IMO) to set the Form's initialValue (or reset). I guess one question is, does calling input.onChange have other side-effects, ie: meta.touched changes when input.onChange is called?

erikras commented 5 years ago

No, change only changes the value. Without a deeper change to FF, it will make it dirty, though.

You know, this is a decent case in favor of https://github.com/final-form/final-form/pull/199. ðŸĪ”

wmertens commented 5 years ago

One more thing: there are use cases for both initial and default values. Maybe the naming isn't right, but consider these scenarios:

  1. confirmation form with values filled in, to be optionally edited and submitted
  2. field value derived from some user entry elsewhere in the app
  3. field value defaults to some non-null value that the underlying field component needs

IMHO, in 1, the form should be dirty and fields should show errors, in 2 the form shouldn't be dirty but the field can already show errors, and in 3 the form shouldn't be dirty and fields shouldn't show errors.

Maybe we can also add per-field flags dirty and touched?

erikras commented 5 years ago

Published a solution in react-final-form@4.1.0 and final-form@4.12.0. Let me know what you guys think?

crobinson42 commented 5 years ago

@erikras it's my understanding and expectation that defaultValue on a <Field /> should only be set if there are no form initialValues set.

The current behavior is that I'm passing initial values to a Form component and a date Field inside the form has defaultValue={new Date()} which is overwriting the field's value when I'm editing a record in the form.

Thoughts on Field only setting the defaultValue prop if there is not already an initialValues field set?

wmertens commented 5 years ago

Difficult semantics: the date should be set if no initial value is given, but it shouldn't make the form dirty.

Perhaps you should set it as initialValue and let it be overwritten by the form iV if that value is present. However, I would expect field iV to overwrite form iV ðŸĪ”

redbmk commented 5 years ago

@crobinson42 @wmertens @erikras I think the new defaultValue and initialValue fields are handy, but a third option would be really useful that is essentially the same as initialValue but lets the initialValue from Form override it.

I'm not sure what the right name would be here, but I think something like this would work:

const FieldWithDefault = ({ unsetInitialValue, ...props }) => {
  const { initialValues } = useFormState({ initialValues: true });
  const initialValue = prop.name in initialValues
    ? initialValues[prop.name]
    : unsetInitialValue;

  return <Field initialValue={initialValue} {...props} />;
}
crobinson42 commented 5 years ago

@redbmk I don't agree with this approach:

a third option would be really useful that is essentially the same as initialValue but lets the initialValue from Form override it.

My reasoning is by lookin at a hierarchy view of a form with who has priority - the closest data provided to a component/input-field should win.

erikras commented 5 years ago

Yeah, @redbmk, can you make a case for why/when this third overridable field initial value would be useful?

redbmk commented 5 years ago

@crobinson42 I actually thought I was saying the same thing as you

@erikras it's my understanding and expectation that defaultValue on a <Field /> should only be set if there are no form initialValues set.

The current behavior is that I'm passing initial values to a Form component and a date Field inside the form has defaultValue={new Date()} which is overwriting the field's value when I'm editing a record in the form.

Thoughts on Field only setting the defaultValue prop if there is not already an initialValues field set?

Here's a simplified example of the scenario I'm facing:

const initialValues = existingDataFromServer || {}; // empty object for new entries

// form's initialValues
<Form initialValues={data} {...etc} />

// some fields
<label>
  <Field type="checkbox" name="showOnDetailReport" unsetInitialValue={true} />
  Show on detailed report
</label>

<label>
  <Field type="checkbox" name="showOnSummaryReport" />
  Show on summary report
</label>

My thinking is if you're editing an existing record from the server, I would want to use the data that was already stored. But if you're creating a new record, it should start out with the value given in the field. I used unsetInitialValue here because defaultValue and initialValue are already taken and I can't really think of a better name ðŸĪ·â€â™‚.

Maybe the right way to do this is when setting the form's initial values:

const initialValues = {
  showOnDetailReport: true,
  ...existingDataFromServer || {},
};

It just seemed a little more intuitive to have the field itself declare what its default should be. If that's too much of an edge case or anti-pattern I'm happy to use a workaround.

crobinson42 commented 5 years ago

@redbmk I think there's a little ambiguity in the comments here ðŸĪŠ

Here's my understanding...

defaultValue used if there is no initial value for a form/field initialValue populate form/field (takes precedence over defaultValue)

<Form 
  initialValues={{ name: 'cory' }}
  render={() => (
      <Field 
        defaultValue="default" // only used if Form does not have initialValues for this field 
        name="name" 
        type="text" 
      />
  )}
/>
<Form 
  initialValues={{ name: 'cory' }}
  render={() => (
      <Field 
        initialValue="jack" // overrides Form's initialValues for this field
        name="name" 
        type="text" 
      />
  )}
/>
redbmk commented 5 years ago

@crobinson42, hmm... I agree with that concept, and that was what I would have expected defaultValue to do intuitively, but it doesn't actually work that way. According to the docs (and this example I hacked together from another example), defaultValue will override initialValue and mark the field as dirty. From the final-form docs:

defaultValue?: any

⚠ïļ You probably want initialValue! ⚠ïļ

The value of the field upon creation. This value is only needed if you want your field be dirty upon creation (i.e. for its value to be different from its initial value).

I guess what I'm suggesting is either a breaking change for defaultValue, or a third option that behaves the way you describe defaultValue behaving.

dprea commented 5 years ago

I believe defaultValue should be the DEFAULT. That means, if NO initialValue was passed in, the defaultValue would be used. If there is an initialValue, it should override the defaultValue. One work around is using an OR in initialValues and skip using defaultValue all together like so:

const initialValues = { name: item.name || defaultName }

The only problem with this if you need the field to be dirty like defaultValue sets it, you wont get it with this method.

redbmk commented 5 years ago

@erikras should we open another issue for further discussion? Sounds like I'm not the only one that would expect defaultValue to work a little differently than it currently does.

thond1st commented 5 years ago

I think the normal usage would/should be like inline(defaultValue) and header(initialValue) where in the defaultValue will be the value even if there is an initialValue. Landed on this issue because I have initialValues for a set of fields in a FieldArray but my defaultValues for plain Fields displays the defaultValues but goes blank after. I have a bit complex and big form where in I have to use different child components with other Fields in it. Needed to separate them in child components to have separate concerns and limit codes on the parent form. Though I moved the Field Arrays with the parent form as I can't set initialValues for the array in the child component.

valstu commented 5 years ago

I have to agree, I also expected the functionality to be exactly what @crobinson42 and @redbmk expected.

So logic would be something like this: Check if initialValue in <Field /> exists? yes ==> use that value no ==> check if initialValues exists in <Form /> yes ==> use that value no ==> use defaultValue in <Field />

Currently the defaultValue will override both initialValue and initialValues

https://codesandbox.io/s/react-final-form-simple-example-yx7bh

mmahalwy commented 5 years ago

Agree with @valstu on this too. Wondering if it makes sense to either support it or have a prop for it? Or make defaultValue not override but initialValue to

paulhan221 commented 5 years ago

also agree with @valstu , isn't this how it was in redux form?

mmahalwy commented 5 years ago

Running into the same challenge with react-final-form-arrays where I'd like to have a default 1 item. That works but once in editing and reusing the same fields, the initialValue overrides the passed in initialValues :(.

@erikras I am happy to put up a PR for this. We can either:

  1. Have initialValue override Form's initialValues but defaultValue to not
  2. Introduce an additional prop to specify whether it should override initialValues. Maybe the prop is a true/false or enum of overrideInitialValues={'initialValue' | 'defaultValue' | null}
  3. Introduce a non-overriding initialValue prop? ðŸĪ·â€â™‚

My current workaround:

import get from 'lodash/get';
import { useFormState } from 'react-final-form';

...

const { initialValues } = useFormState({ subscription: { initialValues: true } });
const field = useField('name', { initialValue: get(initialValues, 'name') });
broveloper commented 5 years ago

@erikras I created a PR to go in line with what was mentioned above.

@crobinson42 @redbmk @valstu @mmahalwy @paulhan221

redbmk commented 5 years ago

@broveloper Thanks for digging into this and getting the ball rolling! I have a question about the implementation though:

The value of the field upon creation only if both initialValue is undefined and the value from initialValues is also undefined. The field will be dirty when defaultValue is used.

I would expect defaultValue not to make the field dirty. This is already a breaking change, so if we want to adjust that behavior now would be a good time. I can see cases where either behavior would be desirable though. Maybe there could be another configuration option to adjust the behavior would be a good option here?

@wmertens suggested:

Maybe we can also add per-field flags dirty and touched?

I'm not sure if this is what he meant, but it might look something like this:

<Form initialValues={{ qwer: 123, foo: "baz" }} onSubmit={console.log}>
  {(handleSubmit) => (
    <form onSubmit={handleSubmit}>
      <Field name="asdf" defaultValue={123} />
      <Field name="zxcv" defaultValue={456} dirty />
      <Field name="qwer" initialValue={789} touched />
      <Field name="foo" defaultValue="bar" />
    </form>
  )}
</Form>

In this case, if initialValues didn't include any of these fields, then:

wmertens commented 5 years ago

@redbmk indeed, this is least surprising API, it looks great

broveloper commented 5 years ago

@redbmk thanks, applied your suggestions

Also, i didn't want to complicate this issue with the debate over if it should be dirty or not. I feel like that could be opened up as a separate discussion since it could be argued both ways.

I simply wanted to get the ball rolling on having defaultValue function in a way most users desired.

paulhan221 commented 5 years ago

bump