The focus timeout introduced in v5 makes the input difficult to test

[jraoult]

Describe the issue and the actual behavior

The timeout introduced in v5 to manage the caret position makes some of my (Cypress based) tests fail. In certain conditions, the timeout triggers after Cypress inputs value in the field, messing with the caret position. There is currently no reliable way to detect when the caret position is stable, so I used an arbitrary delay. It works most time but makes for flaky tests.

Describe the expected behavior

Ideally, we would have a way to know that the caret position is stable. A simple data attribute would work for me. I could then wait for the attribute to be set (or removed). Another option could be a JS callback triggered once the caret position is stable one can hook into and execute their behaviour.

Provide a CodeSandbox link illustrating the issue

This is an approximation since it is impossible to simulate a keyboard interaction in a non-privileged context, but it shows the issue. See the difference between the native input and NumericFormat.

https://stackblitz.com/edit/react-ts-y69m7x?file=App.tsx

Please check the browsers where the issue is seen

• Chrome
• Chrome (Android)
• Safari (OSX)
• Safari (iOS)
• Firefox
• Firefox (Android)

[jraoult]

@s-yadav have you managed to take a look at this? What are your thoughts?

[s-yadav]

Haven't got a chance to look into this. Will look into this over this weekend.

[jraoult]

Great @s-yadav ! If you decide to go ahead and need help; I'm happy to jump in.

[s-yadav]

Thanks @jraoult, will let you know.

[s-yadav]

@jraoult the setTimeout doesn't look like the culprit, as there is a check which matches the value before changing the caret position. So if Cypress updates the value the caret position will not change.

    timeout.current.setCaretTimeout = setTimeout(() => {
      if (el.value === currentValue) setCaretPosition(el, caretPos);
    }, 0);

However, I see in your example that you are setting the value using .value. Directly setting value property, doesn't inform react-number-format that the value has changed, and rnf might reset the value during trying to render formatted value, which would like caret position being messed up. Can you check if something similar is happening in cypress, if yes triggering input/change event on the input instead of setting value through .value should work.

Added console log to demonstrate onChange will not get triggered.
https://stackblitz.com/edit/react-ts-mhmhzg?file=App.tsx

[jraoult]

I see in your example that you are setting the value using .value

@s-yadav it was my broken attempt at simulating Cypress, but they simulate typing by triggering the Keyboard events in the field. What I need to do, then, is create a proper reproduction using actual Cypress to surface the issue.

I know the flaky workaround to the issue is waiting for a long enough amount of time before typing, so there is something going on with delay, caret and render, but it might not be the one I pointed out.

[jraoult]

@s-yadav First of all, sorry for the confusion. I mixed different issues. So forget my initial Stackblitz; it is all wrong since it is not demonstrating my actual problem.

So after digging, I remembered that the issue was not with the field's value but with an attempt to implement an auto-select behaviour:

 useEffect(() => {
      if (autoSelect) {
        // If we call select too early, the `NumericFormat`'s
        // `setPatchedCaretPosition` function will reset the selection. To avoid
        // this, we need to wait for 3 task cycles (empirically determined)
        // before calling it.
        //
        // cf. https://github.com/s-yadav/react-number-format/blob/3443f1adf3a904c19a9fca1183cf3af08a0bf714/src/number_format_base.tsx#L127-L133
        const timeout = setTimeout(() =>
          setTimeout(() => setTimeout(() => localInputRef.current?.select()))
        );

        return () => clearTimeout(timeout);
      }
    }, [autoSelect]);

The issue with Cypress is related: it can not reliably select the input content anymore (using the {selectAll} sequence), probably for the same reason.

I initially encountered this months ago (when I migrated to v5) and implemented a flaky fix for it, then moved on. When I opened this issue a few months later, I had already forgotten about all that and got the problems mixed up.

My suggestion, then, would be to have a way to know when the caret position is stable so I can run any "select-all" algorithm (either the React based one or the Cypress one).

Does it make sense?

[s-yadav]

Hey @jraoult, was occupied with other things since last three weeks, so couldn't look into this.
I understood, what you are saying, and I think I have a proper fix for this without a need of finding when the caret is stable.

Basically will check if the caret position is reset by the browser then only set again inside the time out.

    timeout.current.setCaretTimeout = setTimeout(() => {
      if (el.value === currentValue && el.selectionStart !== caretPos) setCaretPosition(el, caretPos);
    }, 0);

I will fix this. Thanks for your patience.

[s-yadav]

Actually it won't solve your problem, as the problem remains the same. the selectionStart will get changed by cypress {selectAll} sequence. Btw, cypress have delay prop on type method. Have you tried that out?

https://docs.cypress.io/api/commands/type

[jraoult]

cypress have delay prop on type method. Have you tried that out?

Yes, it's been my workaround for the tests, but an arbitrarily selected delay is unreliable. Unless I put an extremely long delay (which ends up slowing down the whole test suite), sometimes the browser scheduling intricacies trigger the timeout after the typing delay.

It is the same problem for the implementation of autoSelect in React. An empirically determined delay sometimes works, but I've seen scheduling issues.

I think a callback is a solution (something like onCaretStable. It can be leveraged in the parent components to toggle a data attribute for cypress to detect. For the autoSelect implementation, I could trigger the select() in the callback, ensuring it only happens once NumberFormat is done with "moving" with the caret.

[s-yadav]

Humm, I am trying to avoid adding an additional prop, but can't think of a better solution. I will give it another thought, otherwise, I will add a prop called onCaretPositionChange.

[s-yadav]

Hey @jraoult, so there are two problems.

1. rnf sets input value in two react render cycle, when values from prop has to be applied, basically during the first mount, or if input value has to be updated based on prop (not after type)
   While I was looking to make this into a single render cycle, looks like its a bigger refactor, and some corner cases are breaking.
   This double rendering is breaking the autoSelect feature. For that you can apply a hack/patch to auto select in next render. Something like this.

function useRnfAutoSelect(autoSelect) {
  const inputRef = useRef();
  const [_autoSelect, setAutoSelect] = useState();
  useEffect(() => {
    setAutoSelect(autoSelect);
  }, [autoSelect])

  useEffect(() => {
    if (_autoSelect) {
      inputRef.current?.select();
    }
  }, [_autoSelect])

  return inputRef;
}

Here's an example for the same. https://stackblitz.com/edit/react-ts-rq99uj?file=App.tsx

2. the {{selectAll}} sequence on test. For this I think putting delay would always work as cypress or any test util also put timeout, so they are always guranteed to happen one after other. Also you don't have to set this globally, only on type events where you have {{selectAll}} sequence.

The proper fix for this would take longer, as it may require some breaking changes.

[s-yadav]

Related PR: #750

[jraoult]

@s-yadav, thanks for your last suggestion, but sadly it doesn't work for me. The auto-select algorithm is implemented in one of the parents of NF, and the parent renders seem to not align with NF's renders. I'm back to square one with an arbitrary delay (for Cypress) or number of task cycles (for React).

[s-yadav]

But even if it the autoselect algo is in parent, you can still have a wrapper component on rnf and do this double effect thing in the wrapper component.
Basically in parent instead of doing autoSelect there if the input you are trying to select is rnf input (you can have some class to identify this), set a prop on parent, and forward this to the wrapper and autoselect here.

I know this is patchy solution, but should work, given that correct solution within rnf would take time to come.

[s-yadav]

@jraoult this is fixed on 5.2.0. Let me know if it doesn't work for you.