fix: client, overlay, progress and server lifecycle defects - #5739
Conversation
Consolidates the actionable fixes from #5724, #5725, #5726, #5727, #5728, #5729, #5730 and #5732. Client: - honor `client.progress: "linear" | "circular"`; the resource query only recognized `"true"`, so both visual modes were silently disabled - parse the resource query with full `key=value` semantics (encoded keys, `+` as space, `=` inside values, malformed escapes ignored) - decode credentials taken from the current script tag so `formatURL` does not encode them twice - apply `client.overlay.warnings` / `.errors` filter functions to what the overlay renders, not only to the decision to render - apply the reconnect limit before the first connection attempt, so `client.reconnect: false` no longer retries when the socket never opens Overlay: - reuse the Trusted Types policy instead of re-creating it per open, which throws under a `trusted-types` CSP - keep only the newest queued render so messages are not duplicated when two batches arrive before the iframe loads - re-register the Escape handler on open; it was removed on first dismiss and never restored - encode the `open-editor` file name, render openable entries as buttons, and restore focus on dismiss Progress: - style the linear bar through `#progress`; the rules targeted `#bar`, which no template emits - clear the `disappear` class and the pending hide timer when a new build starts, so the indicator reappears - skip redundant `attributeChangedCallback` work and expose progressbar ARIA state and reduced-motion styles Server: - reject from `start()` on an occupied port or IPC path instead of throwing from an event handler, and release what setup allocated - fix `bonjour` protocol reporting (`||` bound tighter than the ternary) - only install the WebSocket `upgrade` listener in no-server mode, and remove it on close - skip incomplete interfaces and CIDRs in `findIp`, and hand the listening socket an unbracketed IPv6 address - wait for pending startup before shutting down in plugin mode - build the asset report from `toJson` with only the fields it prints, construct the `serve-index` middleware once, and serialize each broadcast once instead of per client - export `BaseServer` and type `webSocketServer.type` as its constructor Examples: - repair `api/plugin` (CommonJS in an ESM package), `ipc` (`http-proxy`), `proxy` and `general/proxy-simple` (options removed in v5) - serve the shared layout assets through `express.static`, which also works for the `hono` example, and read each README relative to its own directory - restore host and cross-origin checks in the `hono` example, whose `setupMiddlewares` replaces the built-in stack Co-authored-by: Oskar Eichler <62393985+OskarEichler@users.noreply.github.com>
🦋 Changeset detectedLatest commit: 9191d4a 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 |
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. WalkthroughThis patch updates client progress, overlay, query parsing, socket credentials, and reconnect behavior. It updates server startup cleanup, networking, asset reporting, WebSocket handling, and plugin lifecycle behavior. It exposes Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to No concrete merge-blocking risk remains in the supplied review evidence. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 26 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 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: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 82a922f5-a745-4746-a23e-4df03d507e52
📒 Files selected for processing (40)
.changeset/fix-client-and-server-defects.mdclient-src/index.jsclient-src/overlay.jsclient-src/progress.jsclient-src/socket.jsexamples/.assets/layout.htmlexamples/.assets/style.cssexamples/api/middleware/README.mdexamples/api/middleware/webpack.config.jsexamples/api/plugin/README.mdexamples/api/plugin/webpack.config.jsexamples/app/connect/README.mdexamples/app/hono/README.mdexamples/app/hono/webpack.config.jsexamples/compression/false/README.mdexamples/default-cjs/webpack.config.cjsexamples/dev-middleware/webpack.config.jsexamples/general/proxy-simple/webpack.config.jsexamples/general/universal-config/client.jsexamples/headers/array/README.mdexamples/ipc/webpack.config.jsexamples/proxy/README.mdexamples/proxy/webpack.config.jsexamples/server/http2/README.mdexamples/util.jslib/Server.jslib/servers/WebsocketServer.jspackage.jsonscripts/finalize-cjs-build.mjstest/client/index.test.jstest/client/overlay-lifecycle.test.jstest/client/socket-helper.test.jstest/client/utils/createSocketURL.test.jstest/e2e/__snapshots__/api.test.js.snap.webpack5test/e2e/__snapshots__/overlay.test.js.snap.webpack5test/e2e/api.test.jstest/e2e/built-in-routes.test.jstest/e2e/host.test.jstest/server/find-ip.test.jstypes/lib/Server.d.ts
💤 Files with no reviewable changes (1)
- test/e2e/snapshots/api.test.js.snap.webpack5
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
- the example's cross-origin middleware must return early for a valid host, matching the built-in one; it was tagging every response with `Cross-Origin-Resource-Policy: same-origin` - bump `minor`: exporting `BaseServer` adds public API Co-authored-by: Oskar Eichler <62393985+OskarEichler@users.noreply.github.com>
Six puppeteer cases, each verified to fail against main's client: - linear progress renders a 4px green bar (main: 0px — the rules were keyed on `#bar`, which no template emits) and reports itself enabled in the startup banner (main: "Progress disabled") - circular progress renders the ring and its ARIA state - the indicator comes back on a later rebuild instead of staying faded - Escape dismisses a second overlay: `invalid` fires a DISMISS on every rebuild, so on main the first fix-then-break cycle tore down the key handler for the rest of the session - the overlay reopens under an enforced `trusted-types` policy name, where asking for the same name twice is a TypeError - a warning filter decides what the overlay renders, not just whether it opens Co-authored-by: Oskar Eichler <62393985+OskarEichler@users.noreply.github.com>
The 100ms sweep terminates a client that has not ponged yet, and a compilation can block the event loop for longer than that, so a healthy client was dropped before the `ok` stats message reached it. Seen on the macOS Node 24 shard; the same file was stabilized for neighbouring races in #5733. Wait for the build to settle before connecting. Co-authored-by: Oskar Eichler <62393985+OskarEichler@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5739 +/- ##
==========================================
+ Coverage 89.38% 90.29% +0.90%
==========================================
Files 13 13
Lines 6169 6241 +72
==========================================
+ Hits 5514 5635 +121
+ Misses 655 606 -49 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
Consolidates the parts of @OskarEichler's open PRs that fix a real defect into one branch, co-authored with him.
Closes #5724
Closes #5725
Closes #5726
Closes #5727
Closes #5728
Closes #5729
Closes #5730
Closes #5732
The client-side defects are the ones worth naming, because each is now pinned by a browser test that fails on
main: the linear progress bar rendered at0px(its rules were keyed on#bar, which no template emits);client.progress: "linear" | "circular"was dropped by the resource-query parser, so the client reportedProgress disabled; the indicator stayed faded out on every rebuild after the first; Escape stopped dismissing the overlay after one fix-then-break cycle, becauseinvalidsends a DISMISS on every rebuild and that tore down the key handler for good; the overlay could not reopen under an enforcedtrusted-typespolicy name; andclient.overlay.warnings/.errorsfilters decided only whether to open the overlay, not what it rendered.Server-side:
start()threw from an event handler instead of rejecting on an occupied port or IPC path, thebonjourprotocol log was mis-parenthesised,findIpcrashed on interfaces with a nullcidrand handedlisten()a bracketed IPv6 literal, and fourexamples/configs no longer load or validate.#5731 (glob watch rewrite) is not included — it rescans with
globSyncon everyaddevent and still carries unresolved review findings on absolute-glob watch roots andignorednegation, so it is better judged on its own.What kind of change does this PR introduce?
fix
Did you add tests for your changes?
Yes. Six new puppeteer cases in
test/e2e/progress.test.jsandtest/e2e/overlay.test.js, each confirmed to fail againstmain's client and pass against this one. Newtest/client/overlay-lifecycle.test.jsandtest/server/find-ip.test.js; extendedtest/client/index.test.js,test/client/socket-helper.test.js,test/client/utils/createSocketURL.test.js,test/e2e/api.test.js,test/e2e/built-in-routes.test.js,test/e2e/host.test.jsand theapi/overlaysnapshots.Does this PR introduce a breaking change?
No.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
webpack-dev-server/lib/servers/BaseServer.jsis now a public export, so thewebSocketServer.typedocs can reference it directly.examples/app/honoandexamples/proxyREADMEs are updated in this PR.Use of AI
Claude Code was used to review @OskarEichler's nine open PRs against the current sources, decide which changes fix a real defect, merge the accepted ones onto current
main, write the browser coverage, and run the verification below. Every included change was checked against the code it touches, and thestart()/listen()hunks were reworked by hand so cleanup failures cannot mask the startup error and a late servererroris logged rather than crashing the process.Verified locally: full lint (tsc, tsc client, eslint, prettier, cspell) clean;
build:typesreproduces the committedtypes/;test/client+test/serverpass apart from two pre-existing IPv6EAFNOSUPPORTfailures that also fail onmain. Each of the six new browser cases was run against a client built frommainand from this branch, so the failure it catches is recorded rather than assumed. The overlay, progress, api, api-plugin, client, client-reconnect, built-in-routes, web-socket-*, bonjour, on-listening, port, ipc, setup-exit-signals, static-directory, watch-files, server, multi-compiler, allowed-hosts, app, compress, cross-origin-request, entry, headers, history-api-fallback, hot-and-live-reload, lazy-compilation, logging, mime-types, module-federation, options-middleware, range-header, setup-middlewares, static-public-path, stats and target e2e suites were run; their only failures reproduce identically onmain(the sandbox has no IPv6 and no routable network address). All 50examples/**/webpack.config.*load and validate against the schema (4 fail onmain), andexamples/default,app/hono,app/connect,api/middleware,setup-middlewaresandwatch-staticwere smoke-tested serving/and/.assets/*; thehonoexample's CORP behaviour was compared against the built-in middleware directly.Generated by Claude Code
Summary by CodeRabbit
New Features
BaseServerentry point for custom WebSocket server integrations.Bug Fixes
Documentation