Skip to content

Commit

Permalink
Fix initialValue defaultValue props no longer work in edit views
Browse files Browse the repository at this point in the history
  • Loading branch information
@djhi
djhi committed May 12, 2021
1 parent fa47212 commit 3fb9790
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 17 deletions.
131 changes: 130 additions & 1 deletion packages/ra-core/src/form/FormWithRedirect.spec.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as React from 'react';
import { renderWithRedux } from 'ra-test';
import FormWithRedirect from './FormWithRedirect';
import useInput from './useInput';
import { waitFor } from '@testing-library/dom';

describe('FormWithRedirect', () => {
const Input = props => {
Expand Down Expand Up @@ -39,8 +40,57 @@ describe('FormWithRedirect', () => {
/>
);

expect(renderProp.mock.calls[1][0].pristine).toEqual(true);
expect(renderProp.mock.calls[3][0].pristine).toEqual(true);
expect(getByDisplayValue('Foo')).not.toBeNull();
// 4 times because the first initialization with an empty value
// triggers a change on the input which has a defaultValue
// This is expected and identical to what FinalForm does (https://final-form.org/docs/final-form/types/FieldConfig#defaultvalue)
expect(renderProp).toHaveBeenCalledTimes(4);
});

it('Does not make the form dirty when initialized from a record with a missing field and this field has an initialValue', () => {
const renderProp = jest.fn(() => (
<Input source="name" initialValue="Bar" />
));
const { getByDisplayValue } = renderWithRedux(
<FormWithRedirect
save={jest.fn()}
redirect={false}
saving={false}
version={0}
render={renderProp}
record={{ id: 1 }}
/>
);

expect(renderProp.mock.calls[0][0].pristine).toEqual(true);
expect(getByDisplayValue('Bar')).not.toBeNull();
// 4 times because the first initialization with an empty value
// triggers a change on the input which has a defaultValue
// This is expected and identical to what FinalForm does (https://final-form.org/docs/final-form/types/FieldConfig#defaultvalue)
expect(renderProp).toHaveBeenCalledTimes(1);
});

it('Makes the form dirty when initialized from a record with a missing field and this field has a defaultValue', () => {
const renderProp = jest.fn(() => (
<Input source="name" defaultValue="Bar" />
));
const { getByDisplayValue } = renderWithRedux(
<FormWithRedirect
save={jest.fn()}
redirect={false}
saving={false}
version={0}
render={renderProp}
record={{ id: 1 }}
/>
);

expect(renderProp.mock.calls[1][0].pristine).toEqual(false);
expect(getByDisplayValue('Bar')).not.toBeNull();
// 4 times because the first initialization with an empty value
// triggers a change on the input which has a defaultValue
// This is expected and identical to what FinalForm does (https://final-form.org/docs/final-form/types/FieldConfig#defaultvalue)
expect(renderProp).toHaveBeenCalledTimes(2);
});

Expand Down Expand Up @@ -77,4 +127,83 @@ describe('FormWithRedirect', () => {
expect(getByDisplayValue('Foo')).not.toBeNull();
expect(renderProp).toHaveBeenCalledTimes(2);
});

it('Makes the form dirty when reinitialized from a different record with a missing field and this field has a defaultValue', () => {
const renderProp = jest.fn(() => (
<Input source="name" defaultValue="Bar" />
));
const { getByDisplayValue, rerender } = renderWithRedux(
<FormWithRedirect
save={jest.fn()}
redirect={false}
saving={false}
version={0}
record={{ id: 1, name: 'Foo' }}
render={renderProp}
/>
);

expect(renderProp.mock.calls[0][0].pristine).toEqual(true);
expect(getByDisplayValue('Foo')).not.toBeNull();

rerender(
<FormWithRedirect
save={jest.fn()}
redirect={false}
saving={false}
version={0}
record={{
id: 1,
anotherServerAddedProp: 'Bar',
}}
render={renderProp}
/>
);

expect(renderProp.mock.calls[2][0].pristine).toEqual(false);
expect(getByDisplayValue('Bar')).not.toBeNull();
expect(renderProp).toHaveBeenCalledTimes(3);
});

it('Does not make the form dirty when reinitialized from a different record with a missing field and this field has an initialValue', async () => {
const renderProp = jest.fn(() => (
<Input source="name" initialValue="Bar" />
));
const { getByDisplayValue, rerender } = renderWithRedux(
<FormWithRedirect
save={jest.fn()}
redirect={false}
saving={false}
version={0}
record={{ id: 1, name: 'Foo' }}
render={renderProp}
/>
);

expect(renderProp.mock.calls[0][0].pristine).toEqual(true);
expect(getByDisplayValue('Foo')).not.toBeNull();

rerender(
<FormWithRedirect
save={jest.fn()}
redirect={false}
saving={false}
version={0}
record={{
id: 1,
name: undefined,
anotherServerAddedProp: 'Bar',
}}
render={renderProp}
/>
);

await waitFor(() => {
expect(getByDisplayValue('Bar')).not.toBeNull();
expect(
renderProp.mock.calls[renderProp.mock.calls.length - 1][0]
.pristine
).toEqual(false);
});
});
});
8 changes: 3 additions & 5 deletions packages/ra-core/src/form/FormWithRedirect.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,9 @@ const FormWithRedirect = ({
[setOnSave]
);

const finalInitialValues = getFormInitialValues(
initialValues,
defaultValue,
record
);
const finalInitialValues = useMemo(
() => getFormInitialValues(initialValues, defaultValue, record),
[JSON.stringify({initialValues, defaultValue, record})]); // eslint-disable-line

const submit = values => {
const finalRedirect =
Expand Down
14 changes: 9 additions & 5 deletions packages/ra-core/src/form/getFormInitialValues.ts
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import merge from 'lodash/merge';

export default function getFormInitialValues(
initialValues,
defaultValue,
Expand All @@ -9,11 +11,13 @@ export default function getFormInitialValues(
);
}

return {
...getValues(defaultValue, record),
...getValues(initialValues, record),
...record,
};
const finalInitialValues = merge(
{},
getValues(defaultValue, record),
getValues(initialValues, record),
record
);
return finalInitialValues;
}

function getValues(values, record) {
Expand Down
16 changes: 12 additions & 4 deletions packages/ra-core/src/form/useInitializeFormWithRecord.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useEffect } from 'react';
import { useForm } from 'react-final-form';
import merge from 'lodash/merge';
import isEqual from 'lodash/isEqual';
import getFormInitialValues from './getFormInitialValues';

/**
* Restore the record values which should override any default values specified on the form.
Expand All @@ -13,13 +14,20 @@ const useInitializeFormWithRecord = record => {
return;
}

const initialValues = form.getState().initialValues;
const initialValuesMergedWithRecord = merge({}, initialValues, record);
const initialValues = getFormInitialValues(
form.getState().initialValues,
undefined,
record
);

if (isEqual(form.getState().initialValues, initialValues)) {
return;
}

// Disable this option when re-initializing the form because in this case, it should reset the dirty state of all fields
// We do need to keep this option for dynamically added inputs though which is why it is kept at the form level
form.setConfig('keepDirtyOnReinitialize', false);
form.restart(initialValuesMergedWithRecord);
form.restart(initialValues);
form.setConfig('keepDirtyOnReinitialize', true);
}, [form, JSON.stringify(record)]); // eslint-disable-line react-hooks/exhaustive-deps
};
Expand Down
30 changes: 28 additions & 2 deletions packages/ra-core/src/form/useInput.ts
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import { useCallback, ChangeEvent, FocusEvent, useEffect } from 'react';
import {
useField as useFinalFormField,
FieldProps,
FieldRenderProps,
FieldInputProps,
useForm,
} from 'react-final-form';
import { Validator, composeValidators } from './validate';
import isRequired from './isRequired';
import { useCallback, ChangeEvent, FocusEvent, useEffect } from 'react';
import { useFormGroupContext } from './useFormGroupContext';
import { useFormContext } from './useFormContext';
import { useRecordContext } from '../controller';

export interface InputProps<T = any>
extends Omit<
Expand Down Expand Up @@ -51,6 +53,7 @@ const useInput = ({
const finalName = name || source;
const formGroupName = useFormGroupContext();
const formContext = useFormContext();
const record = useRecordContext();

useEffect(() => {
if (!formContext || !formGroupName) {
Expand All @@ -68,7 +71,8 @@ const useInput = ({
: validate;

const { input, meta } = useFinalFormField(finalName, {
initialValue: initialValue || defaultValue,
initialValue,
defaultValue,
validate: sanitizedValidate,
...options,
});
Expand Down Expand Up @@ -107,6 +111,28 @@ const useInput = ({
[onFocus, customOnFocus]
);

// Every time the record changes and didn't include a value for this field
const form = useForm();
const recordId = record?.id;
useEffect(() => {
if (!!input.value) {

This comment has been minimized.

Sign in to view

Copy link
@andion

andion Jun 21, 2021

This is creating an bug where if the value is 0 it applies the defaultValue (or the initial value, or none), because !!0 = false.

I guess it only happens in NumberInputs because !! "0" is true

This comment has been minimized.

Sign in to view

Copy link
@djhi

djhi Jun 21, 2021

Author Contributor

Good catch! Can you submit a PR ?
return;
}
// Apply the default value if provided
// We use a change here which will make the form dirty but this is expected
// and identical to what FinalForm does (https://final-form.org/docs/final-form/types/FieldConfig#defaultvalue)
if (!!defaultValue) {
form.change(source, defaultValue);
}

if (!!initialValue) {
form.batch(() => {
form.change(source, initialValue);
form.resetFieldState(source);
});
}
}, [recordId, input.value, defaultValue, initialValue, source, form]);

// If there is an input prop, this input has already been enhanced by final-form
// This is required in for inputs used inside other inputs (such as the SelectInput inside a ReferenceInput)
if (options.input) {
Expand Down

0 comments on commit 3fb9790

Please sign in to comment.