NumberField disabled state conflicts with onBlur flushSync

[ce2kettu]

Provide a general summary of the issue here

Due to disabling the NumberField when the controlled value is changed but onBlur has not yet triggered, react throws an error: flushSync was called from inside a lifecycle method. React cannot flush when React is already rendering. Consider moving this call to a scheduler task or micro task.

🤔 Expected Behavior?

The value should change and then the NumberField should be disabled.

😯 Current Behavior

Value does not change and throws a react error: flushSync was called from inside a lifecycle method. React cannot flush when React is already rendering. Consider moving this call to a scheduler task or micro task.

💁 Possible Solution

Commit the value immediately when the field becomes disabled?

🔦 Context

Disabling the field if certain conditions are met after the value has changed

🖥️ Steps to Reproduce

https://stackblitz.com/edit/1lmsiyxb

Try to increase the value. It will not change and throw an error in the console.

import { useState } from 'react';

export default function Example() {
  let [value, setValue] = useState(25);
  let [disabled, setDisabled] = useState(false);

  const onChange = (val) => {
    if (disabled) return;
    setValue(val);
    setDisabled(true);
  };

  return (
    <div>
      <NumberField
        label="Cookies to buy"
        value={value}
        onChange={onChange}
        isDisabled={disabled}
      />
      <p>Current value: {value}</p>
    </div>
  );
}

Version

1.17.0

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

MacOS

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response

[snowystinger]

It looks like there two issues, one with stale closures and one with event timing.

If you wrap your onChange in a useEffectEvent from React 19, that fixes the issue where the number gets stuck due to the stale closure. Unfortunately we can't move to it in our repo yet for some other reasons, and it wouldn't fix it for 18 and below.

Here's a unit test for this that reproduces locally for whomever looks at this more next.

  it.only('should update value and disable onChange', async () => {
    function Example() {
      let [value, setValue] = useState(25);
      let [disabled, setDisabled] = useState(false);

      const onChange = val => {
        if (disabled) return;
        setValue(val);
        setDisabled(true);
      };

      return (
        <div>
          <NumberField value={value} onChange={onChange} isDisabled={disabled}>
            <Label>Cookies to buy</Label>
            <Group>
              <Input />
              <Button slot="decrement">-</Button>
              <Button slot="increment">+</Button>
            </Group>
          </NumberField>
          <p>Current value: {value}</p>
        </div>
      );
    }
    let {getByRole} = render(<Example />);
    let input = getByRole('textbox');
    let incrementButton = getByRole('button', {name: 'Increase Cookies to buy'});
    await user.click(incrementButton);
    expect(input).toHaveValue('26');
    expect(input).toBeDisabled();
  });

The second issue, the flushSync during render is going to be a hard one to solve as well. When a component becomes disabled, if it had focus, then we fire the blur event. We could possibly try to schedule it immediately after render completes, though if render is interrupted, I'm not sure if it might still yield to a timeout. It's very difficult to know when rendering is "done".

[ce2kettu]

Alright, thanks for the input! Is there any workaround at the moment? Our current use case is something like: user adds a product to cart and while that API call is in progress the NumberField becomes disabled for that duration (or the API call results in an error).

[snowystinger]

Well, doing something like that isn't great for accessibility. I imagine focus is probably being lost to the body right now? Or at best being shuffled off somewhere else?

Someone using assistive technologies may not get the update that the server is doing something and may not know if the increment was applied successfully or not.

It would probably be better to make the field readonly and display a progress bar of some variety, either inside the field or next to it. Then you don't need to worry about focus leaving and this condition doesn't happen.

[ce2kettu]

With this code using the isReadOnly property instead of the disabled one, after increasing the quantity once, the input will get stuck in an endless loop of increasing values. The flushSync error, however, is resolved.

import { useState } from 'react';

export default function Example() {
  let [value, setValue] = useState(25);
  let [disabled, setDisabled] = useState(false);

  const onChange = (val) => {
    console.log(val);
    setValue(val);
    setDisabled(true);
  };

  return (
    <div>
      <NumberField
        label="Cookies to buy"
        value={value}
        onChange={onChange}
        isReadOnly={disabled}
      />
      <p>Current value: {value}</p>
    </div>
  );
}

[snowystinger]

Thanks for following up. I think the infinite spinning will be resolved by fix: SpinButton spins forever if the first change results in a disabled button in our next release. You can use the nightly version right now to verify that and work around it for now.