test: rewrite to e2e - #2370
Merged
Merged
Conversation
🦋 Changeset detectedLatest commit: 83cef25 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 |
bjohansebas
force-pushed
the
hot-e2e-tests
branch
from
July 25, 2026 03:17
b9ec067 to
f8bb9c6
Compare
* feat: add browser client runtime for HMR
Ports the browser client into `client-src/`, mirroring the layout used
in `webpack-dev-server` (source in `client-src/`, built to `client/`).
The client connects to the SSE endpoint via `EventSource`, parses
query-string options from `__resourceQuery`, dispatches `building`,
`built` and `sync` payloads, applies HMR through `process-update.js`
and renders compile-time errors and warnings through an in-page overlay
(`overlay.js`).
Exposed via the `./client` subpath export so users can wire it as a
webpack entry: `require('webpack-dev-middleware/client')`. The source
is transpiled with a browser-targeted babel override and the resulting
files are shipped under `/client`.
* test: add browser client runtime tests
Covers the public client API and key SSE handling paths in jsdom:
EventSource connection on default and custom paths, ignored heartbeat
messages, dispatch of building/built/sync to subscribers, custom
handler for unknown actions, warnings on invalid JSON, EventSource
wrapper caching across multiple entries, and timeout-driven reconnect.
* test: expand browser client coverage to mirror webpack-hot-middleware
Adds tests covering the original webpack-hot-middleware client suite
(processUpdate invocations on built/sync, errored/warning behavior,
overlay show/hide transitions, the overlayWarnings option, the name
filter), while keeping the new coverage for heartbeat handling,
invalid JSON warnings, EventSource wrapper caching across entries and
timeout-driven reconnects.
* ci: run on push and PRs against the hot-middleware umbrella branch
* refactor(client): switch process-update to promise-only HMR API
The ported logic called `module.hot.check`/`apply` with both a callback
and a Promise-handling branch to support webpack < 2. In webpack 5 both
paths fire, so the callback ran twice, triggering a redundant
`module.hot.apply` on every update.
Drop the legacy callback path and use the Promise API exclusively, which
is the canonical webpack 5 contract and matches our peer dependency.
* chore(client): type-check client-src with a dedicated tsconfig
Mirrors the layout webpack-dev-server uses: a separate
`tsconfig.client.json` (`noEmit`, browser-targeted libs,
`webpack/module` augmentation) runs over `client-src/` via a new
`lint:types-client` script, with a small `client-src/globals.d.ts`
declaring `ansi-html-community` and the per-page singletons the client
stores on `window`.
Refines the JSDoc annotations in `client-src/index.js` and
`client-src/process-update.js` so `module.hot`, `window` extensions
and the HMR `ApplyOptions` type-check cleanly.
* docs: document the browser client runtime in README
Adds a 'Hot Module Replacement client' section explaining how to wire
`webpack-dev-middleware/client` as a webpack entry, the query-string
options that the runtime understands, and the programmatic
`subscribe` / `subscribeAll` / `useCustomOverlay` /
`setOptionsAndConnect` exports.
* chore(client): adopt browser-outdated-recommended-commonjs eslint preset
Switches the client-src lint config to the dedicated preset
`eslint-config-webpack` ships for browser-targeted CommonJS code, the
same family of preset webpack-dev-server uses for its own client.
Brings the per-directory rule overrides down to two:
`no-console` (legitimately used for HMR status messages) and
`no-use-before-define` (relaxed for hoisted function declarations).
Adjusts the source to satisfy the rest of the preset directly: adds
`use strict` headers, fills in JSDoc for every function, renames
`EventSourceWrapper` to `createEventSourceWrapper` (per `new-cap`),
names the anonymous module exports, and reorders `performReload`
before `handleError` so it is declared before use.
* refactor(client): route logging through webpack's runtime logger
Wraps `webpack/lib/logging/runtime` in a small `utils/log.js` module
that exposes a level-based logger registered under the
`webpack-dev-middleware` name (matching the infrastructure logger the
server side already uses). Replaces every `console.log`/
`console.warn` call in the client and HMR update path with the
equivalent `log.info`/`log.warn`/`log.error` calls so output is
prefixed and gated by a single `logging` level.
User-facing API:
- `logging` query-string option accepts `none|error|warn|info|log|verbose`
- The previous `log`, `warn`, `noInfo` and `quiet` flags are dropped
in favour of `logging`
Other cleanups enabled by this:
- Drop the `no-console: off` exception from the client-src ESLint config
- Update README's client option table accordingly
- Add tests covering the new `logging` levels and the logger prefix
* test(client): use snapshots for logger output assertions
Replaces regex-based `some(([msg]) => /.../).toBe(true)` checks on the
mocked console with `toMatchSnapshot()` over `mock.calls`. The
snapshots capture the exact log lines including the
`[webpack-dev-middleware]` prefix and per-call argument count, so any
change to the log format surfaces in the test output instead of silently
passing.
Adds explicit assertions to the existing error / warning flow tests so
`console.error` / `console.warn` mocks are not only silenced but also
verified to be called with the expected output.
* chore: document why client-src needs the ecmaVersion override
`eslint-config-webpack@4.9.6` still ships `browser-outdated-recommended-commonjs`
with `configs["javascript/es5"]` and no parser override, so `const`
is rejected. The module variant of the same preset patches this
upstream — we replicate the patch locally until the commonjs variant
does the same.
* refactor(client): migrate to ES modules and update Babel configuration
* fixup!
* fix(test): correct output path typo in webpack.array.warning fixture
The first compiler entry used `../../outputs/...` which escaped the test
directory and wrote artifacts to the repository root, outside of the
`/test/outputs` paths covered by `.gitignore` and `.prettierignore`.
* feat: implement hot module replacement middleware
Adds a `hot: true | { path, heartbeat, log, statsOptions }` option that
turns the dev middleware into a Server-Sent Events endpoint publishing
`building`, `built` and `sync` payloads from the webpack compiler.
The hot endpoint defaults to `/__webpack_hmr` and is served by the
existing middleware - no separate `app.use()` call is required.
`close()` tears down clients and the heartbeat timer.
* test: add tests for hot middleware
Covers schema validation of the `hot` option (success and failure cases
with snapshots), unit tests for `pathMatch`, `formatErrors`,
`buildModuleMap` and `createEventStream`, and integration tests that
verify SSE headers, the default and custom hot paths, MultiCompiler
support, `close()` teardown, and the `log` option (custom function and
`log: false`).
* test: add unit and integration tests for hot middleware functionality
* feat: enhance honoWrapper to support Web ReadableStream for hot middleware responses
* feat: add TypeScript definitions for hot module replacement functionality
* test(hot): cover publish, sync-on-connect, headers and close behavior
Ports the remaining unit-level cases from webpack-hot-middleware that
were not already covered by the framework matrix in middleware.test.js:
- the public `publish()` API broadcasts custom payloads
- a client connecting after a build receives a `sync` event
initialised from the last stats
- HTTP/1 clients get `Connection: keep-alive`, HTTP/2 clients do not
- when `stats.name` is empty the published payload falls back to
`compilation.name`
- a single broadcast reaches every attached client
- after `close()` further compiler events do not produce writes
* docs: document the hot option in README
* docs: list the hot option in the README options table
* refactor: replace EXPECTED_ANY with specific types in hot module definitions
* docs: update README to clarify default stats options for SSE payload
* refactor: remove log option from hot middleware and update related documentation
* docs: update default value for hot option in README to false
…dency (#2350) The client runtime runs in the browser, so Node's util.stripVTControlCharacters is not an option. Inline the ansi-regex pattern (verified against strip-ansi@6 output) in a small util instead of shipping the dependency. Ref webpack/webpack-hot-middleware#465 Ref webpack/webpack-hot-middleware#474
webpack-dev-server does not send module names over the wire: the HMR runtime logs module ids on apply. Align the SSE payload with that (name, action, time, hash, errors, warnings) instead of serializing a module id → name map that was only used for log cosmetics and was empty with the default stats options anyway. Ref webpack/webpack-hot-middleware#452 Ref webpack/webpack-hot-middleware#306
* feat(client): add disconnect() to close the SSE connection Expose a way to close the EventSource for the current path and stop the reconnection watchdog (e.g. before tearing the page down). The cached wrapper is dropped so a later setOptionsAndConnect() opens a fresh connection. Ref webpack/webpack-hot-middleware#367 * fixup!
The compiler's invalid hook reports which file invalidated the compilation. Forward it as an optional `file` field on the `building` payload and log it in the client, so users can see what triggered a rebuild. Ref webpack/webpack-hot-middleware#173
…er (#2353) * feat(client): bring the error overlay to parity with webpack-dev-server - Render inside an about:blank iframe and style exclusively through the CSSOM so the overlay works under a strict style-src CSP; inline styles from ansi-html are re-applied via style.cssText. - Support Trusted Types: innerHTML writes go through a policy (configurable via overlayTrustedTypesPolicyName). - Capture uncaught runtime errors and unhandled rejections in the overlay (overlayRuntimeErrors, default true), with the same React error boundary heuristic as dev-server. - Inline the HTML entity encoder and drop the html-entities dependency. - Expose the overlay as a standalone subpath export (webpack-dev-middleware/client/overlay) so webpack-dev-server can reuse it. Ref webpack/webpack-hot-middleware#457 * feat(client): support opening file references in the editor When `overlayOpenEditorEndpoint` is set, file chips in the overlay become clickable and issue GET <endpoint>?fileName=<file:line:column> — the same contract as webpack-dev-server's open-editor route. The endpoint implementation is left to the server integration, so no launch-editor dependency is added. * feat(client): bring the error overlay to parity with webpack-dev-server - Render inside an about:blank iframe and style exclusively through the CSSOM so the overlay works under a strict style-src CSP; inline styles from ansi-html are re-applied via style.cssText. - Support Trusted Types: innerHTML writes go through a policy (configurable via the overlay's trustedTypesPolicyName). - Capture uncaught runtime errors and unhandled rejections in the overlay, with the same React error boundary heuristic as dev-server. - Adopt dev-server's client.overlay option shape: a boolean or a JSON object with errors/warnings/runtimeErrors (booleans or filter functions) and trustedTypesPolicyName, so dev-server configs migrate as-is. Warnings are shown by default and no longer block updates, matching dev-server. - Support opening file references in the editor: when overlayOpenEditorEndpoint is set, file chips issue GET <endpoint>?fileName=<file:line:column> (same contract as dev-server's open-editor route); the endpoint implementation is left to the server integration. - Inline the HTML entity encoder and drop the html-entities dependency. - Expose the overlay as a standalone subpath export (webpack-dev-middleware/client/overlay) so webpack-dev-server can reuse it. Ref webpack/webpack-hot-middleware#457 Ref webpack/webpack-hot-middleware#184 * fix(client): track overlay problems per compilation With a MultiCompiler the client receives one event per bundle, and a successful build from one bundle used to wipe another bundle's still-valid errors from the overlay. Keep the live problems keyed by compilation name, render the union, and only clear the overlay when every compilation is clean. The console de-duplication cache is also keyed per bundle so interleaved payloads do not defeat it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(examples): add a MultiCompiler hot example Keep examples/hot as the minimal single-compiler setup (now with runtime-error buttons and an opt-in DEMO_WARNING build warning), and add examples/hot-multi-compiler: two compilers ("app" and "admin") sharing one middleware instance and a single SSE connection, with per-bundle `?name=` client scoping and recipes for the overlay's per-compilation error tracking. Each multi config sets a distinct output.uniqueName (and prefixed hot-update filenames): with both bundles on one page, a shared webpackHotUpdate global would make each bundle's updates land in the wrong runtime. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(client): nest overlay extensions inside the overlay option Move styles, ansiColors and openEditorEndpoint into the overlay object as webpack-dev-middleware extensions of dev-server's client.overlay shape, replacing the flat overlayStyles/ansiColors/ overlayOpenEditorEndpoint options. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Isolate the HMR runtime lookup behind a small util so process-update is loadable under jest, and add coverage for the updated-modules console output.
The success path now logs a one-line summary with the module count at the default "info" level, and the per-module detail as a collapsed console group visible from the "log" level up (webpack's runtime logger gates groups below that level). The unaccepted-modules warning list stays flat: it is diagnostic output for the failing case. Ref webpack/webpack-hot-middleware#311
The overlay now shows one problem at a time, with a header row holding
the problem badge and prev/next navigation (colored after the problem
type), a page counter and arrow-key navigation. Each new problem set
starts at its first page, accumulated runtime errors land on the
newest one, and identical re-published sets (e.g. another bundle of a
multi-compiler syncing) keep the current page. Enabled by default;
disable with the overlay option's `paginate` extension
(`?overlay={"paginate":false}`) to render the full list.
Also ignore click targets re-rendered away mid-dispatch in the
backdrop-dismiss listener, which otherwise closed the overlay when
clicking the navigation buttons.
* fix(client): avoid double slash when joining dynamicPublicPath When output.publicPath ends with a slash (the common case) and the SSE path starts with one, the naive concatenation produced URLs like `https://host//__webpack_hmr`, which never match the server-side pathname check. Ref webpack/webpack-hot-middleware#366 * fix(client): append path to the public path like a filename in dynamicPublicPath The default `path` starts with "/", so the naive concatenation with a public path ending in "/" produced URLs like `https://host//__webpack_hmr`, which never match the server-side pathname check. Strip the leading slash from `path` and concatenate without any other normalization: the public path itself is left untouched (intentional double slashes, e.g. nginx rewrites, are preserved) and is expected to end with "/". Ref webpack/webpack-hot-middleware#366
#2358) The client shows a small badge (shadow DOM, CSSOM-only styles) while a rebuild is in progress, enabled by default (disable with `?progress=false` on the client entry). When the server enables the new `hot.progress` option, webpack's ProgressPlugin publishes throttled `progress` events over SSE (deduplicated by rounded percent, reset on each new build) and the badge renders a CSP-safe SVG progress ring with the compilation percentage. The palette is shared with the error overlay through a common theme module, and the indicator is exposed as a standalone subpath export (webpack-dev-middleware/client/indicator) so other tooling can reuse it. Both the indicator and the overlay now bail out safely when `document.body` does not exist yet. Ref webpack/webpack-hot-middleware#167
* docs: add HMR notes and troubleshooting section Covers the recurring questions from the webpack-hot-middleware backlog: browser connection limits and HTTP/2 (webpack/webpack-hot-middleware#423, webpack/webpack-hot-middleware#298), warning filtering layers (webpack/webpack-hot-middleware#319, webpack/webpack-hot-middleware#228), absolute paths and public paths (webpack/webpack-hot-middleware#281, webpack/webpack-hot-middleware#203, webpack/webpack-hot-middleware#193), and a custom-events recipe (webpack/webpack-hot-middleware#118, webpack/webpack-hot-middleware#226). The client `reload` option now also notes its default differs from webpack-hot-middleware (webpack/webpack-hot-middleware#370). * fixup!
* fix(hot): publish sync instead of built for unchanged bundles With a MultiCompiler, rebuilding one child re-emits done for every child. Clients of the unchanged bundles then try to fetch a hot-update manifest that was never emitted (404 / "Cannot find update", or an unwanted full reload with reload=true). Compare each bundle's hash against the previous build and announce unchanged bundles as `sync` instead of `built`. The first build still publishes `built`, and new clients are still caught up with `sync` — but no longer while a rebuild is in progress. Ref webpack/webpack-hot-middleware#312 * fixup!
* fix(client): reload when an accept handler errors during apply An error thrown inside a module.hot.accept handler was logged and ignored, leaving the page running stale code with no recovery. When the `reload` option is enabled (the default), onErrored now falls back to a full page reload, like the other unrecoverable update paths. The HMR runtime and page-reload calls are isolated behind small utils so process-update finally has direct test coverage. A missing HMR runtime is now reported with a single actionable error instead of throwing at bundle evaluation. Ref webpack/webpack-hot-middleware#334 * fixup! * fixup!
- A client connecting after a build now receives the catch-up `sync` alone instead of it being broadcast to every connected client, which re-triggered their reporters on each new tab. - `publishBundles` pairs the previous build's bundles by name instead of array index, so a changing set of compilations cannot compare a bundle against a sibling's hash. Unnamed bundles keep the positional pairing. - The client's console dedup cache is cleared per bundle, so a sibling's clean build no longer re-logs another bundle's unchanged problems.
instance.close() left `context.hot` set while `handle()` silently returned, so a request to the hot path after close received no response, no `next()`, and hung until the socket timed out. Detach the SSE intercept on close so requests fall through to the regular middleware, and answer 404 from `handle()` for requests that raced the intercept.
…#2369) The badge state lives in a window singleton (same pattern as the overlay), so a second bundled copy of the module drives the same badge instead of stacking a duplicate, and fields missing from another version's state are filled in place. show() and hide() take an optional source: each concurrent build keeps the badge alive until every source finished, so in a MultiCompiler one bundle's `built` no longer hides the badge while a sibling is still compiling — and with two clients sharing the badge, one client's hide() cannot drop the other's indication. hide() without a source still removes the badge unconditionally. Progress payloads carry no name, so the client attributes them to the most recent `building` event.
…ource (#2368) * feat(client): share the overlay across bundled copies and report by source The overlay DOM and problem state now live in a window singleton, so a second bundled copy of the module (e.g. the webpack-dev-server client once it adopts this overlay) renders into the same iframe instead of stacking a duplicate. Fields missing from a state created by an older copy are filled in place, and the Trusted Types policy moves into the shared state too — creating two policies with the same name throws under an enforced CSP. * test(overlay): prevent re-rendering when clearing a source that reported nothing
…and remove cspell.config.json
The building payload now carries the name of the compilation that invalidated (tapped per child compiler, since the MultiCompiler hook does not say which one fired), so clients can pair it with the built or sync that follows — without it the building indicator registered every build under "" and could never be hidden for named compilations. On the client, the inactivity watchdog is restarted inside init(), so it survives a reconnect instead of dying with the first clearInterval, and the reconnect timeout handle is now stored and cleared by close(), so disconnect() during the reconnect window no longer resurrects an orphaned, uncloseable connection. All three defects were inherited from webpack-hot-middleware.
…tener The handshake now ends a response whose headers were already sent instead of crashing on writeHead, and the middleware routes handshake errors to next() — an exception there previously became an unhandled rejection that killed the process. The overlay's Escape listener on the host document is now attached lazily inside ensureOverlay (once per page, through the shared state), matching how webpack-dev-server registers it inside createOverlay, so importing the client in a non-DOM environment (SSR bundle, worker) no longer throws at evaluation time.
…ate-name bundles by occurrence statsOptions now accepts only the object form everywhere: the schema rejected string presets the types allowed, and booleans validated but were silently ignored since toBundles only merges objects over the middleware's base stats options. Schema, JSDoc, and generated types all agree now, so invalid forms fail validation instead of at runtime. heartbeat: 0 was schema-valid but silently replaced with the 10s default by a falsy check — the schema now requires >= 1 and the code uses ?? so the option and its validation tell the same story.
The sibling-sync race runs deterministically against the real runtime: a slowed-down manifest route holds the sibling's check open while the own sync locks the compilation name, and the failed check is discarded in silence — snapshotted as a connect and a single "Checking". Trying to force the runtime's failure status through a missing update chunk surfaced the real behavior instead: check rejects with a ChunkLoadError, no failure status is reached, and the client logs "Update check failed" and stays put — now pinned as its own test. The abort-status unit stays: no real flow reaches that state on demand. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The failure-status branch it exercised is unreachable through the real runtime: the client's ignoreUnaccepted/ignoreDeclined/ignoreErrored options close every path webpack 5.109 has to the abort and fail statuses. The branch itself stays as a cheap guard for upstream changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
Author
|
I know this is a large PR. I can split it into smaller parts if that helps. Also, it currently only uses Express would you like us to test the other frameworks as well? |
bjohansebas
marked this pull request as ready for review
July 26, 2026 05:11
…or real A fixture function literally named invokeGuardedCallbackDev throws, so the genuine stack carries the marker React's dev build leaves on boundary-handled errors — the overlay ignores it while a plain error right after proves the listeners live. The runtime slot resets through its real trigger: a clean rebuild's reporter clears the accumulated errors, and the next one starts a fresh slot with no pager. The jsdom runtime-errors describe is removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2370 +/- ##
==========================================
+ Coverage 94.20% 96.20% +2.00%
==========================================
Files 4 12 +8
Lines 1242 1660 +418
Branches 380 0 -380
==========================================
+ Hits 1170 1597 +427
+ Misses 64 63 -1
+ Partials 8 0 -8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The ERROR/WARNING badges (text and background) and the highlighted file path are checked where Chrome actually renders them — on webpack's own parse error and critical-dependency warning. Three jsdom rendering tests leave. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The final overlay rendering details run in Chrome: the code-frame highlight on webpack's real parse error, the re-mount after something wiped the iframe (re-triggered by an unchanged rebuild, exercising the identical-set guard), clear() as a no-op with nothing shown, and the query-configured custom card styles and ansi colors driving the badge and border. With no jsdom consumer left, jest-environment-jsdom leaves the dev dependencies. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Migrating server behavior to the browser suites moved its execution out of the coverage run — the heartbeat write, the SSE handshake, and every hot.js path only e2e exercises showed as uncovered. The e2e step now instruments src/ too (the middleware runs inside the jest process even under a browser), writes to its own directory, and the artifact carries both lcov files for codecov to merge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
macOS fired a spurious startup rebuild whose unchanged hash broadcast a legitimate sync to every client — indistinguishable from a re-sent catch-up at the frame level. The helper's settle() drains builds until the watcher goes quiet, and the catch-up test runs after it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replaces Array#includes inside the filter over updated modules (O(n·m) → O(n+m)) — noticeable on updates touching hundreds of modules.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 37cc3ff. Configure here.
| uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 | ||
| with: | ||
| token: ${{ secrets.CODECOV_TOKEN }} | ||
| directory: coverage |
There was a problem hiding this comment.
Coverage upload skipped on test failures
Low Severity
upload-coverage depends on test with no if: always() (or equivalent). If any matrix shard fails, the coverage job is skipped entirely, so Codecov gets nothing for that run. Previously each test job submitted coverage on its own, so partial results still reached Codecov.
Reviewed by Cursor Bugbot for commit 37cc3ff. Configure here.
| * and error events already queued behind the close (the EventSource fires | ||
| * one when its connection dies) can no longer resurrect the wrapper. | ||
| */ | ||
| const close = () => { |
Member
There was a problem hiding this comment.
is this double check really needed? whats the condition this will fail using only stop once?
Member
There was a problem hiding this comment.
It is needed, and there is a concrete case behind it.
handleDisconnect is registered as the EventSource's error listener, and it unconditionally schedules a reconnect. stop() cancels the timers and closes the source, but it cannot cancel an error event the EventSource has already queued. So with stop() alone the sequence is:
disconnect()→stop()— timers cleared, source closed, nothing pending.- The already-queued
errorevent fires. handleDisconnectruns and callssetTimeout(init, timeout).
A connection that was closed for good comes back, and the page starts receiving updates again after the host asked it to stop. The closed flag is what makes step 3 a no-op; init() resets it, so a genuine reconnect still works.
It is a race, so it does not reproduce on demand in the browser suite — the branch is marked istanbul ignore with that reason rather than covered by a contrived test.
Generated by Claude Code
- skip payload serialization in publish() when no client is connected, and cut the ProgressPlugin callback early via eventStream.hasClients() - start the heartbeat interval with the first client and stop it with the last one, instead of ticking forever - pair bundles against the previous build through a name-grouped Map, replacing the per-bundle filter (O(n²) → O(n)) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
alexander-akait
force-pushed
the
hot-middleware
branch
from
September 1, 2026 15:08
11f24ab to
e1a5e29
Compare
Resolves 20 files. Server side keeps both intents: the branch's lazy heartbeat and hasClients() alongside main's idempotent disconnect, writableEnded guards and the stats-driven diagnostics from #2392. client-src takes main's ES5-hardened versions, which the branch predates (appendChild over append, a hand-rolled parseQuery, Object.create(null) over Map). Two of the branch's client fixes survive that: the Trusted Types log guard is kept as-is, and the EventSource closed flag is re-applied on top of main's wrapper, which still schedules a reconnect from an error event queued behind a close. The third is superseded -- main computes unaccepted modules with indexOf, which is what the Set was for and is ES5-safe. Coverage now collects from client-src as well as src, so a gap in the hot client shows up on the report like any other.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe change adds client-served hot module replacement support with lazy SSE heartbeats, connection safeguards, guarded logging, and optimized bundle pairing. It adds package exports, Puppeteer-based E2E infrastructure, browser coverage collection, and CI coverage aggregation. New browser tests cover client updates, SSE protocol behavior, indicators, logging, overlays, multi-compiler builds, and update-processing failures. Legacy client, indicator, overlay, and update-processing unit tests are removed. Merge Risk: 🟡 Moderate · up to This PR replaces client tests with browser E2E coverage, expands CI execution, changes hot-client behavior, and narrows package exports. Current risks include lost failure diagnostics, credential exposure during CI test execution, E2E checks that do not fully prove their intended behavior, possible intermittent failures, and deep-import breakage under a minor release. Merge should wait for fixes or explicit owner acceptance. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 97.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 40 functions across 24 files. (1 skipped: 1 unsupported.) ✨ 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: 4
🧹 Nitpick comments (1)
test/e2e/client.test.js (1)
423-424: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWait for the watcher to go quiet before the strict assertions.
This test asserts exact equality on the subscriber actions and snapshots the whole console. A spurious startup rebuild adds a
builtaction and an extra console line, so both assertions fail intermittently.test/e2e/protocol.test.js(lines 97-99) documents this exact hazard and callsapp.settle()for it.Call
await app.settle()aftercreateHotAppand beforepage.goto.♻️ Proposed change
({ page, browser } = await runBrowser()); const console_ = collectConsole(page); + // A startup rebuild would broadcast extra traffic and break the exact + // action list below. + await app.settle(); + await page.goto(app.url);
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Team
Run ID: 3381a217-9036-41c0-9181-07c267616def
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (29)
.changeset/hot-middleware-migration.md.github/workflows/nodejs.yml.gitignorebabel.config.jsclient-src/index.jsclient-src/utils/log.jseslint.config.mjsjest.config.jspackage.jsonsrc/hot.jstest/e2e/__snapshots__/client.test.js.snap.webpack5test/e2e/__snapshots__/logging.test.js.snap.webpack5test/e2e/__snapshots__/multi-compiler.test.js.snap.webpack5test/e2e/__snapshots__/overlay.test.js.snap.webpack5test/e2e/__snapshots__/process-update.test.js.snap.webpack5test/e2e/client.test.jstest/e2e/indicator.test.jstest/e2e/logging.test.jstest/e2e/multi-compiler.test.jstest/e2e/overlay.test.jstest/e2e/process-update.test.jstest/e2e/protocol.test.jstest/helpers/console-collector.jstest/helpers/e2e.jstest/helpers/hot-app.jstest/helpers/puppeteer-constants.jstest/helpers/run-browser.jstest/logging.test.jstypes/hot.d.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.
Comment on lines
+31
to
+45
| env: { | ||
| test: { | ||
| presets: [ | ||
| [ | ||
| "@babel/preset-env", | ||
| { | ||
| targets: { | ||
| node: "18.12.0", | ||
| }, | ||
| }, | ||
| ], | ||
| ], | ||
| plugins: ["@babel/plugin-transform-runtime"], | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify that the test early-return branch precedes the env.test block.
sed -n '1,70p' babel.config.jsRepository: webpack/webpack-dev-middleware
Length of output: 1446
Apply the test settings in the active Babel branch.
When api.env("test") is true, Babel returns before it reaches env.test. Tests therefore use NODE_TARGETS (node: "20.9.0") and do not apply node: "18.12.0" or @babel/plugin-transform-runtime. Merge these settings into the early return, or remove the early return.
Comment on lines
+115
to
+120
| // The widget's own rebuild drops its cache: same text logs again. | ||
| app.edit("widget", widgetWithWarning("widget-v2")); | ||
| await waitForText(page, "out-widget", "widget-v2"); | ||
| await console_.waitForCount("App is up to date", 3); | ||
|
|
||
| expect(normalizeConsole(console_.messages)).toMatchSnapshot(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the warning after the widget's own rebuild.
Lines 115-120 require the second widget build to log the unchanged warning again. The test only waits for "App is up to date". The snapshot records only one Critical dependency warning. This test passes if the widget warning cache never resets.
test/e2e/multi-compiler.test.js#L115-L120: wait forconsole_.waitForCount("Critical dependency", 2)before the snapshot assertion.test/e2e/__snapshots__/multi-compiler.test.js.snap.webpack5#L23-L30: update the snapshot to include the second widget warning.
Proposed test change
app.edit("widget", widgetWithWarning("widget-v2"));
await waitForText(page, "out-widget", "widget-v2");
await console_.waitForCount("App is up to date", 3);
+ await console_.waitForCount("Critical dependency", 2);
expect(normalizeConsole(console_.messages)).toMatchSnapshot();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // The widget's own rebuild drops its cache: same text logs again. | |
| app.edit("widget", widgetWithWarning("widget-v2")); | |
| await waitForText(page, "out-widget", "widget-v2"); | |
| await console_.waitForCount("App is up to date", 3); | |
| expect(normalizeConsole(console_.messages)).toMatchSnapshot(); | |
| // The widget's own rebuild drops its cache: same text logs again. | |
| app.edit("widget", widgetWithWarning("widget-v2")); | |
| await waitForText(page, "out-widget", "widget-v2"); | |
| await console_.waitForCount("App is up to date", 3); | |
| await console_.waitForCount("Critical dependency", 2); | |
| expect(normalizeConsole(console_.messages)).toMatchSnapshot(); |
📍 Affects 2 files
test/e2e/multi-compiler.test.js#L115-L120(this comment)test/e2e/__snapshots__/multi-compiler.test.js.snap.webpack5#L23-L30
The e2e suite already exercised client-src, but nothing counted it: only
jest-instrumented modules were measured, so the browser-only paths read 0%
and the number came from the jsdom tests alone.
The e2e bundle now instruments client-src as webpack builds it, and the
counters are read off each page before its browser closes. Two details
matter. Istanbul's default way of reaching the global is
new Function("return this"), which a page enforcing
require-trusted-types-for 'script' refuses to compile, so the scope is
named directly -- instrumenting must not change what the client can run
under. And a reload takes window.__coverage__ with it, which is exactly
what reloadPage and the unaccepted-update branches would have reported,
so the counters are parked in sessionStorage on pagehide and picked up
afterwards.
client-src goes from 94.58% to 96.71% of statements and 85.35% to 90.07%
of branches; theme.js, utils/get-hot.js and utils/reload.js were each
reported at 0% and are now fully covered. Two e2e cases cover what was
left reachable: the overlay query override, an empty pair in the client's
own query string, and the non-JSON overlay value that falls back to on.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Team
Run ID: 56cd034d-0dac-4a06-8291-1e5e4f0e87a2
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
.cspell.json.github/workflows/nodejs.yml.gitignorepackage.jsontest/e2e/overlay.test.jstest/helpers/browser-coverage.jstest/helpers/e2e.jstest/helpers/hot-app.jstest/helpers/merge-coverage.jstest/helpers/run-browser.js
🚧 Files skipped from review as they are similar to previous changes (1)
- .gitignore
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.
The four jsdom suites duplicated what test/e2e now drives through a real browser, and since the client is measured from the browser they were no longer carrying coverage either. Removing them takes jest-environment-jsdom with them; the remaining jest suites are node-only. client-src is at 97.82% of statements from the browser suite alone. Four cases cover what the deletions left reachable: a second connect for an already-subscribed path, a host-supplied custom overlay, an update to a declined module, and an error event carrying neither an error nor a message. The environment guards a browser cannot produce -- a document-less import, a script running before <body>, a frame without a document -- are marked rather than faked, since contriving them is how these suites would turn flaky. The browser suite says the same thing on every OS and node version while costing the slowest job in the matrix, so it runs on ubuntu/22.x alone. All four shards still run there, so the suite is covered exactly once.
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Team
Run ID: e0dcf68c-368b-4ec1-8bda-de5d5fc1d22f
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
.github/workflows/nodejs.ymlclient-src/indicator.jsclient-src/overlay.jspackage.jsontest/__snapshots__/client.test.js.snap.webpack5test/client.test.jstest/e2e/client.test.jstest/e2e/overlay.test.jstest/e2e/process-update.test.jstest/indicator.test.jstest/overlay.test.jstest/process-update.test.js
💤 Files with no reviewable changes (6)
- test/overlay.test.js
- test/client.test.js
- test/snapshots/client.test.js.snap.webpack5
- test/process-update.test.js
- test/indicator.test.js
- package.json
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.
| console_.messages.filter((text) => | ||
| text.includes("Ignored an update to declined module"), | ||
| ), | ||
| ).toHaveLength(1); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert that the declined update does not reload the page.
The current assertion verifies only the log count. A reload recreates this fixture with "v1", so the test does not prove that the page stayed active. Set a reload marker after the first render and assert that it remains after the declined-update message. Reuse the marker approach from the following reload test.
Two of my own tests were green for the wrong reason. The custom-overlay one stubbed show/clear, but the client calls showProblems/clear, so only clear ever ran and a length check passed on it; it now asserts showProblems with the errors. The declined-update one asserted a log count while the page was in fact reloading underneath it -- a decline cannot be applied, so the client falls back to a full reload, which the test now waits for and asserts. The bare-error-event test checked the DOM only after triggering a real error, which would have reused a wrongly created overlay. The runtime-error tests raced the client: attachRuntimeErrorListeners runs during the client's own setup, while the tests only waited for the app to render, so an error thrown first landed nowhere and the overlay never appeared. Two different tests failed on two consecutive runs from this. They now wait for the listeners through the state the client already exposes -- including after a reload, and excluding the test whose configuration deliberately attaches none. Also: the merge left an unreachable env.test block in babel.config.js behind the early return, naming a plugin the same merge removed; a failed coverage read could leave Chromium running; the SSE reader dropped frames split across TCP chunks; snapshots pinned V8 and puppeteer wording that a toolchain bump would change; a multi-compiler comment claimed a re-log that the dedup cache makes impossible; and upload-coverage skipped entirely when a shard failed.
The browser suite was running in all 48 matrix jobs, and the 4-way shard axis this branch added existed to absorb that cost. It drives a real Chromium, so every OS and node version reports the same thing. It moves to a single ubuntu job. The shard axis goes with it, which puts the test matrix back to what main has: 3 OS x 4 node versions, 12 jobs running the node suite unsharded. Together with lint, the client job and the coverage upload, a pull request goes from 48 test jobs to 12 plus one.
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Team
Run ID: b0f50d01-b2d5-4d1f-988a-43fa428347a2
📒 Files selected for processing (1)
.github/workflows/nodejs.yml
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
Comment on lines
+89
to
+93
| - name: Upload coverage artifact | ||
| uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 | ||
| with: | ||
| name: coverage-${{ matrix.os }}-${{ matrix.node-version }}-${{ strategy.job-index }} | ||
| path: coverage/lcov.info |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- workflow excerpt ---'
cat -n .github/workflows/nodejs.yml | sed -n '1,165p'
printf '%s\n' '--- related status conditions and coverage references ---'
rg -n -C 3 'test:coverage|upload-artifact|download-artifact|always\\(\\)|cancelled\\(\\)|coverage/lcov|needs:' .github/workflows/nodejs.ymlRepository: webpack/webpack-dev-middleware
Length of output: 7478
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- package scripts ---'
node - <<'JS'
const fs = require('fs');
const p = JSON.parse(fs.readFileSync('package.json', 'utf8'));
for (const k of ['test:coverage', 'test:e2e', 'coverage:client']) {
console.log(`${k}: ${p.scripts?.[k] ?? '<missing>'}`);
}
JS
printf '%s\n' '--- coverage-related files ---'
fd -HI -t f 'coverage|e2e|client' . | head -80
printf '%s\n' '--- coverage:client definitions and report paths ---'
rg -n -C 5 'coverage:client|coverage-client|coverage-e2e|lcov\\.info' package.json .github scripts test test* 2>/dev/null || trueRepository: webpack/webpack-dev-middleware
Length of output: 8572
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- merge-coverage implementation ---'
cat -n test/helpers/merge-coverage.js
printf '%s\n' '--- browser coverage producer/consumer references ---'
rg -n -C 6 'browser-coverage|E2E_COVERAGE|coverage-final|coverage-e2e|collectCoverage|coverageDirectory' test jest* package.json .github/workflows/nodejs.yml 2>/dev/null || trueRepository: webpack/webpack-dev-middleware
Length of output: 10903
Run coverage collection and artifact uploads after test failures.
The test and client jobs use the default success() condition for dependent steps. A failed unit-test run can skip coverage/lcov.info upload. A failed E2E run can skip both coverage collection and artifact upload. upload-coverage.if: always() cannot recover artifacts that producer jobs did not upload.
Add if: ${{ !cancelled() }} to the affected collection and upload steps. Set explicit if-no-files-found: ignore handling for missing reports. Validate one failed unit-test run and one failed E2E run.
| cancel-in-progress: true | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
sed -n '80,125p' .github/workflows/nodejs.yml
printf '\nWorkflow-level and job-level permissions:\n'
rg -n -C 3 'permissions:|pull_request:|workflow_dispatch:|browser|checkout|npm ci|test:e2e|coverage:client' .github/workflows/nodejs.ymlRepository: webpack/webpack-dev-middleware
Length of output: 4479
🌐 Web query:
actions/checkout persist-credentials default true README
💡 Result:
In the GitHub Actions actions/checkout action, the persist-credentials input defaults to true [1][2]. When persist-credentials is set to true (the default), the action configures the authentication token or SSH key used to fetch the repository into the local git configuration [1][2]. This allows subsequent steps in the same job to execute authenticated git commands (e.g., git push) without needing to manually re-configure authentication [1][3]. The action automatically removes these credentials during the post-job cleanup phase [1][4]. If you wish to prevent this behavior, you can set persist-credentials to false in your workflow YAML file: - name: Checkout uses: actions/checkout@v4 with: persist-credentials: false While some users have raised security concerns regarding this default behavior [5], the action stores these credentials securely (e.g., in $RUNNER_TEMP rather than the repository's .git/config in recent versions) to limit potential exposure [1][5]. Users who do not require authenticated git operations in their scripts may choose to disable it as a security best practice [1].
Citations:
- 1: https://github.com/actions/checkout
- 2: https://raw.githubusercontent.com/actions/checkout/v6/action.yml
- 3: https://github.com/marketplace/actions/checkout
- 4: https://github.com/actions/checkout/blob/0299a0d2b67d48224ce047d03c69693b37fe77fe/README.md
- 5: GitHub issue 2312 in actions/checkout (link omitted to avoid creating a cross-reference)
Sensitive Data Exposure (CWE-522): Insufficiently Protected Credentials
Reachability: External · Exploitability: Moderate
Disable checkout credential persistence in the client job.
The job runs npm ci and browser tests after checkout. Set persist-credentials: false. Keep the existing workflow permission scope of contents: read; do not grant additional permissions.
🧰 Tools
🪛 zizmor (1.29.0)
[warning] 107-107: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
Source: Linters/SAST tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What kind of change does this PR introduce?
Did you add tests for your changes?
Does this PR introduce a breaking change?
If relevant, what needs to be documented once your changes are merged or what have you already documented?
Use of AI
Note
Medium Risk
Touches production hot-client reconnect/logging behavior and replaces most client test coverage with slower, flakier browser e2e; CI matrix size grows with four shards per OS/Node combo.
Overview
Replaces the large jsdom
test/client.test.jssuite with Puppeteer browser tests undertest/e2e/(HMR client, logging, overlay, multi-compiler, indicator, process-update), bumps to 8.1.0, swaps jest-environment-jsdom for puppeteer, and wirestest:e2einto the defaultnpm testflow.CI shards Jest (
--shard=1/4…4/4) for both unit coverage and e2e, uploads merged lcov artifacts, and uploads to Codecov in a dedicated job.Client fixes alongside the test move: EventSource
disconnect()uses aclosedflag andstop()so late error events cannot schedule reconnects;logmethods are try/catch-wrapped for Trusted Types CSP;logUpdatesuses aSetfor renewed module ids.Reviewed by Cursor Bugbot for commit 37cc3ff. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
hotoption for enabling Hot Module Replacement directly through the development middleware.Bug Fixes