Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 154 additions & 0 deletions src/Field.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {};

Expand Down Expand Up @@ -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 () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Typo in test name: "unregistred" → "unregistered"

Proposed fix
-  it("should have validating state false after a field has been unregistred during mixed async validation", async () => {
+  it("should have validating state false after a field has been unregistered during mixed async validation", async () => {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it("should have validating state false after a field has been unregistred during mixed async validation", async () => {
it("should have validating state false after a field has been unregistered during mixed async validation", async () => {
🤖 Prompt for AI Agents
In `@src/Field.test.js` at line 1172, Rename the test title string in the test
case for mixed async validation from "should have validating state false after a
field has been unregistred during mixed async validation" to "should have
validating state false after a field has been unregistered during mixed async
validation" so the test name uses correct spelling; locate the it(...) call with
that exact string in the test file (the test case surrounding the mixed async
validation scenario) and update the description only, leaving the test body and
identifier unchanged.

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

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add explicit type="button" to prevent default form submission.

The button is inside a form and defaults to type="submit". While it works here because the test expects the click, adding an explicit type improves clarity and avoids accidental submission behavior.

Proposed fix
-          <button data-testid="hide" onClick={() => setHasField(false)}>
+          <button type="button" data-testid="hide" onClick={() => setHasField(false)}>
             Hide field
           </button>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<button data-testid="hide" onClick={() => setHasField(false)}>
Hide field
<button type="button" data-testid="hide" onClick={() => setHasField(false)}>
Hide field
</button>
🧰 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 form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset

(lint/a11y/useButtonType)

🤖 Prompt for AI Agents
In `@src/Field.test.js` around lines 1201 - 1202, The hide button inside the form
is missing an explicit type and will default to submit; update the button
element with data-testid="hide" (the one using onClick={() =>
setHasField(false)}) to include type="button" so clicking it won't trigger form
submission—locate the button in Field.test.js and add the type attribute to that
element.

</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 () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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 () => {
🤖 Prompt for AI Agents
In `@src/Field.test.js` at line 1247, The test description contains a typo: change
the "it" description string in the test case currently titled "should have
validating state false after a field has been unregistred during full async
validation" to use the correct spelling "unregistered" (i.e., update the test
declaration for that case so the string reads "...after a field has been
unregistered during full async validation").

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

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add explicit type="button" to prevent default form submission.

Proposed fix
-          <button data-testid="hide" onClick={() => setHasField(false)}>
+          <button type="button" data-testid="hide" onClick={() => setHasField(false)}>
             Hide field
           </button>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<button data-testid="hide" onClick={() => setHasField(false)}>
Hide field
<button type="button" data-testid="hide" onClick={() => setHasField(false)}>
Hide field
</button>
🧰 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 form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset

(lint/a11y/useButtonType)

🤖 Prompt for AI Agents
In `@src/Field.test.js` around lines 1279 - 1280, The button with
data-testid="hide" currently lacks an explicit type so it may submit its parent
form; update the JSX for the Hide field button (the element that calls
setHasField(false)) to include type="button" to prevent default form submission
and ensure it only triggers the onClick handler.

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

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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, lastname also has async validation.

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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// when an other field with sync validation is present, it should not have side effect on the async validation
expect(queryByTestId("lastname")).toBeInTheDocument();
// when another field with async validation is present, it should not have side effect on the original async validation
expect(queryByTestId("lastname")).toBeInTheDocument();
🤖 Prompt for AI Agents
In `@src/Field.test.js` around lines 1316 - 1317, The inline test comment is
incorrect: it mentions "sync validation" but this test exercises full async
validation; update the comment guarding the assertion that references
queryByTestId("lastname") to state that the other field uses async validation
(or remove the misleading clause entirely) so it correctly describes the test
scenario; locate the assertion containing queryByTestId("lastname") in
Field.test.js and edit the preceding comment to reference "async validation" (or
simply say "another field with validation") to match the test behavior.

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(
Expand Down