TextInput: Fix clearing issues w/ methods to circumvent the event count to force set a value from JS

[hannojg]

Problem

This discussion tries to find a solution for this issue, that highlights how clearing a text input can be broken in react native:

• TextInput desynchronizes with its native counterpart facebook/react-native#37722

The TextInput component uses an event count mechanism to keep the UI and JS state in sync. If JS is sending outdated state updates (such as a prop update after onChangeText) the event is dropped. Code references:

• https://github.com/facebook/react-native/blob/dbbb4069b9d2df42fd446231925fd03d51ab7293/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java#L650-L658
• https://github.com/facebook/react-native/blob/dbbb4069b9d2df42fd446231925fd03d51ab7293/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputViewManager.mm#L123-L136

We encountered a use case where this is problematic:

When clearing text: The user calls textInputRef.clear() in JS expecting it to get cleared. But the user might continues typing, and once the event from JS to set the TextInputs content to "" the event is dropped.

Question: Is that a problem that you’d be open to fix in react-native core?

Solutions

There are multiple solutions that come to my mind. I opened this as a discussion so feel free to add your own, or comment on mine. I am trying to find the solution that works best and would be merged into react-native.

1. Adding an explicit clear view command that bypasses the event count

Right now, when the user calls ref.clear() internally it will call setTextAndSelection(viewTag, "", 0, 0) which is subject to the event count mechanism.

We could create a separate view command for clearing that’s not suing the event count from JS, but just takes the native one and increments it.

I have an example implementation here:

https://github.com/margelo/expensify-app-fork/blob/fix/composer-not-clearing-force-clear-event/patches/react-native+0.73.4+018+textInputClear.patch

It’s important that the event count is incremented natively, so any other events that might happened in between get dropped (e.g. JS sending the value prop).

2. Adding a way to force call setTextAndSelection or adding a new public view command sich as forceSetTextAndSelection

The idea is similar to the one above, we could either add a new public view command called forceSetTextAndSelection or add a force: boolean parameter to setTextAndSelection and expose it to the user.

As explained in solution (1) we’d bypass and increment the event count on the native side.

This solution would also help in serving use case (2).

3. Using partial updates from native in JS

originally explained here

In react native the text input sends the whole text that's currently in the text input in the onChange/onChangeText event callbacks. However, we maintain the "source of truth" for our text input in JavaScript in our react state variable value which we pass as a prop to the <TextInput value={value} />.

So it can happen that we set our JS value to "" to clear the input, but the native side is ignoring that event (as the JS thread is lagging behind in events), and sends us instead a new update with the current native input, which would be "aaaaaabbbbbb" by then.

Instead of fully setting the whole native text we receive, we can only apply the updates that happened to the text to our JS value.

Meaning if the input was "aaaaaa" and the user types "b", instead of taking the native text update and setting "aaaaaab" as our value, we just take the change "b" and append it to our value:

• value is "aaaaaa"
• User presses clear button, setting value to ""
• User types "b"
• Native side sends "aaaaaab"
• JS just takes the partial diff value "b" and appends it to our value, resulting in "" + "b" = "b"
• Native side is updated with the new value prop, showing "b"

So the user would see "aaaaaa" -> "aaaaaab" -> "b".

I have a PR for that here, as we need to receive the replacement range from the native side in JS to implement this:

• feat: include information about text range changes in onChange facebook/react-native#45248

4. Keeping it all on one thread

One solution would be to work with the text input only on the JS or UI thread. I don’t think thats a a real solution, as we want to keep the JS and UI thread independent from each other / none-blocking each other.

5. Force re-render TextInput

It could be fixed from the user side, without any changes to react-native, by force re-rendering the TextInput on clear. I think this is best explained in code:

const MyTextInputWrapper = React.forwardRef((props, ref) => {
	const [rerenderKey, setRerenderKey] = useState();
	const [value, setValue] = useState(props.value)
	
	useImperativeHandle(() => ({
	  clear: () => {
	    setValue("")
	    setRerenderKey(Date.now())
	  }
	}), [ref])

	return <TextInput key={rerenderKey} {...props} onChangeText={value} value={value} />
})

However, i am not sure if such a user workaround is a good idea? I know that fabric recycles native views, but i still feel that this would be a bit overkill?

Any other ideas? Personally, I am leaning towards solution (1).

[hannojg]

cc @cipolleschi tagged you directly as I remember you are very familar with the textInput component and you helped us a lot previously with the numberOfLines prop PR 🙌

[cipolleschi]

Hi, thanks for the ping and for writing such a detailed explanation.

I'm not actually that expert of the TextInput component, but I can surely forward the proposal to people with more knolwedge.

The first question, though, is this issue happening also in the New Architecture?

If it is fixed there, we are not going to move forward as our plan is to aggressively start pushing the ecosystem toward that architecture, so major refactoring of Old Architecture parts would be time sinks for the team with low value for React Native.

If the problem persists in the New Architecture, we want to have it fixed and we can iterate on this!

[hannojg]

Thanks for getting back to me! The problem also exists on the new arch yes, I explicitly tested it on new arch.