Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NumericInput: Initial render fails to preserve floating-point precision #7199

Open
ggdouglas opened this issue Jan 27, 2025 · 0 comments
Open

Comments

@ggdouglas
Copy link
Contributor

Environment

  • Package version(s): v5.16.4
  • Browser name and version: Chrome 132.0.6834.111

Code Sandbox

https://codesandbox.io/p/sandbox/lm7rdk?file=%2Fsrc%2FExample.tsx

Steps to reproduce

  1. Create a controlled <NumericInput> component with:
    • A small floating-point number as the value (e.g., "0.001" or "2.7E-10").
    • A min or max value set, for example:
<NumericInput value="2.7E-10" min={-Number.MAX_VALUE} />
  1. Observe that the input displays "0" as the initial value on the first render.
  2. Click into the <NumericInput> field to see the correct/expected value appear.

Expected behavior

The floating-point value should display accurately on the initial render without user interaction.

Actual behavior

On initial render, the floating-point value is truncated or rounded to "0".

Explanation

The issue seems to originate from the roundAndClampValue logic called in the getDerivedStateFromProps() lifecycle method. By default, stepMaxPrecision is set to 1, causing toMaxPrecision to round the floating-point value to "0".

? NumericInput.roundAndClampValue(value, stepMaxPrecision, props.min, props.max, 0, props.locale)

private static roundAndClampValue(
value: string,
stepMaxPrecision: number,
min: number | undefined,
max: number | undefined,
delta = 0,
locale: string | undefined,
) {
if (!isValueNumeric(value, locale)) {
return NumericInput.VALUE_EMPTY;
}
const currentValue = parseStringToStringNumber(value, locale);
const nextValue = toMaxPrecision(Number(currentValue) + delta, stepMaxPrecision);
const clampedValue = clampValue(nextValue, min, max);
return toLocaleString(clampedValue, locale);
}

/**
* Round the value to have _up to_ the specified maximum precision.
*
* This differs from `toFixed(5)` in that trailing zeroes are not added on
* more precise values, resulting in shorter strings.
*/
export function toMaxPrecision(value: number, maxPrecision: number) {
// round the value to have the specified maximum precision (toFixed is the wrong choice,
// because it would show trailing zeros in the decimal part out to the specified precision)
// source: http://stackoverflow.com/a/18358056/5199574
const scaleFactor = Math.pow(10, maxPrecision);
return Math.round(value * scaleFactor) / scaleFactor;
}

The first time getDerivedStateFromProps is executed, the sanitized value is set as value in the component state. On subsequent executions of getDerivedStateFromProps, didBoundsChange evaluates to false, which causes the original value from props, e.g. "2.7E-10", to be set in the component state. This is why we see the correct value get displayed only after clicking into the field.

Also, conversely, this behavior does not occur if min or max is not set, precisely because didBoundsChange evaluates to false on initial render (and thus the sanitization behavior is not executed on the value from props).

Possible solution

A very hacky workaround for this issue would be to force an additional update on initial mount:

public componentDidMount() {
    this.forceUpdate();
}

However, this seems very suboptimal. A better solution might require us correctly determining the step max precision based on the original value from props in addition to minorStepSize .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant