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

TextInput desynchronizes with its native counterpart #37722

Open
cubuspl42 opened this issue Jun 6, 2023 · 4 comments
Open

TextInput desynchronizes with its native counterpart #37722

cubuspl42 opened this issue Jun 6, 2023 · 4 comments
Labels
Component: TextInput Related to the TextInput component. Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Needs: Triage 🔍 Newer Patch Available

Comments

@cubuspl42
Copy link
Contributor

Description

React TextInput component can go "out of sync" with the native text input widget. The desynchronization can be triggered by quick user interactions. The problem becomes severe when the efficiency of the application code is not perfect; in this case, the user's interactions don't have to be quick to trigger the desynchronization.

The React TextInput component eventually re-synchronizes with its native counterpart, but the user experience and application behavior are affected.

See "Steps to reproduce" for the example and details.

React Native Version

0.71.7

Output of npx react-native info

System:
OS: macOS 13.3
CPU: (12) arm64 Apple M2 Max
Memory: 13.88 GB / 64.00 GB
Shell: 5.9 - /bin/zsh
Binaries:
Node: 16.15.1 - ~/.nvm/versions/node/v16.15.1/bin/node
Yarn: 1.22.19 - ~/.nvm/versions/node/v16.15.1/bin/yarn
npm: 8.11.0 - ~/.nvm/versions/node/v16.15.1/bin/npm
Watchman: Not Found
Managers:
CocoaPods: 1.12.0 - /opt/homebrew/bin/pod
SDKs:
iOS SDK:
Platforms: DriverKit 22.2, iOS 16.2, macOS 13.1, tvOS 16.1, watchOS 9.1
Android SDK: Not Found
IDEs:
Android Studio: 2022.2 AI-222.4459.24.2221.9787799
Xcode: 14.2/14C18 - /usr/bin/xcodebuild
Languages:
Java: 18.0.2 - /usr/bin/javac
npmPackages:
@react-native-community/cli: Not Found
react: 18.2.0 => 18.2.0
react-native: 0.71.7 => 0.71.7
react-native-macos: Not Found
npmGlobalPackages:
react-native: Not Found

Steps to reproduce

Let's consider the example app, which consists of a TextInput component and a "Clear input" button, which clears the text input. A real-life case would be a chat app with a "Send" button.

This is a Git repository with the MCVE.

  1. Type something into the text input, for example, "abcd"
  2. Press "Clear input", but immediately after that type some letter, let's say "e" (it's easier on an emulator, but possible on a physical device too, especially if it has an older CPU)
  3. Observe the text input content
    • Expected content: "e"
    • Actual content (if the problem is reproduced): "abcde"

Screen recording:

TextInput-sync-1.mp4

If the performance of the application JavaScript code is not-so-good, at step 2. you don't have to type the next letter immediately. If the task in which the TextInput is cleared takes 200 ms, the window to trigger the unexpected behavior will have 200 ms.

You can experiment with the performance implications in the provided MCVE by enabling the "Slow down" switch. It will artificially slow down the render by adding an extra computation.

Screen recording with non-ideal performance:

TextInput-sync-2.mp4

Snack, code example, screenshot, or link to a repository

https://github.com/cubuspl42/react-native-TextInput-sync-issue-mcve-1

@github-actions
Copy link

github-actions bot commented Jun 6, 2023

⚠️ Newer Version of React Native is Available!
ℹ️ You are on a supported minor version, but it looks like there's a newer patch available - 0.71.8. Please upgrade to the highest patch for your minor or latest and verify if the issue persists (alternatively, create a new project and repro the issue in it). If it does not repro, please let us know so we can close out this issue. This helps us ensure we are looking at issues that still exist in the most recent releases.

@cubuspl42
Copy link
Contributor Author

Also reproducible on 0.71.8. I pushed the new revision of the MCVE with all dependencies upgraded to their newest versions.

@hannojg
Copy link
Contributor

hannojg commented Jul 2, 2024

I've worked on this issue and found multiple possible solutions, but only one that really seems to make sense.

