Skip to content
Closed
Show file tree
Hide file tree
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
7 changes: 5 additions & 2 deletions src/useField.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,14 @@ function useField<FormValues: FormValuesShape>(

React.useEffect(
() =>
register((state) => {
register((newState) => {
if (firstRender.current) {
firstRender.current = false;
if(state.initial != newState.initial){
setState(newState);
}
} else {
setState(state);
setState(newState);
}
Comment on lines +115 to 123

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 | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "useField.js" | head -20

Repository: final-form/react-final-form

Length of output: 88


🏁 Script executed:

cat -n ./src/useField.js | head -150

Repository: final-form/react-final-form

Length of output: 5439


🏁 Script executed:

cat -n ./src/useField.js | sed -n '95,140p'

Repository: final-form/react-final-form

Length of output: 1818


🏁 Script executed:

rg "type FieldState" --type js --type ts -A 10

Repository: final-form/react-final-form

Length of output: 53


🏁 Script executed:

cat -n ./src/types.js | head -100

Repository: final-form/react-final-form

Length of output: 117


🏁 Script executed:

fd "types" --type f -e ts -e js | head -20

Repository: final-form/react-final-form

Length of output: 53


🏁 Script executed:

rg "registerField" --type js --type ts -B 2 -A 5 | head -50

Repository: final-form/react-final-form

Length of output: 986


🏁 Script executed:

rg "form.registerField" -B 5 -A 15 | head -80

Repository: final-form/react-final-form

Length of output: 1205


🏁 Script executed:

rg "Object.is\|defaultIsEqual" --type js -B 2 -A 2

Repository: final-form/react-final-form

Length of output: 53


🏁 Script executed:

rg "isEqual" --type js -B 2 -A 2 | head -40

Repository: final-form/react-final-form

Length of output: 2028


🏁 Script executed:

rg "first render\|firstRender" --type js -B 3 -A 3

Repository: final-form/react-final-form

Length of output: 53


🏁 Script executed:

cat -n ./src/useField.js | sed -n '58,92p'

Repository: final-form/react-final-form

Length of output: 1482


🏁 Script executed:

rg "FieldState" --type js -A 5 | head -40

Repository: final-form/react-final-form

Length of output: 2017


🏁 Script executed:

cat -n ./src/useField.js | sed -n '113,130p'

Repository: final-form/react-final-form

Length of output: 690


Violation of stale closure pattern: avoid capturing state in the register callback, and use strict equality.

Lines 59–63 explicitly warn against using state inside register closures because it captures stale state from the current execution context. Line 118 directly violates this by reading state.initial inside the callback. Additionally, loose equality (!=) can coerce null and undefined as equal, skipping real initial value changes. Use a functional updater with strict comparison instead.

🔧 Proposed fix
      register((newState) => {
        if (firstRender.current) {
          firstRender.current = false;
-         if(state.initial != newState.initial){
-           setState(newState);
-         }
+         setState((prev) =>
+           Object.is(prev.initial, newState.initial) ? prev : newState,
+         );
        } else {
          setState(newState);
        }
      }, false),
🤖 Prompt for AI Agents
In `@src/useField.js` around lines 115 - 123, The register callback currently
closes over state and uses loose inequality (state.initial != newState.initial);
change it to avoid reading the captured state by using the setState functional
updater and compare newState.initial to the previous state's initial using
strict !==; specifically, inside the register((newState) => { ... }) callback,
call setState(prev => { if (firstRender.current) { firstRender.current = false;
if (prev.initial !== newState.initial) return newState; return prev; } return
newState; }) so you never reference the outer `state` variable and you use
strict comparison for `initial`.

}, false),
// eslint-disable-next-line react-hooks/exhaustive-deps
Expand Down
23 changes: 23 additions & 0 deletions src/useField.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -455,4 +455,27 @@ describe("useField", () => {
expect(spy.mock.calls[3][1]).toBe(spy.mock.calls[2][1]); // onFocus
expect(spy.mock.calls[3][2]).toBe(spy.mock.calls[2][2]); // onBlur
});

it("should listen to initial value 2", () => {
const MyFieldListener = () => {
const isFirstRender = React.useRef(true)
const { input, meta } = useField("name");
if(!isFirstRender.current){
expect(meta.initial).toBe("test");
// expect(input.value).toBe("test");
}
isFirstRender.current = false
return null;
};
render(
<Form onSubmit={onSubmitMock}>
{() => (
<form>
<MyFieldListener />
<Field name="name" component="input" data-testid="name" initialValue="test"/>
</form>
Comment on lines +459 to +476

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

Make the test fail if the re-render never happens.

The expectation runs only on the second render; if no re-render occurs, the test passes without exercising the behavior. Add an explicit wait/assertion that meta.initial becomes "test".

✅ Suggested adjustment
-import { render, fireEvent, cleanup } from "@testing-library/react";
+import { render, fireEvent, cleanup, waitFor } from "@testing-library/react";

-  it("should listen to initial value 2", () => {
+  it("should listen to initial value 2", async () => {
+    const initialSpy = jest.fn();
     const MyFieldListener = () => {
-      const isFirstRender = React.useRef(true)
-      const { input, meta } = useField("name");
-      if(!isFirstRender.current){
-        expect(meta.initial).toBe("test");
-        // expect(input.value).toBe("test");
-      }
-      isFirstRender.current = false
+      const { meta } = useField("name");
+      React.useEffect(() => {
+        initialSpy(meta.initial);
+      }, [meta.initial]);
       return null;
     };
     render(
       <Form onSubmit={onSubmitMock}>
         {() => (
           <form>
             <MyFieldListener />
             <Field name="name" component="input" data-testid="name" initialValue="test"/>
           </form>
         )}
       </Form>,
     );
+    await waitFor(() => expect(initialSpy).toHaveBeenCalledWith("test"));
   });
🤖 Prompt for AI Agents
In `@src/useField.test.js` around lines 459 - 476, The test currently only asserts
meta.initial inside MyFieldListener on the second render, so it will pass if no
re-render occurs; update the test to explicitly fail when the re-render never
happens by adding a wait/assertion after render that checks meta.initial becomes
"test" (e.g., use waitFor or a polling expect) or have MyFieldListener flip a
ref/state or call a jest mock when it observes meta.initial === "test" and then
assert that mock was called; target the MyFieldListener component and the
useField hook's meta.initial to ensure the test fails if the re-render never
occurs.

)}
</Form>,
);
});
});