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

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

Open
Dosant opened this issue Sep 23, 2024 · 1 comment
Open
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@Dosant
Copy link

Dosant commented Sep 23, 2024

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

  • React 17: Works correctly (baseline).
  • React 18 (legacy mode): Inputs break; letters are skipped.
  • React 18 (concurrent mode): Works correctly (same as React 17).

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

  • Could the team investigate if there is an underlying bug in React 18 legacy mode that needs to be addressed?
  • If not, could you please provide more details on this change and suggest guidance on fixing similar patterns or what to watch out for?

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

@Dosant Dosant added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Sep 23, 2024
@darshancn
Copy link

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

2 participants