livechat / chat-widget-adapters

This project contains a set of libraries for adapting LiveChat Chat Widget with certain frontend frameworks
https://developers.livechat.com
MIT License
24 stars 8 forks source link

Type error if the Widget is creating-destroying quick enough #51

Closed andrnag closed 2 years ago

andrnag commented 2 years ago

Describe the bug In our setup, the Widget is inside the router component. So it is destroyed and created each time the User changes the page. If one does it quickly enough we can catch this error.

To Reproduce Steps to reproduce the behavior:

  1. Put the widget inside the router component
  2. Change routes quickly
  3. Get the error

Expected behavior Widget is checking if window.LiveChatWidget is present

Screenshots The top of the callstack:

instrument.js:107 TypeError: Cannot read properties of undefined (reading 'call') at Object.destroy (widget-core.esm.js:321:29) at LN.i.useEffect.n (widget-react.esm.js:292:120)

Desktop (please complete the following information):

Additional context I think this is a bug in the Widget because here https://github.com/livechat/chat-widget-adapters/blob/master/packages/widget-react/src/LiveChatWidget.tsx#L11 it creates an API but here https://github.com/livechat/chat-widget-adapters/blob/master/packages/widget-core/src/create-widget.ts#L73 it is not checking a global object is present. If we repeat those functions quickly enough 'widgetRef.current' could be present but 'window.LiveChatWidget' still no.

walaszczykm commented 2 years ago

Hello @andrnag πŸ‘‹ Thanks for the report. Sad to hear you have encountered an issue.

I have investigated it a little bit; thanks for pointing it out in the code btw πŸ™‚

We cannot just guard the access of window.LiveChatWidget global object access as it, of course, made the error go away but also the required action of destroying the current widget instance would not happen leading to uncontrolled behavior or even leakage of global variables and not cleaned up properly script tags or other HTML elements on the website.

What looks like a proper solution here is to prevent successive calls of init and destroy while the widget is in a loading state, as we cannot properly destroy it until it is fully loaded. Preserving the last desired state from the user's perspective would allow deciding after initialization if the widget should be present or not.

I have implemented a draft of this solution, testing it with react-router and quickly repeated route changes as also as the consequent LiveChatWidget component mounting and unmounting on interval.

In both cases everything worked very stable, without any error, and was not leaking any leftover HTML elements or global variables.

Here you can find a Draft PR with the changes: https://github.com/livechat/chat-widget-adapters/pull/53

Please let me know if it would be possible for you to test the locally built package from the branch and test it in your setup to confirm 100% that the issue is gone. I will support you at any moment if help is required πŸ™‚

andrnag commented 2 years ago

Thank you, Maciej! I'll try to test it next week.

MonstraG commented 2 years ago

Also hit this problem, any progress?

Tested the branch out, problem doesn't occur there.

andrnag commented 2 years ago

Hello! Unfortunately, I had no time to test this solution. I will try to get it this week. I'll post the results here.

MonstraG commented 2 years ago

@walaszczykm , can you release it as alpha/canary at least?

walaszczykm commented 2 years ago

@MonstraG, I will finalize the PR, and we will proceed with the regular release. I just wanted to confirm that it properly solves the issue you have encountered πŸ™‚ Thanks for the confirmation πŸ™Œ

andrnag commented 2 years ago

@walaszczykm thank you! The fix is working. I have tested it today.

walaszczykm commented 2 years ago

Hello @andrnag and @MonstraG πŸ‘‹

The related PR has been merged, so the issue is closed automatically. The implemented fix is released on npm under version 1.2.1 πŸ™‚

Once again, thanks for help with this issue πŸ™Œ