Skip to content

Commit b037c53

Browse files
author
erikras-dinesh-agent
committed
Merge remote-tracking branch 'origin/main' into fix-869-dynamic-field-name
2 parents 8babb88 + e82df67 commit b037c53

3 files changed

Lines changed: 83 additions & 14 deletions

File tree

src/Field.test.js

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1338,3 +1338,42 @@ describe("Field", () => {
13381338
expect(getByTestId("thirty").checked).toBe(true);
13391339
});
13401340
});
1341+
1342+
describe("Field.nodeName issue #871", () => {
1343+
it("should not crash when field name is 'nodeName'", () => {
1344+
const onSubmit = jest.fn();
1345+
const { getByTestId } = render(
1346+
<Form onSubmit={onSubmit}>
1347+
{({ handleSubmit }) => (
1348+
<form onSubmit={handleSubmit}>
1349+
<Field
1350+
name="nodeName"
1351+
component="input"
1352+
data-testid="nodeName-input"
1353+
/>
1354+
<button type="submit" data-testid="submit">
1355+
Submit
1356+
</button>
1357+
</form>
1358+
)}
1359+
</Form>,
1360+
);
1361+
1362+
const input = getByTestId("nodeName-input");
1363+
const submit = getByTestId("submit");
1364+
1365+
// Should not crash when interacting with the field
1366+
expect(() => {
1367+
fireEvent.change(input, { target: { value: "test" } });
1368+
fireEvent.blur(input);
1369+
fireEvent.click(submit);
1370+
}).not.toThrow();
1371+
1372+
// Verify the field value was set correctly
1373+
expect(onSubmit).toHaveBeenCalledWith(
1374+
{ nodeName: "test" },
1375+
expect.any(Object),
1376+
expect.any(Function),
1377+
);
1378+
});
1379+
});

src/Field.tsx

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,19 +68,20 @@ function FieldComponent<
6868

6969
if (typeof component === "string") {
7070
// ignore meta, combine input with any other props
71-
const inputProps = { ...mergedField.input };
71+
const { name: inputName, ...restInputProps } = mergedField.input;
7272

7373
// Ensure multiple select has array value
7474
if (
7575
component === "select" &&
7676
multiple &&
77-
!Array.isArray(inputProps.value)
77+
!Array.isArray(restInputProps.value)
7878
) {
79-
inputProps.value = [] as any;
79+
restInputProps.value = [] as any;
8080
}
8181

8282
return React.createElement(component, {
83-
...inputProps,
83+
name: inputName, // Pass name explicitly to avoid shadowing DOM properties
84+
...restInputProps,
8485
children,
8586
ref,
8687
...rest,

src/useFormState.ts

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,32 @@ function useFormState<FormValues = Record<string, any>>({
1414
const onChangeRef = React.useRef(onChange);
1515
onChangeRef.current = onChange;
1616

17-
// Initialize state with current form state without callbacks
18-
const [state, setState] = React.useState<FormState<FormValues>>(() => {
19-
// Get initial state synchronously but without callbacks
20-
return form.getState();
21-
});
17+
// Initialize with current form state WITHOUT triggering callbacks during render.
18+
// We intentionally use getState() here so render-prop consumers (e.g. <FormSpy>{...})
19+
// can read a fully-populated initial state on first render.
20+
const [state, setState] = React.useState<FormState<FormValues>>(() =>
21+
form.getState(),
22+
);
23+
24+
// We want `onChange` to be called AFTER render (fixes #809) and only with the
25+
// subscription-filtered state.
26+
const firstSubscriptionRef = React.useRef(true);
27+
const pendingOnChangeRef = React.useRef<FormState<FormValues> | null>(null);
28+
const lastOnChangeRef = React.useRef<FormState<FormValues> | null>(null);
2229

2330
React.useEffect(() => {
24-
// Subscribe to form state changes after initial render
2531
const unsubscribe = form.subscribe((newState) => {
32+
// Ensure we set state at least once from the subscription, even if equal,
33+
// so that `onChange` can be fired from an effect after the first render.
34+
const isFirst = firstSubscriptionRef.current;
35+
if (isFirst) {
36+
firstSubscriptionRef.current = false;
37+
}
38+
39+
pendingOnChangeRef.current = newState;
40+
2641
setState((prevState) => {
27-
if (!shallowEqual(newState, prevState)) {
28-
if (onChangeRef.current) {
29-
onChangeRef.current(newState);
30-
}
42+
if (isFirst || !shallowEqual(newState, prevState)) {
3143
return newState;
3244
}
3345
return prevState;
@@ -38,6 +50,23 @@ function useFormState<FormValues = Record<string, any>>({
3850
// eslint-disable-next-line react-hooks/exhaustive-deps
3951
}, []);
4052

53+
React.useEffect(() => {
54+
const pending = pendingOnChangeRef.current;
55+
if (!pending || !onChangeRef.current) {
56+
return;
57+
}
58+
59+
// Only fire when the subscription has produced a new state and it differs
60+
// from what we've already emitted.
61+
if (lastOnChangeRef.current === null || !shallowEqual(pending, lastOnChangeRef.current)) {
62+
onChangeRef.current(pending);
63+
lastOnChangeRef.current = pending;
64+
}
65+
66+
// Clear pending once we've handled it.
67+
pendingOnChangeRef.current = null;
68+
}, [state]);
69+
4170
const lazyState = {};
4271
addLazyFormState(lazyState, state);
4372
return lazyState as FormState<FormValues>;

0 commit comments

Comments
 (0)