perf: reduce asset reporting and broadcast overhead - #5729
OskarEichler wants to merge 3 commits into
Conversation
🦋 Changeset detectedLatest commit: fc2f579 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: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. WalkthroughThe assets-report middleware calls Merge Risk: ⚪ Minimal · up to The performance changes preserve asset ordering, compiler handling, request method behavior, and broadcast semantics while reducing repeated work. Focused and existing tests pass, with no API or deployment change, so no actionable merge-blocking risk remains beyond normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 027f3a94-8e91-49f9-90f1-e47fa0af7f48
📒 Files selected for processing (2)
.changeset/server-reporting-overhead.mdlib/Server.js
Included review availability: Your plan provides up to 8 included reviews per hour; 5 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
Improvements
Compatibility
No API, dependency, engine or intended output change. Empty/closed-only client sets still do no serialization. Asset order, compiler names and single/multi-compiler handling are retained.
Verification
Real webpack compilation with 31 emitted assets confirms complete report output and narrow stats options. Controls cover directory GET/HEAD and non-GET delegation, single/multiple/closed clients and serialization counts. Combined middleware/reporting/startup controls: 15 pass. Existing report, middleware and client/server tests pass in the frozen full run. Build and lint/type checks pass. No unsupported wall-clock speedup claim.
Audit scope
This is a focused, independently based change from a broader source review at f804962. The combined frozen-source run executed 1,002 tests: 951 passed, 43 failed, one cancelled, seven skipped. Failures were traced to the separately proposed overlay DOM snapshots, reconnect-disabled test expectation and IPv6 host-test assumptions; it is not represented as a green full suite. Relevant focused results are listed above. No checked-in tests/specs/snapshots were added or modified. Hosted CI and the full OS/Node matrix remain pending.
Summary by CodeRabbit