-
Notifications
You must be signed in to change notification settings - Fork 41
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
Fix #268/#71: HMR support for web sockets #270
Conversation
a7f581d
to
d9ef025
Compare
Tested with both Vite and NextJS successfully. |
0fa9401
to
9129166
Compare
Any websocket you want ignored you can still use the ignored paths property to skip! |
5b8774a
to
1540b6e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that's very cool, I've just added a few comments.. I will give it a try later 👍
runtime/src/main/java/io/quarkiverse/quinoa/QuinoaDevProxyHandler.java
Outdated
Show resolved
Hide resolved
runtime/src/main/java/io/quarkiverse/quinoa/QuinoaDevProxyHandler.java
Outdated
Show resolved
Hide resolved
runtime/src/main/java/io/quarkiverse/quinoa/QuinoaDevProxyHandler.java
Outdated
Show resolved
Hide resolved
runtime/src/main/java/io/quarkiverse/quinoa/QuinoaDevProxyHandler.java
Outdated
Show resolved
Hide resolved
Very happy with this version now, more graceful and removes Vite hack |
I've test vite+svelte and get this error:
Here is the project: https://github.com/ia3andy/vite-svelte-quinoa |
Thanks for the reproducer I will investigate and let you know! |
Weird your reproducer is working fine for me and HMR is working great!
|
Its also strange yours called the websocket twice and mine only called it once correctly.
|
Also from looking at your threads it looks like the socket got called from thread 2 and 3 simultaneously from your browser? |
I am using chrome |
ah maybe I had two tabs open |
Yep try it fresh from incognito mode just to take all caching out of the equation and starts a fresh websocket. |
|
So weird that is not happening for me with your project and Svelte is hot reloading fine. I can make a change in a page and see it reflected immediately and see the traffic on the websocket. |
Any hint on how I could debug it to figure it out? |
So I have DEBUG messages in all the socket send and receive you can flip them to INFO to see any WebSocket traffic. |
Interesting your error Are you using a MacOS Chrome by any chance? |
yes! |
One commenter wrote:
|
Can you try Safari on MacOS? I have been using both Chrome and Firefox on Windows and everything works properly, |
It's working on Safari but the problem is in the Quinoa WS code as it works with Chrome if I add this:
|
Yeah but I think this line...
Bypasses our 8080 route and just goes right to 5173. Which is totally fine. But its just weird the passthrough socket doesn't work with that error on Chrome on Mac OS. |
Yes that's what I meant, it means the problem is not Chrome Mac or the Node server as it works when bypassing. |
OK what Node version are you using? |
With the reproducer project, it's the one specified in the application.properties |
oK yeah I tried 16.17 and 18.8.0 and I can't reproduce your issue . |
OK I pushed a fix not to forward websocket headers on to Vite. Can you try again? It also gets rid of that strange socket error |
It's still failing but I have been debugging all morning and I made a bit of progress.. I'll let you know.. |
Awesome. I appreciate it. I wish I could make it happen regularly so you didn't have to debug it! |
Ok I added a commit:
Most of the things where added to allow me to debug the problem but are good to keep. Thanks @tsegismont you saved my day ! |
@melloware now we are waiting for your review :) I've tested vite, react and angular, and it works like a charm! Also if ok, please squash it all to one commit. |
testing it now! |
OK tested NextJS and Vite on Windows and both looking good. |
I think this is safe to merge! |
Commits Squashed |
Thanks @melloware this is a very good contribution!!! |
I appreciate the assist this make this Extension even more awesome for people to adopt! |
@all-contributors please add @melloware for code, doc |
I've put up a pull request to add @melloware! 🎉 |
Fix #268 Netx JS support
Fix #71 Handle websockets in Dev Mode