OvidijusParsiunas / deep-chat

Fully customizable AI chatbot component for your website
https://deepchat.dev
MIT License
1.26k stars 170 forks source link

websocket mode doesn't work in shadow DOM #193

Closed mchill closed 1 month ago

mchill commented 1 month ago

I am writing a wrapper web component for Deep Chat. The "basic" REST handler works, but problems arise when I set websocket: true. If url is given, the connection is never initiated, and if handler if given, it is never called. The submit button isn't enabled when I type in the input field, I assume because that only gets enabled once the connection is established. I noticed this only happens when rendering deep-chat inside of the shadow DOM. If I don't create a shadow DOM, it works, but this isn't the recommended way to create custom components.

I created a minimal reproducible example that shows the difference in behavior when Deep Chat is used in the light and shadow DOM. If you set websocket to false, you'll see that the examples behave identically. https://stackblitz.com/edit/lit-nmk2pm

I've been trying to track down the issue, but I haven't had any luck so far. My best guess is that Deep Chat queries elements in the DOM to add an event listener somewhere, but I haven't been able to find evidence of that. Maybe you will have a better idea of what the problem could be.

As an aside, the reason I'm attempting to switch to the websocket handler is because I want to render multiple responses from a single HTTP response (the actual text response, and a source for the response also returned by the API). I came across this comment that recommends using the websocket handler to achieve this.

mchill commented 1 month ago

Of course as soon as I open the issue, I find it. The websocket connection never gets created because the code is checking for the existence of Deep Chat in the document body.

mchill commented 1 month ago

I'm not entirely sure why you make that check, but if all you want is to make sure Deep Chat is still attached to the DOM, I think this is a valid replacement:

// Current check
if (!document.body.contains(io.deepChat)) return;

// Proposed check
if (!(io.deepChat.getRootNode({composed: true}) instanceof Document)) return;

I'll go ahead and make a PR with that change.

OvidijusParsiunas commented 1 month ago

Hi @mchill.

Thankyou very much for doing the investigation and finding the fix for the problem! I have recently got a new job and have been struggling to balance it with my open source endeavours, hence I apologise for getting back so late.

The reason why I had included the document.body.contains(io.deepChat) code was because in MPA (Multi Page Apps) if Deep Chat was configured to connect to a websocket connection and the connection would fail, the component would attempt to automatically reconnect via the retryConnection method. The problem arises when the user attempts to navigate to another page as this method would keep trying to reconnect anyway. Hence, I added the check to make sure that the component is still rendered in the page if it is attempting to reconnect.

I have tested your solution and the intended functionality still works, hence if it also works for your case it is a fantastic solution. Thankyou very much for your efforts!

May I ask, why were you trying to render Deep Chat as a light dom component? What particular use-case does this help you accomplish? Thanks!

OvidijusParsiunas commented 1 month ago

I have published your change in deep-chat-dev and deep-chat-react-dev packages version 9.0.172. These packages behave just like the main packages except their names are different. They also contain a few property name changes, such as the request property is changed to connect and stream is moved to the connect object. All of the current/old properties will still keep working as usual except you will see warnings in the console for their deprecation and get a few TypeScript warnings which you can ignore.

My work should become a bit more relaxed next month, hence that is when I plan to release this fix to the main packages. In the meanwhile, please use the dev packages and look out for the next release. Thanks!

mchill commented 1 month ago

May I ask, why were you trying to render Deep Chat as a light dom component?

I wasn't, it's the other way around - it was broken when nested inside of another shadow DOM. I'm using the Lit framework, which by default wraps each component in a shadow DOM. I just tried rendering to the light DOM while debugging, which led me to the root cause of the bug.

Thanks for the quick reply! I'm already using the dev package for some other fixes, so that's no problem.

OvidijusParsiunas commented 1 month ago

Understood, thankyou for the help once again!