feat(hot): serve the events over a WebSocket too - #2420
Conversation
`hot.transport` chooses between Server-Sent Events and a WebSocket. Both answer the same calls, so nothing downstream of `createHot` knows which one it publishes to, and the SSE path is unchanged. A WebSocket handshake is an upgrade the HTTP server answers, which the middleware never sees, so the server is handed over through `hot.server` or the middleware's new `attach` method. `ws` is an optional dependency, needed only by this transport.
🦋 Changeset detectedLatest commit: fc45c77 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. WalkthroughHot module replacement now supports WebSocket and custom transports. Server-Sent Events remains the default. The WebSocket stream handles upgrades, clients, heartbeats, publishing, and closure. Hot instances and the middleware API support server attachment. New clients receive cached build statistics when available. Schemas, public types, documentation, dependencies, and tests cover the transports. Merge Risk: ⚪ Minimal · up to No actionable merge-blocking risk is established by the supplied evidence. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 5fdd5dd2-fa3e-4670-a750-7f8a903abeb7
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
.changeset/hot-transport-ws.mdpackage.jsonsrc/hot.jssrc/index.jssrc/options.check.jssrc/options.jsonsrc/servers/WebSocketServer.jstest/__snapshots__/validation-options.test.js.snap.webpack5test/hot.test.jstypes/hot.d.tstypes/index.d.tstypes/servers/WebSocketServer.d.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2420 +/- ##
==========================================
- Coverage 97.11% 96.24% -0.88%
==========================================
Files 13 14 +1
Lines 1664 1783 +119
==========================================
+ Hits 1616 1716 +100
- Misses 48 67 +19 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
`hot.transport` took one of two names, so the only way to carry the events over anything else was to fork the middleware. It now also takes a function, which is handed the resolved path and heartbeat and returns the same client stream the built-in two are. A returned object missing one of those calls throws, naming what is absent, rather than failing later from wherever it is first published to.
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: f13f8813-d93f-41d4-be04-00c166e7f254
📒 Files selected for processing (9)
.changeset/hot-transport-ws.mdREADME.mdsrc/hot.jssrc/options.check.jssrc/options.jsontest/__snapshots__/validation-options.test.js.snap.webpack5test/hot.test.jstest/validation-options.test.jstypes/hot.d.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/hot-transport-ws.md
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
| } else if (transport === "ws") { | ||
| eventStream = createWebSocketStream({ heartbeat, path }, logger); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '475,535p' src/hot.js
sed -n '180,205p' client-src/index.js
sed -n '550,630p' client-src/index.jsRepository: webpack/webpack-dev-middleware
Length of output: 5491
Add WebSocket support to the bundled client.
When hot.transport is "ws", src/hot.js creates a WebSocket stream. The bundled client always creates an EventSource, and its bootstrap only calls connect() when window.EventSource exists. It has no WebSocket transport selection or fallback path. Therefore, the ordinary bundled-client hot-update workflow does not work with "ws".
Expose the selected transport to the client and add a WebSocket connection path while keeping SSE as the default.
…e its client
`StreamClient` referred to `import("ws")`, and `types/index.d.ts` reaches it,
so every consumer loaded the optional dependency's declarations — including
one on Server-Sent Events, whose build failed outright with TS2307 under the
default `skipLibCheck: false`. It is a structural type now, and a test fails
if the import comes back.
`publishTo` also took only that union, so under `strictFunctionTypes` a
transport of your own could not be written in TypeScript at all: narrowing
the parameter to its own client type was rejected. `ClientStream` and
`ClientStreamFactory` are parameterized by the client type instead.
Summary
hot.transportchooses how hot module replacement events reach the clients:"sse"(the default, unchanged) or"ws".This is the first step of moving the HMR clients into webpack-dev-middleware so webpack-dev-server can reuse them instead of carrying its own. It is the server half of the WebSocket transport only — the browser client still speaks Server-Sent Events, so
transport: "ws"is not useful on its own yet. Splitting it this way keeps each piece reviewable; the client steps follow in their own PRs.How it fits together
Both transports satisfy one
ClientStreaminterface, socreateHotpublishes without knowing which one it is talking to and the SSE path is untouched:Two things worth calling out, because they are the parts that are not mechanical:
attachis new API, and it has to be. A WebSocket handshake is an upgrade the HTTP server answers; the middleware only ever sees(req, res, next)and never gets the chance. So the server is handed over, either ashot.serverwhen it already exists, or through the middleware's newattach(server)when it is built later (which is webpack-dev-server's case). A plainGETon the path undertransport: "ws"answers426 Upgrade Requiredrather than being left hanging on a stream it cannot read.onConnectis what let the two share a code path. Catching a newly connected client up — thesyncevents carrying the last hashes, without which it can never apply the next update — used to be written inline inhandle()and so only worked for Server-Sent Events. It is now a stream callback that both transports invoke once a client has joined, so a WebSocket client is caught up the same way. There is a test for it.src/servers/WebSocketServer.jsis ported from webpack-dev-server'slib/servers/WebsocketServer.js(noServer: trueplus an upgrade listener), with ping/pong reaping half-open sockets — one that never emitsclose, so nothing else would drop it — and a heartbeat that runs only while clients are connected, matching what the SSE stream already does.wsis an optional dependency, required lazily, so Server-Sent Events users do not pull it. Asking fortransport: "ws"without it throws an error naming the install.What kind of change does this PR introduce?
feat
Did you add tests for your changes?
Yes —
test/hot.test.jsgains five cases over a real HTTP server and a realwsclient: publishing a build to a connected client, catching a late joiner up withsync, answering a plain request with 426, refusing upgrades once closed, and the log line naming the transport.Each was checked against a deliberately broken
attach: the two connection cases fail and the rest still pass, so they are not passing by accident. Doing that also turned up two defects in the tests themselves, both fixed here — the connect helper waited onopenforever instead of timing out, and a failing assertion leaked the HTTP server, which is now torn down inafterEachregardless of outcome.Unrelated to this change but worth flagging:
test/logging.test.jsfails 74/74 on a cleanmainwith none of these changes applied, so it is red before this PR and red after it.Does this PR introduce a breaking change?
No.
transportdefaults to"sse"and the Server-Sent Events path is unchanged. The one user-visible difference is that the startup log line now names the transport —Hot module replacement enabled, serving events at "/__webpack_hmr" over Server-Sent Events— since the two are configured alike but fail in different places.If relevant, what needs to be documented once your changes are merged or what have you already documented?
hot.transport,hot.serverand the middleware'sattachmethod, including thatwsmust be installed for the WebSocket transport.Use of AI
AI-assisted (Claude Code). It was used to write the transport, the tests and the schema changes, and to verify them: every test here was run against a broken implementation to confirm it fails, and the pre-existing
logging.test.jsfailure was confirmed by running that suite on a clean checkout. All output was reviewed before committing.🤖 Generated with Claude Code
https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
Generated by Claude Code
Summary by CodeRabbit
attachmethod to connect WebSocket upgrades to an HTTP server when no server is configured directly.