🔎 The problem

Imagine the following flow:

  • Spam the 'a' key, typing 'aaaaaa...'
  • Press a button that clears the input (ie. setting value to "", or calling .clear() [both do the same under the hood])
  • Spam the 'b' key, typing 'bbbbbb...'
  • You'd expect that the input first shows 'aaaaaa', then clears it, then it shows 'bbbbbb'. This works in most cases, but there is a race condition as this bug reports:

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.

Solution 🅰: make .clear() a separate view command that bypasses the event count mechanism

When calling textInputRef.current.clear() we currently issue a view command called setTextAndSelection("", 0, 0, mostRecentEventCount), which is basically the same as apssing the prop value={""}, as this would also call the setTextAndSelection view command.

We could add a new view command clear() that bypasses the event count mechanism and just clears the native text input.

This would basically mean that we will never ignore an explicit clear event.

⚠️ Problems

Lets assume the flow from above, here is what the user would see:

  • "aaaaaa"
  • (press on clear button)
  • "aaaaaabb"
  • (native side receives the clear command)
  • "bbb" - Oops, the first two "bb"s were removed, as they happened before the event was received on the native side. The user would've expected to see "aaaaaa" -> "" -> "bbbbbb".

Additionally the text input wouldn't be cleared immediately when calling clear(), but only when the native side receives the command.

Solution 🅱: Control from a single thread / have one source of truth

Currently when using the value prop, there are two sources of truth:

  1. The text in the native text input
  2. The value state in the JS thread

One could think of a solution where the input is completely controlled by the value from JS. Meaning when a change happens, the change is first dispatched to JS, and only if the value property updates the update is reflected in the native view.

⚠️ Problems

The updating of the native text input would be bound to the performance of the JS thread. Lets say its very busy and only "performs at 10 FPS", then our UI updates would appear lagging to the user.

Solution 🅲: Maintain an event order by using timestamps

When we update the state of the textInput component (using the value prop) we attach a timestamp of the event.

Internally an order/stream of events is maintained from which the current displayed input text is constructed.

  • Text input is "aaaaaa"
  • User presses clear button, sending event { type: 'clear', timestamp: 1 }
  • User types "b", the native side stores the event with { type: 'text', text: 'b', timestamp: 2 }, now text input is "aaaaaab"
  • User types "b", the native side stores the event with { type: 'text', text: 'b', timestamp: 3 }, now text input is "aaaaaabb"
  • The native side receives the clear event, and puts it into the correct order:
    • { type: 'clear', timestamp: 1 }
    • { type: 'text', text: 'b', timestamp: 2 }
    • { type: 'text', text: 'b', timestamp: 3 }
  • The native input text shows "bb"
    So the user would see "aaaaaa" -> "aaaaaabb" -> "bb".

The good part is that opposed to solution 🅰 we wouldn't drop any text.

⚠️ Problems

Implementing an event order system is not trivial and could lead to a lot of complexity.

Solution 🅳: Only apply partial updates from the native side to our JS value

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".

⚠️ Problems

For a brief moment the user might still see an unexpected input text "aaaaaab" before it gets updated to "b".
However, we didn't drop any text and its quite easy to implement such a solution.

Where to go from here

The overarching problem is that we have two threads that communicate asynchronously with each other.
While the JS thread dispatched a clear event and we're waiting on the native side to receive it, the user might have already typed new text on the UI thread.

Given this architecture I feel like solution 🅳 is the most feasible one, as it doesn't require a complete rewrite of the text input component and is quite easy to implement.

We only need to add information about the range in which the native text was changed. I created a PR here, which does that:

I applied the changes from the PR to the reproducer of this issue:

Before ✨ After ✨
BEFORE.webm
FIXED.webm

@hannojg
Copy link
Contributor

hannojg commented Jul 18, 2024

I started a discussion here to discuss with meta how to fix this:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: TextInput Related to the TextInput component. Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Needs: Triage 🔍 Newer Patch Available
Projects
None yet
Development

No branches or pull requests

3 participants