react-use-intercom icon indicating copy to clipboard operation
react-use-intercom copied to clipboard

feat: add isOpen flag to useIntercom() (re-open)

Open NicolasGn opened this issue 3 years ago • 0 comments

Hi ! I re-open this PR because it seems like my previous explanations were not clear enough.

The problem does not come from this lib side, but from the Intercom side. Let me be more precise: when you register a callback with Intercom('onShow') and Intercom('onHide'), there is absolutely no way to remove it . I mean, this is not implemented in the Intercom object.

So you're correct when you say it's perfectly possible to keep track of the opening status in a local state, attaching listeners in a useEffect hook. The problem is that as you can't remove the added listeners, they will continue to exist even after the unmounting of your component. It's not a big memory leak, but it is one, and it is not wanted for clean stuff.

The only clean solution is to track the opening state at a root level, in a component that will never be unmounted to avoid the memory leak. The cleanest way imo would be to have it in the IntercomContext.

You consider this feature to be out of the scope of this lib, but I totally disagree. The only other clean alternative (from our product side) would be to wrap the IntercomProvider in an other context prodiver simply adding this isOpen flag. This is something that you don't really want when it could be part of the original lib.

Your lib, your choice obviously. I hope this more detailed explanation of the problem will make you change your mind on this feature.

All the best 👋

NicolasGn avatar Aug 23 '22 13:08 NicolasGn