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: Click event using window.addEventListener is fired prematurely #24657

Closed
RobinMalfait opened this issue Jun 2, 2022 · 6 comments
Closed
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@RobinMalfait
Copy link

React version:

  • Works in React v17.1.0
  • Doesn't work in React v18.0.0 & v18.1.0

Steps To Reproduce

Link to code example:

The current behavior

If you open the React 18 CodeSandbox link from above, what you will notice when you hit the Open Dialog button is that you will receive an event like Got event in <Dialog />!

I tried to minimize the reproduction, the idea is that we have a Dialog component that receives an open prop, based on this prop we know to show the Dialog or not. However a Dialog should also have "outside click" behaviour, to fake this I setup a window.addEventListener('click') inside a useEffect to "capture" this behaviour. However when I click the Open Dialog button this is what happens:

  1. Click Open Dialog
  2. Set open state to true
  3. Re-render component
  4. Pass open={true} to the Dialog component
  5. In a useEffect, setup the window.addEventListener('click') if the open value is true
  6. The callback of the event listener receives the click event from step 1.

Console logs after clicking once on the Open Dialog button:

Open: false
*onClick*
Open: true
Got event in <Dialog />!

The expected behavior

If you open the React 17 CodeSandbox and repeat those steps from above, then you will see the expected behaviour where clicking on the Open Dialog does not trigger the log from within the window.addEventListener('click') it's only the second time that you click that button that it will log something because now the Dialog is considered in an open state.

Console logs after clicking once on the Open Dialog button:

Open: false
*onClick*
Open: true

Is this a bug, a regression or am I doing something incorrect here? I'm confused how a window.addEventListener can receive an event from a click that happened before the listener was even setup 🤔

@RobinMalfait RobinMalfait added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Jun 2, 2022
@PiNengShaoNian
Copy link

React version:

  • Works in React v17.1.0
  • Doesn't work in React v18.0.0 & v18.1.0

Steps To Reproduce

Link to code example:

The current behavior

If you open the React 18 CodeSandbox link from above, what you will notice when you hit the Open Dialog button is that you will receive an event like Got event in <Dialog />!

I tried to minimize the reproduction, the idea is that we have a Dialog component that receives an open prop, based on this prop we know to show the Dialog or not. However a Dialog should also have "outside click" behaviour, to fake this I setup a window.addEventListener('click') inside a useEffect to "capture" this behaviour. However when I click the Open Dialog button this is what happens:

  1. Click Open Dialog
  2. Set open state to true
  3. Re-render component
  4. Pass open={true} to the Dialog component
  5. In a useEffect, setup the window.addEventListener('click') if the open value is true
  6. The callback of the event listener receives the click event from step 1.

Console logs after clicking once on the Open Dialog button:

Open: false
*onClick*
Open: true
Got event in <Dialog />!

The expected behavior

If you open the React 17 CodeSandbox and repeat those steps from above, then you will see the expected behaviour where clicking on the Open Dialog does not trigger the log from within the window.addEventListener('click') it's only the second time that you click that button that it will log something because now the Dialog is considered in an open state.

Console logs after clicking once on the Open Dialog button:

Open: false
*onClick*
Open: true

Is this a bug, a regression or am I doing something incorrect here? I'm confused how a window.addEventListener can receive an event from a click that happened before the listener was even setup 🤔

React version:

  • Works in React v17.1.0
  • Doesn't work in React v18.0.0 & v18.1.0

Steps To Reproduce

Link to code example:

The current behavior

If you open the React 18 CodeSandbox link from above, what you will notice when you hit the Open Dialog button is that you will receive an event like Got event in <Dialog />!

I tried to minimize the reproduction, the idea is that we have a Dialog component that receives an open prop, based on this prop we know to show the Dialog or not. However a Dialog should also have "outside click" behaviour, to fake this I setup a window.addEventListener('click') inside a useEffect to "capture" this behaviour. However when I click the Open Dialog button this is what happens:

  1. Click Open Dialog
  2. Set open state to true
  3. Re-render component
  4. Pass open={true} to the Dialog component
  5. In a useEffect, setup the window.addEventListener('click') if the open value is true
  6. The callback of the event listener receives the click event from step 1.

Console logs after clicking once on the Open Dialog button:

Open: false
*onClick*
Open: true
Got event in <Dialog />!

The expected behavior

If you open the React 17 CodeSandbox and repeat those steps from above, then you will see the expected behaviour where clicking on the Open Dialog does not trigger the log from within the window.addEventListener('click') it's only the second time that you click that button that it will log something because now the Dialog is considered in an open state.

Console logs after clicking once on the Open Dialog button:

Open: false
*onClick*
Open: true

Is this a bug, a regression or am I doing something incorrect here? I'm confused how a window.addEventListener can receive an event from a click that happened before the listener was even setup 🤔

This looks like the expected behavior, synchronously registered event handlers are called at initialization, look at this example,
https://codesandbox.io/s/sweet-zeh-lbbqes?file=/src/index.js the reason it works in React17 is that the effect is dispatched using MessageChannel (similar to setTimeout), in your case to print the result as expected you just need to change window .addEventListener("click", handle); to setTimeout(() => { window.addEventListener("click", handle); }); and it works

@irinakk
Copy link
Contributor

irinakk commented Jun 8, 2022

This is related to #21150 in React 18, the effects resulted from onClick are flushed synchronously.

@gaearon
Copy link
Collaborator

gaearon commented Jun 8, 2022

The behavior in 17 was inconsistent. Here's an example using 17 but the event does get handled synchronously:

https://codesandbox.io/s/runtime-glitter-256mit?file=/src/App.js

The only difference is that I added rendering of a portal. This issue was originally described in #20074.

In 18, the behavior is consistent. Effects caused by a render from an intentional user event like clicking/typing are always flushed synchronously. So you'd need to tweak the logic of the modal but at least it will always work the same way.

One possible workaround is to delay the subscription with a zero timeout. Another possible workaround is to capture the current event during the effect and ignore that particular event in the handlers (like in microsoft/fluentui#18323).

@gaearon gaearon closed this as completed Jun 8, 2022
@RobinMalfait
Copy link
Author

Beautiful, thank you!

@s-h-a-d-o-w
Copy link

Maybe I'm missing something but wouldn't it be better to stop event propagation before we show the dialog? (Not my idea)

@orange4glace
Copy link

I'm curious why useLayoutEffect also works like useEffect in this situation as long as useLayoutEffect fires before painting.

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

6 participants