adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.58k stars 1.09k forks source link

`NumberField` value is reset when re-rendered #5900

Open polc opened 6 months ago

polc commented 6 months ago

Provide a general summary of the issue here

When filling a NumberField with a non-empty formatOptions property, a re-render will erase the current value.

Everything work correctly if I omit the formatOptions property.

🖥️ Steps to Reproduce

To reproduce, try to fill the following input : Code Sandbox

Version

What browsers are you seeing the problem on?

Firefox, Chrome

What operating system are you using?

Arch Linux

🕷 Tracking Issue

No response

ryo-manba commented 6 months ago

It seems that if you remove the following code, the input is retained even if re-rendered. https://github.com/adobe/react-spectrum/blob/8b7fb3e2b3d557bf7932dd541c5702d218354ad9/packages/%40react-stately/numberfield/src/useNumberFieldState.ts#L139

However, it leads to an increased number of clicks required for spinbutton's increment and decrement actions, more investigation needed!

sookmax commented 6 months ago

To piggyback on @ryo-manba's comment, it looks like the following if-statement runs whenever formatOptions !== prevFormatOptions (which only checks the referential equality since formatOptions is an object): https://github.com/adobe/react-spectrum/blob/8b7fb3e2b3d557bf7932dd541c5702d218354ad9/packages/%40react-stately/numberfield/src/useNumberFieldState.ts#L138-L143

And since the following code in the example passes a new formatOptions object to <NumberField /> on every render:

<NumberField
  formatOptions={{
    style: "percent",
    maximumFractionDigits: 0,
  }}
>
  <Input />
</NumberField>

The following block of code (from the if-statement above) always executes on every re-render:

{ 
   setInputValue(format(numberValue)); 
   setPrevValue(numberValue); 
   setPrevLocale(locale); 
   setPrevFormatOptions(formatOptions); 
 }

I'm not sure why executing the above block would lead to resetting the input value without further digging though.

But one way to workaround this is to declare formatOptions outside the component as constant so that the referential inequality check formatOptions !== prevFormatOptions is always false(of course, do this only when your formatting doesn't change over time).

Here is a codesandbox that demonstrates this: sandbox

snowystinger commented 6 months ago

Thanks for the in-depth dive @sookmax ! Linking to a related issue https://github.com/adobe/react-spectrum/issues/1893