Skip to content

Commit 58c3ba0

Browse files
committed
fixes a infinite loop updating value problem
1 parent 8686d95 commit 58c3ba0

4 files changed

Lines changed: 377 additions & 6 deletions

File tree

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,4 @@
5454
"react": ">=17.0.0"
5555
},
5656
"packageManager": "yarn@1.22.19"
57-
}
57+
}

src/Form/index.tsx

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,25 @@ function Form(props: FormProps, ref: React.Ref<FormRef>) {
4848
}
4949
}, [props.state || {}])
5050

51-
// when the state of the form changes, we call the onChange function
51+
// Notify parent *only* when our internal state changed and *not* immediately
52+
// after the parent has just provided us with a new `props.state`. We detect
53+
// an external change by keeping the previous `props.state` (cloned into
54+
// `propsState`) in a ref and comparing it to the current one. This prevents
55+
// a feedback-loop where the form sends its stale value back to the parent
56+
// right after the parent updated it (see failing test case).
57+
const prevPropsStateRef = useRef(propsState)
58+
5259
useEffect(() => {
60+
const propsStateChanged = !isEqual(prevPropsStateRef.current, propsState)
61+
62+
// Update ref for next render cycle
63+
prevPropsStateRef.current = propsState
64+
5365
if (!props.onChange) return
54-
if (isEqual(propsState, state)) return
55-
// Use startTransition for parent notifications to keep form fields responsive
66+
if (isEqual(propsState, state)) return // nothing changed
67+
if (propsStateChanged) return // skip, change came from parent
68+
69+
// Internal change → notify parent
5670
startTransition(() => {
5771
props.onChange(state)
5872
})

src/NumberInput.test.tsx

Lines changed: 355 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,355 @@
1+
import {fireEvent, render, screen} from '@testing-library/react'
2+
import {act, useEffect, useState} from 'react'
3+
import Field from './Field'
4+
import Form from './Form'
5+
import {FieldProps} from './types'
6+
import '@testing-library/jest-dom'
7+
8+
jest.useFakeTimers()
9+
10+
// Mock TextInput component since it's external
11+
interface TextInputProps {
12+
value: string
13+
onChange: (value: string) => void
14+
onBlur?: () => void
15+
}
16+
17+
function MockTextInput(props: TextInputProps) {
18+
const {onChange} = props
19+
return (
20+
<input
21+
data-testid="number-input"
22+
value={props.value}
23+
onChange={e => onChange(e.target.value)}
24+
onBlur={props.onBlur}
25+
/>
26+
)
27+
}
28+
29+
// Mock remeda functions
30+
const isNumber = (value: any): value is number => typeof value === 'number' && !Number.isNaN(value)
31+
const round = (value: number, decimals: number): number => {
32+
return Math.round(value * 10 ** decimals) / 10 ** decimals
33+
}
34+
35+
// NumberInput component (as provided by user)
36+
type Props = Omit<TextInputProps, 'onChange'> & {
37+
onBlur?: (value: number) => any
38+
maxDecimals?: number
39+
inputClassName?: string
40+
}
41+
42+
const locale = 'es-CL'
43+
44+
function format(value: number) {
45+
return value.toLocaleString(locale)
46+
}
47+
48+
function unformat(text: string, props: FieldProps<number, Props>) {
49+
if (text === '-') return 0
50+
const parts = (1234.5).toLocaleString(locale).match(/(\D+)/g)
51+
let unformatted = text
52+
53+
unformatted = unformatted.split(parts[0]).join('')
54+
unformatted = unformatted.split(parts[1]).join('.')
55+
56+
const float = Number.parseFloat(unformatted)
57+
58+
return round(float, isNumber(props.maxDecimals) ? props.maxDecimals : 8)
59+
}
60+
61+
function NumberInput(props: FieldProps<number, Props>) {
62+
const value = Number(props.value)
63+
64+
const [text, _setText] = useState<string>(
65+
!isNumber(props.value) || Number.isNaN(props.value) ? '' : format(value),
66+
)
67+
68+
const setText = (text: string) => {
69+
_setText(text)
70+
if (!props.onChange) return
71+
72+
if (text) {
73+
const unformatted = unformat(text, props)
74+
75+
if (Number.isNaN(unformatted)) {
76+
_setText('')
77+
} else {
78+
const newValue = unformatted
79+
80+
if (props.value !== newValue) {
81+
props.onChange(newValue)
82+
}
83+
}
84+
} else {
85+
if (isNumber(props.value)) {
86+
props.onChange(null)
87+
}
88+
}
89+
}
90+
91+
const newText = !isNumber(props.value) || Number.isNaN(props.value) ? '' : format(value)
92+
93+
useEffect(() => {
94+
if (newText !== text) {
95+
setText(newText)
96+
}
97+
}, [newText])
98+
99+
return (
100+
<MockTextInput
101+
{...props}
102+
value={text}
103+
onChange={setText}
104+
onBlur={() => {
105+
const unformattedOnBlur = unformat(text, props)
106+
const formattedOnBlur = format(unformattedOnBlur)
107+
108+
setText(formattedOnBlur)
109+
if (props.onBlur) {
110+
props.onBlur(unformattedOnBlur)
111+
}
112+
}}
113+
/>
114+
)
115+
}
116+
117+
describe('NumberInput', () => {
118+
test('should format numbers according to es-CL locale', () => {
119+
render(
120+
<Form state={{price: 1234.56}}>
121+
<Field fieldName="price" type={NumberInput} />
122+
</Form>,
123+
)
124+
125+
const input = screen.getByTestId('number-input')
126+
expect(input).toHaveValue('1.234,56')
127+
})
128+
129+
test('should handle empty/undefined values', () => {
130+
render(
131+
<Form state={{price: undefined}}>
132+
<Field fieldName="price" type={NumberInput} />
133+
</Form>,
134+
)
135+
136+
const input = screen.getByTestId('number-input')
137+
expect(input).toHaveValue('')
138+
})
139+
140+
test('should handle NaN values', () => {
141+
render(
142+
<Form state={{price: Number.NaN}}>
143+
<Field fieldName="price" type={NumberInput} />
144+
</Form>,
145+
)
146+
147+
const input = screen.getByTestId('number-input')
148+
expect(input).toHaveValue('')
149+
})
150+
151+
test('should unformat user input and update form state', async () => {
152+
let formState = {price: 0}
153+
154+
function TestForm() {
155+
const [state, setState] = useState(formState)
156+
formState = state
157+
158+
return (
159+
<Form state={state} onChange={setState}>
160+
<Field fieldName="price" type={NumberInput} />
161+
</Form>
162+
)
163+
}
164+
165+
render(<TestForm />)
166+
const input = screen.getByTestId('number-input')
167+
168+
await act(async () => {
169+
fireEvent.change(input, {target: {value: '1.500,75'}})
170+
jest.advanceTimersByTime(0)
171+
})
172+
173+
expect(formState.price).toBe(1500.75)
174+
})
175+
176+
test('should handle dash input as zero', async () => {
177+
let formState = {price: 0}
178+
179+
function TestForm() {
180+
const [state, setState] = useState(formState)
181+
formState = state
182+
183+
return (
184+
<Form state={state} onChange={setState}>
185+
<Field fieldName="price" type={NumberInput} />
186+
</Form>
187+
)
188+
}
189+
190+
render(<TestForm />)
191+
const input = screen.getByTestId('number-input')
192+
193+
await act(async () => {
194+
fireEvent.change(input, {target: {value: '-'}})
195+
jest.advanceTimersByTime(0)
196+
})
197+
198+
expect(formState.price).toBe(0)
199+
})
200+
201+
test('should clear invalid input and reset to empty', async () => {
202+
let formState = {price: 100}
203+
204+
function TestForm() {
205+
const [state, setState] = useState(formState)
206+
formState = state
207+
208+
return (
209+
<Form state={state} onChange={setState}>
210+
<Field fieldName="price" type={NumberInput} />
211+
</Form>
212+
)
213+
}
214+
215+
render(<TestForm />)
216+
const input = screen.getByTestId('number-input')
217+
218+
await act(async () => {
219+
fireEvent.change(input, {target: {value: 'invalid text'}})
220+
jest.advanceTimersByTime(0)
221+
})
222+
223+
expect(input).toHaveValue('')
224+
})
225+
226+
test('should set value to null when clearing input', async () => {
227+
let formState = {price: 100}
228+
229+
function TestForm() {
230+
const [state, setState] = useState(formState)
231+
formState = state
232+
233+
return (
234+
<Form state={state} onChange={setState}>
235+
<Field fieldName="price" type={NumberInput} />
236+
</Form>
237+
)
238+
}
239+
240+
render(<TestForm />)
241+
const input = screen.getByTestId('number-input')
242+
243+
await act(async () => {
244+
fireEvent.change(input, {target: {value: ''}})
245+
jest.advanceTimersByTime(0)
246+
})
247+
248+
expect(formState.price).toBe(null)
249+
})
250+
251+
test('should format value on blur', async () => {
252+
render(
253+
<Form state={{price: 0}}>
254+
<Field fieldName="price" type={NumberInput} />
255+
</Form>,
256+
)
257+
258+
const input = screen.getByTestId('number-input')
259+
260+
await act(async () => {
261+
fireEvent.change(input, {target: {value: '1500,5'}})
262+
jest.advanceTimersByTime(0)
263+
})
264+
265+
await act(async () => {
266+
fireEvent.blur(input)
267+
jest.advanceTimersByTime(0)
268+
})
269+
270+
expect(input).toHaveValue('1.500,5')
271+
})
272+
273+
test('should call onBlur callback with numeric value', async () => {
274+
const onBlurMock = jest.fn()
275+
276+
render(
277+
<Form state={{price: 0}}>
278+
<Field fieldName="price" type={NumberInput} onBlur={onBlurMock} />
279+
</Form>,
280+
)
281+
282+
const input = screen.getByTestId('number-input')
283+
284+
await act(async () => {
285+
fireEvent.change(input, {target: {value: '1.234,56'}})
286+
jest.advanceTimersByTime(0)
287+
})
288+
289+
await act(async () => {
290+
fireEvent.blur(input)
291+
jest.advanceTimersByTime(0)
292+
})
293+
294+
expect(onBlurMock).toHaveBeenCalledWith(1234.56)
295+
})
296+
297+
test('should update display when value changes externally', async () => {
298+
function TestForm() {
299+
const [state, setState] = useState({price: 100})
300+
301+
return (
302+
<div>
303+
<Form state={state} onChange={setState}>
304+
<Field fieldName="price" type={NumberInput} />
305+
</Form>
306+
<button
307+
type="button"
308+
onClick={() => setState({price: 2500.75})}
309+
data-testid="external-update"
310+
>
311+
Update Price
312+
</button>
313+
</div>
314+
)
315+
}
316+
317+
render(<TestForm />)
318+
const input = screen.getByTestId('number-input')
319+
const button = screen.getByTestId('external-update')
320+
321+
expect(input).toHaveValue('100')
322+
323+
await act(async () => {
324+
fireEvent.click(button)
325+
jest.advanceTimersByTime(0)
326+
})
327+
328+
expect(input).toHaveValue('2.500,75')
329+
})
330+
331+
test('should handle decimal-only input', async () => {
332+
let formState = {price: 0}
333+
334+
function TestForm() {
335+
const [state, setState] = useState(formState)
336+
formState = state
337+
338+
return (
339+
<Form state={state} onChange={setState}>
340+
<Field fieldName="price" type={NumberInput} />
341+
</Form>
342+
)
343+
}
344+
345+
render(<TestForm />)
346+
const input = screen.getByTestId('number-input')
347+
348+
await act(async () => {
349+
fireEvent.change(input, {target: {value: ',75'}})
350+
jest.advanceTimersByTime(0)
351+
})
352+
353+
expect(formState.price).toBe(0.75)
354+
})
355+
})

0 commit comments

Comments
 (0)