Pomax / react-onclickoutside

An onClickOutside wrapper for React components
MIT License
1.83k stars 187 forks source link

Compatibility Issues Between "React" and "React-OnClickOutside" Libraries. #395

Closed OrMeirchak closed 6 months ago

OrMeirchak commented 1 year ago

Our project currently has 'react-onclickoutside': '^6.7.0' installed. We recently updated our project from 'react': '^16.2.0' to 'react': '^18.2.0'. However, we have encountered an issue with a component that is wrapped in onClickOutside. After upgrading to the new version of React, the handleClickOutside function is called immediately after the component is created, without the user performing any action. When we rolled back to 'react': '^16.2.0', the problem was resolved.

We would like to know if there is a version of 'react-onclickoutside' that is supported by 'react': '^18.2.0'?

Pomax commented 1 year ago

I assume you kept the class notation rather than rewriting things to functions with hooks, like the React folks have been pushing for since 16.5?

OrMeirchak commented 1 year ago

Hi Pomax,

Thank you for your reply.

I am working on a large project that has been in development for many years. As a result, a significant portion of our components are still written using class notation.

Pomax commented 1 year ago

Thanks, just wanted to make sure. I don't suppose you have a small self-contained example that can be dropped into the issue for testing?

OrMeirchak commented 1 year ago

Hi pomax , thank you very much for your willingness to help.

Unfortunately, I was not able to create an example that I could share. After a long time of debugging, I noticed that during the creation of the wrapped component, it is registered to the event in this line: document.addEventListener(eventName, handlersMap[this._uid], getEventHandlerOptions(this, eventName)); And immediately after that, the handlersMap function is activated. (The wrapped component is created by clicking on the screen that creates the event)

This is a behavior that also occurs in the old version of React, but for some reason, the event does not happen for the component that was just registered.

Thanks,or

Pomax commented 1 year ago

Remember that an example doesn't need to be "your code". If you're seeing it do the wrong thing, it should be possible to create a standalone "new code" setup with a few dummy components that shows things going wrong. (because if you can't, there aren't really any steps to reproduce the problem, which means we can't make a test case)

OrMeirchak commented 1 year ago

Hi Pomax, I've found a solution to the problem. In the constructor of the wrapped component, I save an initTimeStamp. Then, every time before I execute the logic of the handleClickOutside function in the wrapped component, I check if the timeStamp of the event is greater than the initTimeStamp. Do you now understand better what is causing the problem, or would you like me to try to create a pull request?"

OrMeirchak commented 1 year ago

Anyway, I created a pr

Pomax commented 1 year ago

hm, I don't see a PR in the open PR list?

OrMeirchak commented 1 year ago

hm, I don't see a PR in the open PR list?

https://github.com/Pomax/react-onclickoutside/pull/396

akmittal commented 6 months ago

Im also facing the same issue. Was working fine for React 16.x, migration to 18.2 causes the issue. Im using Class components.

Pomax commented 6 months ago

I'm going to have to point at https://react.dev/reference/react/Component at this point though.

Class components are still supported by React, but we don’t recommend using them in new code.

React went all in on functional components and hooks, whether we like it or not (I sure don't, the amount of horrible code that hooks gave rise to is truly staggering). There is just no point in trying to fight that, and functional components don't need this HoC anymore.

And I'd be very surprised if they don't flat out remove the Component class in React 19 or 20.

Pomax commented 6 months ago

published v6.13.1, but I will reiterate here that if a project relied on this HoC enough for this PR to still be merged in: please read the first paragraph of the README.md and see what you can do.