-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
BREAKING CHANGE: switch default transportMode to ws #2531
Conversation
Codecov Report
@@ Coverage Diff @@
## v4 #2531 +/- ##
=======================================
Coverage 93.43% 93.43%
=======================================
Files 34 34
Lines 1309 1309
Branches 369 369
=======================================
Hits 1223 1223
Misses 83 83
Partials 3 3
Continue to review full report at Codecov.
|
@Loonride hm, CI broken |
/cc @Loonride friendly ping |
Yes it made sense to get rid of iframe/live mode first, since it relied on sockjs I believe |
decce77
to
24c50ce
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.
Great job!
BREAKING CHANGE: switch default transportMode to ws
@Loonride @hiroppy @alexander-akait I've recently upgraded my WDS to 4.x and I stated seeing issues with the websocket randomly dying when the server is running. Switching back to sockjs works, but I'm just curious what was the reason for changing the default to websockets? |
@patrickszmucer Are you using any 3rd party plugin like the |
I'm not - I have error overlays disabled and I wrote some custom code that listens to these events and changes the color of my banner to show when the server is ready/reloading/stopped. I noticed that the banner now occasionally flickers between stopped/ready because I'm getting "webscket close" events (with a 1006 status). |
Please create an issue with reproducible example. |
Will do - I just need to collect more details. I was just curious about the motivation for this change :) |
For Bugs and Features; did you add new tests?
No
Motivation / Use-Case
switching to ws as default
this may require getting rid of iframe to pass tests since I think iframe mode relies on sockjs
Should the warning that
transportMode
is an experimental option stay for the moment, or should we get rid of it?Breaking Changes
ws will be default instead of sockjs
Edit: also changed default sockPath from
sockjs-node
tows
Additional Info