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 happens synchronously when useLayoutEffect was called #21400

Closed
just-boris opened this issue Apr 30, 2021 · 1 comment
Closed
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug Type: Question

Comments

@just-boris
Copy link
Contributor

React version: 17.0.2 (also tested on 16.9.0, same result)

Steps To Reproduce

  1. Open this codesandbox: https://0gobm.csb.app/ (Source)
  2. Try to click on the "Works" button
  3. Dropdown opens
  4. Try to click on the "Broken" button
  5. Dropdown does not open

Link to code example:

https://0gobm.csb.app/

The issue is related to how useEffect calls are scheduled. We have this effect:

  useEffect(() => {
    if (!open) {
      return;
    }
    const clickListener = (event) => {
      if (!dropdownRef.current.contains(event.target)) {
        onDropdownClose();
      }
    };

    window.addEventListener("click", clickListener);

    return () => {
      window.removeEventListener("click", clickListener);
    };
  }, [open, onDropdownClose]);

Normally, this effect happens asynchronously. However, sometimes, it is synchronous and we add listener to the window before the event fully bubbles and the dropdown gets closed by the same event.

This only happens if any underlying component contains useLayoutEffect with setState call (as shown on codesandbox). Looks like this causes forced render and this chain of events.

P.S. The report looks very similar to another one: #20074. However, I decided to open a new issue, because the circumstances are different (no portals in this case)

@just-boris just-boris added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Apr 30, 2021
@bvaughn
Copy link
Contributor

bvaughn commented Apr 30, 2021

useEffect is only synchronously run when an update is scheduled from useLayoutEffects. Updates from within layout effects must be applied synchronously (before paint) as that's the only point where users can e.g. tweak tooltip positions, dialog sizes, etc without the user seeing a flash of invalid content. In order to preserve the overall contract about when effects will run, React must also synchronously flush pending passive effects before handling the update.

Unfortunately this is expected behavior, but it also has a simple workaround: listen to the event during the capture phase.

window.addEventListener("click", clickListener, true);

Alternately you could use a setTimeout to debounce when the "click" event is attached:

useEffect(() => {
  // Delay until after the current call stack is empty,
  // in case this effect is being run while an event is currently bubbling.
  // In that case, we don't want to listen to the pre-existing event.
  let timeoutID = setTimeout(() => {
    timeoutID = null;

    window.addEventListener("click", clickListener);
  }, 0);

  return () => {
    if (timeoutID !== null) {
      clearTimeout(timeoutID);
    }

    window.removeEventListener("click", clickListener);
  };
}, []);

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 Type: Question
Projects
None yet
Development

No branches or pull requests

3 participants
@bvaughn @just-boris and others