Skip to content

Commit

Permalink
fixed issue with validateData being stale (#215)
Browse files Browse the repository at this point in the history
  • Loading branch information
strahinjaajvaz committed Apr 28, 2021
1 parent 9ad66d8 commit 803a16b
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 21 deletions.
17 changes: 11 additions & 6 deletions src/components/Form.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,17 @@ function Form(_props) {

lastMouseDownInputElement.current = inputElement;
};
const registerField = (name, field) => {
const registerField = useCallback((name, field) => {
fields.current[name] = field;
};
const unregisterField = (name) => {
}, []);
const unregisterField = useCallback((name) => {
delete fields.current[name];
};
setState((state) =>
deletePath(state, `errors.${name}`, {
deleteEmptyObjects: { except: ["errors"] },
})
);
}, []);
const providerValue = {
state,
onFocus, // should be called by inputs
Expand All @@ -140,7 +145,7 @@ function Form(_props) {
};
const getFieldErrors = useCallback((values, name) => {
const value = getPath(values, name);
const field = fields.current[name];
const field = fields.current[name].current;

if (
!field || // See: https://stackoverflow.com/q/65659161/247243
Expand Down Expand Up @@ -182,7 +187,7 @@ function Form(_props) {
);
const validateField = useCallback(
(name) => {
if (fields.current[name]) {
if (fields.current[name].current) {
setState((state) => {
const errors = getFieldErrors(state.values, name);

Expand Down
84 changes: 76 additions & 8 deletions src/components/Form.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,17 @@ const hungryOptions = [
},
];

const yesNoOptions = [
{
label: "Yes",
value: "yes",
},
{
label: "No",
value: "no",
},
];

function SimpleForm({ testId }) {
const initialValues = {
name: "",
Expand Down Expand Up @@ -334,14 +345,12 @@ describe("Form", () => {

userEvent.type(screen.getByLabelText("Name"), "Helena");

await waitFor(() => {
expect(
screen.queryByText("This name is already taken.")
).not.toBeInTheDocument();
expect(
screen.queryByText("Try to spell it differently.")
).not.toBeInTheDocument();
});
expect(
screen.queryByText("This name is already taken.")
).not.toBeInTheDocument();
expect(
screen.queryByText("Try to spell it differently.")
).not.toBeInTheDocument();
});

it("doesn't validate fields when they become disabled", async () => {
Expand Down Expand Up @@ -613,3 +622,62 @@ describe("Form", () => {
});
});
});

it("with hidden fields", async () => {
const onSubmit = jest.fn();
const { container } = render(
<Form
initialValues={{ hasMiddleName: "", middleName: "" }}
onSubmit={onSubmit}
>
{({ state }) => (
<>
<RadioGroup
name="hasMiddleName"
label="Do you have a middle name?"
options={yesNoOptions}
/>
{state.values.hasMiddleName === "yes" && (
<Input name="middleName" label="Middle name" />
)}
<Button type="submit">Submit</Button>
</>
)}
</Form>
);

const yesInput = screen.getByLabelText("Yes");
const yesLabel = container.querySelector(
`label[for="${yesInput.getAttribute("id")}"]`
);
const noInput = screen.getByLabelText("No");
const noLabel = container.querySelector(
`label[for="${noInput.getAttribute("id")}"]`
);

userEvent.click(yesLabel);

const middleNameInput = screen.getByLabelText("Middle name");
middleNameInput.focus();
middleNameInput.blur();

// Wait until form state is updated with the error.
// Without this await, the test ends before form's state gets the error.
await waitFor(() => {
expect(screen.getByText("Required")).toBeInTheDocument();
});

userEvent.click(noLabel);
screen.getByRole("button", { name: "Submit" }).click();

await waitFor(() =>
expect(onSubmit).toBeCalledWith({
errors: {},
values: {
hasMiddleName: "no",
middleName: "",
},
setErrors: expect.any(Function),
})
);
});
32 changes: 25 additions & 7 deletions src/hooks/internal/useField.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import { useEffect } from "react";
import { useEffect, useRef } from "react";
import useForm from "./useForm";
import { getPath } from "../../utils/objectPath";

function useField(componentName, { name, disabled, optional, validate, data }) {
if (typeof name !== "string" || name.trim() === "") {
throw new Error(`${componentName} component is missing a name prop`);
}

const {
state,
onFocus,
Expand All @@ -17,6 +16,16 @@ function useField(componentName, { name, disabled, optional, validate, data }) {
unregisterField,
} = useForm(componentName);

const registerDataRef = useRef({
registerField,
unregisterField,
name,
disabled,
optional,
validate,
data,
});

if (typeof state.values === "undefined") {
throw new Error("Form is missing initialValues");
}
Expand All @@ -30,16 +39,17 @@ function useField(componentName, { name, disabled, optional, validate, data }) {
const errors = getPath(state.errors, name);
const hasErrors = Array.isArray(errors) && errors.length > 0;

// we need to store the data in a ref so that if the validateData prop changes, we have a reference
// that we can update with the new value.
useEffect(() => {
registerField(name, {
registerDataRef.current = {
registerField,
unregisterField,
name,
disabled,
optional,
validate,
data,
});

return () => {
unregisterField(name);
};
}, [
name,
Expand All @@ -51,6 +61,14 @@ function useField(componentName, { name, disabled, optional, validate, data }) {
unregisterField,
]);

useEffect(() => {
registerField(name, registerDataRef);

return () => {
unregisterField(name);
};
}, [registerField, unregisterField, name]);

return {
value,
errors,
Expand Down

1 comment on commit 803a16b

@vercel
Copy link

@vercel vercel bot commented on 803a16b Apr 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.