-
-
Notifications
You must be signed in to change notification settings - Fork 496
the form stays in validating state in a wizard when field is unregistred
#1018
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,6 +4,7 @@ import "@testing-library/jest-dom"; | |||||||||||
| import { ErrorBoundary, Toggle, wrapWith } from "./testUtils"; | ||||||||||||
| import Form from "./ReactFinalForm"; | ||||||||||||
| import Field from "./Field"; | ||||||||||||
| import { useFormState } from "."; | ||||||||||||
|
|
||||||||||||
| const onSubmitMock = (_values) => {}; | ||||||||||||
|
|
||||||||||||
|
|
@@ -1168,6 +1169,159 @@ describe("Field", () => { | |||||||||||
| expect(getByTestId("validating")).toHaveTextContent("Not Validating"); | ||||||||||||
| }); | ||||||||||||
|
|
||||||||||||
| it("should have validating state false after a field has been unregistred during mixed async validation", async () => { | ||||||||||||
| const Test = () => { | ||||||||||||
| const [hasField, setHasField] = React.useState(true); | ||||||||||||
| const state = useFormState({ subscription: { validating: true } }); | ||||||||||||
|
|
||||||||||||
| return ( | ||||||||||||
| <div> | ||||||||||||
| {!hasField && ( | ||||||||||||
| <Field | ||||||||||||
| name="lastname" | ||||||||||||
| component="input" | ||||||||||||
| validate={(value) => (value ? undefined : "Required")} | ||||||||||||
| data-testid="lastname" | ||||||||||||
| /> | ||||||||||||
| )} | ||||||||||||
| {hasField && ( | ||||||||||||
| <Field | ||||||||||||
| name="name" | ||||||||||||
| component="input" | ||||||||||||
| validate={async (value) => { | ||||||||||||
| await timeout(5); | ||||||||||||
| return value === "erikras" ? "Username taken" : undefined; | ||||||||||||
| }} | ||||||||||||
| data-testid="name" | ||||||||||||
| /> | ||||||||||||
| )} | ||||||||||||
| <div data-testid="validating"> | ||||||||||||
| {state.validating === true ? "Spinner" : "Not Validating"} | ||||||||||||
| </div> | ||||||||||||
| <button data-testid="hide" onClick={() => setHasField(false)}> | ||||||||||||
| Hide field | ||||||||||||
|
Comment on lines
+1201
to
+1202
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add explicit The button is inside a form and defaults to Proposed fix- <button data-testid="hide" onClick={() => setHasField(false)}>
+ <button type="button" data-testid="hide" onClick={() => setHasField(false)}>
Hide field
</button>📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome (2.3.14)[error] 1201-1202: Provide an explicit type prop for the button element. The default type of a button is submit, which causes the submission of a form when placed inside a (lint/a11y/useButtonType) 🤖 Prompt for AI Agents |
||||||||||||
| </button> | ||||||||||||
| </div> | ||||||||||||
| ); | ||||||||||||
| }; | ||||||||||||
|
|
||||||||||||
| const { getByTestId, queryByTestId } = render( | ||||||||||||
| <Form onSubmit={onSubmitMock}> | ||||||||||||
| {({ handleSubmit }) => ( | ||||||||||||
| <form onSubmit={handleSubmit}> | ||||||||||||
| <Test /> | ||||||||||||
| </form> | ||||||||||||
| )} | ||||||||||||
| </Form>, | ||||||||||||
| ); | ||||||||||||
|
|
||||||||||||
| expect(getByTestId("validating")).toHaveTextContent("Spinner"); | ||||||||||||
| await sleep(6); | ||||||||||||
| expect(getByTestId("name").value).toBe(""); | ||||||||||||
|
|
||||||||||||
| // validating state is ok as long as a Field with async validation is visible | ||||||||||||
| expect(getByTestId("validating")).toHaveTextContent("Not Validating"); | ||||||||||||
| fireEvent.change(getByTestId("name"), { target: { value: "erik" } }); | ||||||||||||
|
|
||||||||||||
| expect(getByTestId("validating")).toHaveTextContent("Spinner"); | ||||||||||||
| await sleep(6); | ||||||||||||
|
|
||||||||||||
| expect(getByTestId("validating")).toHaveTextContent("Not Validating"); | ||||||||||||
|
|
||||||||||||
| // validating state is KO if Field is unregistered while validating | ||||||||||||
| fireEvent.change(getByTestId("name"), { target: { value: "erikr" } }); | ||||||||||||
|
|
||||||||||||
| expect(getByTestId("validating")).toHaveTextContent("Spinner"); | ||||||||||||
| fireEvent.click(getByTestId("hide")); | ||||||||||||
| expect(queryByTestId("name")).not.toBeInTheDocument(); | ||||||||||||
|
|
||||||||||||
| // when an other field with sync validation is present, it should not have side effect on the async validation | ||||||||||||
| expect(queryByTestId("lastname")).toBeInTheDocument(); | ||||||||||||
| expect(getByTestId("validating")).toHaveTextContent("Spinner"); | ||||||||||||
|
|
||||||||||||
| await sleep(6); | ||||||||||||
|
|
||||||||||||
| expect(getByTestId("validating")).toHaveTextContent("Not Validating"); | ||||||||||||
| }); | ||||||||||||
|
|
||||||||||||
| it("should have validating state false after a field has been unregistred during full async validation", async () => { | ||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo in test name: "unregistred" → "unregistered" Proposed fix- it("should have validating state false after a field has been unregistred during full async validation", async () => {
+ it("should have validating state false after a field has been unregistered during full async validation", async () => {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||
| const Test = () => { | ||||||||||||
| const [hasField, setHasField] = React.useState(true); | ||||||||||||
| const state = useFormState({ subscription: { validating: true } }); | ||||||||||||
|
|
||||||||||||
| return ( | ||||||||||||
| <div> | ||||||||||||
| {!hasField && ( | ||||||||||||
| <Field | ||||||||||||
| name="lastname" | ||||||||||||
| component="input" | ||||||||||||
| validate={async (value) => { | ||||||||||||
| await timeout(5); | ||||||||||||
| return value ? undefined : "Required"; | ||||||||||||
| }} | ||||||||||||
| data-testid="lastname" | ||||||||||||
| /> | ||||||||||||
| )} | ||||||||||||
| {hasField && ( | ||||||||||||
| <Field | ||||||||||||
| name="name" | ||||||||||||
| component="input" | ||||||||||||
| validate={async (value) => { | ||||||||||||
| await timeout(5); | ||||||||||||
| return value === "erikras" ? "Username taken" : undefined; | ||||||||||||
| }} | ||||||||||||
| data-testid="name" | ||||||||||||
| /> | ||||||||||||
| )} | ||||||||||||
| <div data-testid="validating"> | ||||||||||||
| {state.validating === true ? "Spinner" : "Not Validating"} | ||||||||||||
| </div> | ||||||||||||
| <button data-testid="hide" onClick={() => setHasField(false)}> | ||||||||||||
| Hide field | ||||||||||||
|
Comment on lines
+1279
to
+1280
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add explicit Proposed fix- <button data-testid="hide" onClick={() => setHasField(false)}>
+ <button type="button" data-testid="hide" onClick={() => setHasField(false)}>
Hide field
</button>📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome (2.3.14)[error] 1279-1280: Provide an explicit type prop for the button element. The default type of a button is submit, which causes the submission of a form when placed inside a (lint/a11y/useButtonType) 🤖 Prompt for AI Agents |
||||||||||||
| </button> | ||||||||||||
| </div> | ||||||||||||
| ); | ||||||||||||
| }; | ||||||||||||
|
|
||||||||||||
| const { getByTestId, queryByTestId } = render( | ||||||||||||
| <Form onSubmit={onSubmitMock}> | ||||||||||||
| {({ handleSubmit }) => ( | ||||||||||||
| <form onSubmit={handleSubmit}> | ||||||||||||
| <Test /> | ||||||||||||
| </form> | ||||||||||||
| )} | ||||||||||||
| </Form>, | ||||||||||||
| ); | ||||||||||||
|
|
||||||||||||
| expect(getByTestId("validating")).toHaveTextContent("Spinner"); | ||||||||||||
| await sleep(6); | ||||||||||||
| expect(getByTestId("name").value).toBe(""); | ||||||||||||
|
|
||||||||||||
| // validating state is ok as long as a Field with async validation is visible | ||||||||||||
| expect(getByTestId("validating")).toHaveTextContent("Not Validating"); | ||||||||||||
| fireEvent.change(getByTestId("name"), { target: { value: "erik" } }); | ||||||||||||
|
|
||||||||||||
| expect(getByTestId("validating")).toHaveTextContent("Spinner"); | ||||||||||||
| await sleep(6); | ||||||||||||
|
|
||||||||||||
| expect(getByTestId("validating")).toHaveTextContent("Not Validating"); | ||||||||||||
|
|
||||||||||||
| // validating state is KO if Field is unregistered while validating | ||||||||||||
| fireEvent.change(getByTestId("name"), { target: { value: "erikr" } }); | ||||||||||||
|
|
||||||||||||
| expect(getByTestId("validating")).toHaveTextContent("Spinner"); | ||||||||||||
| fireEvent.click(getByTestId("hide")); | ||||||||||||
| expect(queryByTestId("name")).not.toBeInTheDocument(); | ||||||||||||
|
|
||||||||||||
| // when an other field with sync validation is present, it should not have side effect on the async validation | ||||||||||||
| expect(queryByTestId("lastname")).toBeInTheDocument(); | ||||||||||||
|
Comment on lines
+1316
to
+1317
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment is misleading: refers to "sync validation" but this test uses async validation. The comment was likely copied from the mixed validation test. In this "full async" test, Proposed fix- // when an other field with sync validation is present, it should not have side effect on the async validation
+ // when another field with async validation is present, it should not have side effect on the original async validation📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||
| expect(getByTestId("validating")).toHaveTextContent("Spinner"); | ||||||||||||
|
|
||||||||||||
| await sleep(6); | ||||||||||||
|
|
||||||||||||
| expect(getByTestId("validating")).toHaveTextContent("Not Validating"); | ||||||||||||
| }); | ||||||||||||
|
|
||||||||||||
| it("not call record-level validation on Field mount", () => { | ||||||||||||
| const validate = jest.fn(); | ||||||||||||
| const { getByText } = render( | ||||||||||||
|
|
||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in test name: "unregistred" → "unregistered"
Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents