facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
228.59k stars 46.79k forks source link

Bug: useEffect and Event Handler Timing Regressions in React 18 Legacy Mode #31023

Open Dosant opened 1 month ago

Dosant commented 1 month ago

Summary

A change in useEffect and event handler timing causes regressions when upgrading to React 18 in legacy mode (React 18 in concurrent mode doesn't have the regression).

React version: 18.3.1 (affects versions since 18.0.0)

Steps to Reproduce

This example uses React 18 in legacy mode. The pattern involves a controlled input directly updating its value through the DOM, which seems to be breaking in React 18 when using legacy mode.

useEffect(() => {
    if (inputRef.current) {
      inputRef.current.value = value;
    }
  }, [value]);

Current Behavior

Expected Behavior

No breaking changes should occur that are specific to React 18 legacy mode. The behavior should be consistent between legacy mode and concurrent mode.


Investigation

I suspect the issue is related to the following change from the React 18 upgrade guide:

Other Breaking Changes: consistent useEffect timing: React now always synchronously flushes effect functions if the update was triggered during a discrete user input event such as a click or a keydown event. Previously, the behavior wasn’t always predictable or consistent.

There are no detailed examples I could find to fully understand this change, so I’m not entirely sure if this is the root cause. However, this broken example might indicate unintended behavior in legacy mode.

Context

We are in the process of upgrading a large React project with many independent ReactDOM.render calls to React 18. Our initial plan was to upgrade to React 18 and allow teams to transition to the new renderer independently. However, the upgrade has resulted in several end-to-end test failures, mostly due to this breaking change in the pattern shown in the example.

Two real-world components using this pattern that are affected:

Workaround

I temporarily resolved the issue by replacing useEffect with useLayoutEffect. This fixes the problem, but I am unsure if this is the best solution without requiring a significant refactor. I’m also uncertain if this workaround should be applied only in legacy mode while retaining the original useEffect for concurrent mode.

There is also a concern about other possible issues that didn't show up in our test that could have been caused by this change.

Ask for Assistance

Any assistance or guidance on this issue would be greatly appreciated as it is impacting our upgrade path for a large project.

darshancn commented 3 weeks ago

Bug: useEffect and Event Handler Timing Regressions in React 18 Legacy Mode #31023 Update: Issue Resolved

I have identified the root cause of the bug, which is related to the timing of the useEffect hook in React 18's legacy mode. By replacing useEffect with useLayoutEffect, the issue is resolved.

The useLayoutEffect hook ensures that DOM updates happen synchronously before the next browser paint, preventing the input value from being overwritten by previous renders, which was causing the skipped letters issue during fast typing.

After applying this change, the input component behaves as expected in both React 18 legacy and concurrent modes.

Thank you for the support and investigation!