Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<Field> initialValue / defaultValue prop #387

Open
crobinson42 opened this issue Nov 27, 2018 · 31 comments
Open

<Field> initialValue / defaultValue prop #387

crobinson42 opened this issue Nov 27, 2018 · 31 comments

Comments

@crobinson42
Copy link

crobinson42 commented Nov 27, 2018

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
Copy link
Author

#213

@borm
Copy link

borm commented Dec 20, 2018

Up

@rflmyk
Copy link

rflmyk commented Feb 12, 2019

up

1 similar comment
@cloneali
Copy link

up

@crobinson42
Copy link
Author

@erikras, any thoughts here?

@crobinson42
Copy link
Author

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
Copy link
Member

erikras commented Feb 25, 2019

@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
Copy link
Author

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
Copy link
Member

erikras commented Feb 25, 2019

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 final-form/final-form#199. 🤔

@wmertens
Copy link

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
Copy link
Member

erikras commented Mar 4, 2019

Published a solution in [email protected] and [email protected]. Let me know what you guys think?

@crobinson42
Copy link
Author

@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
Copy link

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
Copy link
Contributor

redbmk commented Jul 6, 2019

@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
Copy link
Author

@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
Copy link
Member

erikras commented Jul 6, 2019

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

@redbmk
Copy link
Contributor

redbmk commented Jul 6, 2019

@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
Copy link
Author

@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
Copy link
Contributor

redbmk commented Jul 7, 2019

@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
Copy link

dprea commented Jul 9, 2019

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
Copy link
Contributor

redbmk commented Jul 29, 2019

@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
Copy link

thond1st commented Aug 6, 2019

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
Copy link

valstu commented Aug 13, 2019

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
Copy link

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

@erikras erikras reopened this Aug 19, 2019
@paulhan221
Copy link

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

@mmahalwy
Copy link

mmahalwy commented Aug 27, 2019

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
Copy link

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

@crobinson42 @redbmk @valstu @mmahalwy @paulhan221

@redbmk
Copy link
Contributor

redbmk commented Sep 19, 2019

@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:

  • asdf would be set to 123 but wouldn't be dirty or touched
  • zxcv would be set to 456 and would be dirty but not touched
  • qwer would be set to 789 and would be marked as touched (e.g. in order to show errors)
  • foo would be set to "baz" and wouldn't be dirty or touched

@wmertens
Copy link

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

@broveloper
Copy link

@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
Copy link

bump

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests