feat(hot): add apply and connect, replacing six browser options - #2458
alexander-akait wants to merge 6 commits into
Conversation
🦋 Changeset detectedLatest commit: e92977b 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 |
Two unions, both of options that shipped after 8.3.0 — so nothing released
has to keep working and no aliases are needed.
`apply` replaces `hot`, `liveReload` and `reload`. Only four of their eight
combinations ever differed, which the control flow says plainly:
`liveReload` is read only in the `else` branch of `if (hot)`, and `reload`
only inside `applyUpdate`, which the `if` branch calls. Three booleans for
one decision.
hot: true, reload: true -> apply: "hmr"
hot: true, reload: false -> apply: "hmr-only"
hot: false, liveReload: true -> apply: "reload"
hot: false, liveReload: false -> apply: "nothing"
`connect` replaces `autoConnect`, `reconnect` and `timeout`: `false` does
not connect on load, and an object carries `retries` and `timeout`. The
per-transport semantics are unchanged, still in `socket-options.js`, which
is why `connect` could land without re-deriving them.
The page-url parameter follows `apply` and gains from it: it used to be
able only to turn something off, so `-hot=false` and `-liveReload=false`
were two switches with a gap between them. `-apply=<mode>` asks for an
outcome, and `=false` is still taken as `nothing` because that is what the
old pair meant together.
Both are the shape this codebase already uses everywhere — `overlay`,
`token`, `cors`, `progress` and `transport` are all a scalar that may be an
object — so neither reads as an exception.
Two things found on the way. The symmetry test could not see a type with an
object literal in it: `[^}]+` stopped at the brace inside
`{ retries?: number }`, so `connect` looked absent from the typedef. And
one live-reload case had combined a recognised `hot=false` with an
unrecognised `live-reload=false`; with one option that scenario cannot
exist, so it now asserts what it was really about — an unrecognised value
leaves the default in force.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
`-hot=false` turned Hot Module Replacement off and left live reload on, so the build reached the page as a reload. I had mapped it to `-apply=nothing`, which is the one mode where the build reaches the page not at all — so the test waited thirty seconds for text that was never going to change. `reload` is the mode that behaviour had a name for. The equivalent fixture in `live-reload.test.js` was mapped correctly, which is what made the difference visible. Also drops the snapshot left behind by a renamed test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. WalkthroughThe client now uses Priority: ➖ Normal Merge Risk: 🔵 Low · up to Readers may try a page-URL connection setting that has no effect. Clarify that only apply has a page-URL override; the remaining issue is bounded and does not prevent merging. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The changes remain concentrated in development-client configuration and update behavior. Page URLs gain the ability to select an update strategy, but cannot use this mechanism to select an endpoint, transport, or credential. No concrete security exploit was established; deployment exposure remains uncertain. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Update the entry-query example and the setOptionsAndConnect comment to the new… · README.md:795
README.md:795
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the entry-query example and the
setOptionsAndConnectcomment to the new option names.The example at Line 795 uses
reload=false. The comment at Line 925 says "whenautoConnect=false". This PR removes both options.setOverridesreads onlyapplyandconnect, so the client ignoresreload=false. As a result, a user who copies the example keeps the default"hmr"mode with reload fallback.Proposed fix
- "webpack-dev-middleware/client?reload=false&overlay=false", + "webpack-dev-middleware/client?apply=hmr-only&overlay=false",-// Connect manually when `autoConnect=false`. Accepts the same option keys as +// Connect manually when `connect=false`. Accepts the same option keys as
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
ec9d3156-784a-4916-b1db-0359ac5bc391
📒 Files selected for processing (18)
.changeset/hot-client-apply-and-connect.md.changeset/reconnect-and-timeout-per-transport.mdREADME.mdclient-src/index.jsclient-src/utils/socket-options.jssrc/hot.jssrc/options.check.jssrc/options.jsontest/client-socket-options.test.jstest/e2e/__snapshots__/process-update.test.js.snap.webpack5test/e2e/client.test.jstest/e2e/inject.test.jstest/e2e/live-reload.test.jstest/e2e/process-update.test.jstest/inject-client.test.jstypes/client/index.d.tstypes/client/utils/socket-options.d.tstypes/hot.d.ts
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
Not a breaking change any more: `hot`, `liveReload`, `reload`,
`autoConnect`, `reconnect` and `timeout` all still apply, and are folded
into the two options that replaced them.
hot: true, reload: true -> apply: "hmr"
hot: true, reload: false -> apply: "hmr-only"
hot: false, liveReload: true -> apply: "reload"
hot: false, liveReload: false -> apply: "nothing"
autoConnect: false -> connect: false
reconnect, timeout -> connect: { retries, timeout }
The new option wins when both are set: the other way round, a migration
that sets it and leaves the old name behind would silently not apply. Each
old name warns, and it has to warn in two places — node, when it is set on
`hot.client`, and the browser, when it arrives on the query, since an entry
somebody wired by hand never passes through the node side at all.
The folding happens in the browser rather than in `clientQuery` for the
same reason: a hand-written query has to work, and node cannot translate
one it never sees.
The symmetry test needed to learn about this. The deprecated names are read
from a list rather than written out one by one, so `overrides.hot` does not
appear in the source for it to find; it now collects them from
`LEGACY_OPTIONS`, with an assertion on the length of that list so a
regex that stops matching cannot quietly shrink the comparison.
Five browser tests cover the folding, including the precedence rule and
both warning channels.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
8abbfa0 to
8dd2841
Compare
alexander-akait
changed the title
apply and connectapply and connect, replacing six browser options
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2458 +/- ##
==========================================
+ Coverage 96.28% 96.36% +0.08%
==========================================
Files 23 23
Lines 2449 2476 +27
==========================================
+ Hits 2358 2386 +28
+ Misses 91 90 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟠 Major · Serialize hot.client.connect objects as JSON, or the object form is lost. · utils.js:1489-1491
src/utils.js:1489-1491
🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSerialize
hot.client.connectobjects as JSON, or the object form is lost.
clientQueryrunsString(value)on every key exceptoverlay.
- The schema accepts
hot.client.connect: { retries: 3, timeout: 5000 }.clientQueryturns that object into the query value"[object Object]".- In
client-src/index.js,setOverridescallsJSON.parseon that value. The parse throws.- The fallback sets
options.connect = "[object Object]" !== "false", which istrue.As a result,
retriesandtimeoutset from Node are dropped without a warning. The e2e tests only passconnectthrough a hand-written entry query, so they do not cover this path.🐛 Proposed fix
- if (key !== "overlay") { - query[key] = String(value); - continue; - } + if (key === "connect" && typeof value === "object" && value !== null) { + query.connect = JSON.stringify(value); + continue; + } + + if (key !== "overlay") { + query[key] = String(value); + continue; + }
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
9d239294-69bc-478b-a744-2d0beb9a1f25
📒 Files selected for processing (14)
.changeset/hot-client-apply-and-connect.mdREADME.mdclient-src/index.jsclient-src/utils/socket-options.jssrc/hot.jssrc/options.check.jssrc/options.jsonsrc/utils.jstest/client-socket-options.test.jstest/e2e/client.test.jstest/e2e/live-reload.test.jstest/inject-client.test.jstypes/client/utils/socket-options.d.tstypes/hot.d.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/hot-client-apply-and-connect.md
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
Both are listed as the replacement the six deprecated rows point to, but the table had no row for either, so following those links left you without their values, default or object shape. The `urlPrefix` row still described the parameter as turning `hot` and `liveReload` off, which is the shape it had before `apply` replaced them. Rebasing onto main resolved every conflicting hunk to the legacy side, which also dropped content that happened to share those hunks: these rows, the `urlPrefix` wording, and three tests covering the shapes `connect` takes. Restored, and the table now reads live options first with the six deprecated names together at the end. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
All of these land in one release, so the changelog is read by someone who only ever sees the final names. Three entries described options and page-url parameters as they were partway through: the retry count and silence timeout under their replaced names, a `-liveReload=false` parameter that no longer exists under any name, and a claim that every option has exactly one spelling, which the six kept for a release contradict. The replaced names are noted where they were the subject, since they still work. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
`clientQuery` special-cased `overlay` and ran `String(value)` over everything
else, which was true of every other option until `connect` took an object too.
`{ retries: 3, timeout: 5000 }` became the text `"[object Object]"`; the client
parses `connect` as JSON, the parse threw, and the fallback read the text as a
boolean — so both fields were dropped in silence and the connection ran on the
defaults.
The rule is now the value's shape rather than the option's name: an object goes
over as JSON, `overlay` keeping the encoding its filter functions need. The e2e
tests set `connect` through a hand-written entry query, which skips this path
entirely, so the new one sets it on the middleware and watches for the
shortened watchdog interval actually arriving — it fails on `String(value)`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
|
Both out-of-diff findings handled.
|
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:
bcba2f18-e7dc-45df-a2f3-5449ed6507d6
📒 Files selected for processing (9)
.changeset/client-live-reload.md.changeset/client-options-both-ways.md.changeset/connect-object-from-node.md.changeset/reconnect-and-timeout-per-transport.mdREADME.mdsrc/utils.jstest/client-socket-options.test.jstest/e2e/client.test.jstest/inject-client.test.js
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
| entry's query as `<name>=<value>`, with the same effect, in both places and in | ||
| the page-url parameters below. The last six are the names `apply` and `connect` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
State that the page-URL override applies only to apply. Both passages imply that every client option can be set through a page-URL parameter. The client reads a page-URL override for apply; ?webpack-dev-middleware-connect=false, for example, will not disable the connection. (raw.githubusercontent.com)
README.md#L810-L811: limit “the same effect” tohot.clientand the entry query, then describe theapplypage-URL override separately..changeset/client-options-both-ways.md#L39-L41: make the same distinction between entry-query options and theapplypage-URL parameter.
📍 Affects 2 files
README.md#L810-L811(this comment).changeset/client-options-both-ways.md#L39-L41
1 participant
Two unions on the browser options, six names down to two. Not a breaking change — every old name keeps working and is folded into the one that replaced it, with removal left for a major.
Retargeted to
mainnow that #2457 has merged.applyreplaceshot,liveReloadandreloadNot a taste call — the control flow says they were one decision written three ways.
liveReloadis read only in theelsebranch ofif (hot), andreloadonly insideapplyUpdate, which theifbranch calls. Of eight boolean combinations only four were reachable:hot: true, reload: trueapply: "hmr"hot: true, reload: falseapply: "hmr-only"hot: false, liveReload: trueapply: "reload"hot: false, liveReload: falseapply: "nothing"The page-url parameter gains from it
It could previously only turn something off, so
-hot=falseand-liveReload=falsewere two switches with a gap between them — a page could not ask for an outcome. Now it can:=falseis kept because that is what the old pair meant together, so the habit survives.connectreplacesautoConnect,reconnectandtimeoutfalsedoes not connect on load —setOptionsAndConnect()still does. The per-transport behaviour is unchanged and still lives insocket-options.js, soretriesandtimeoutmean exactly what #2457 made them mean.Compatibility
All six old names apply as before and are folded into the new ones. The new option wins when both are set: the other way round, a migration that sets it and leaves the old name behind would silently not apply.
Each old name warns in two places, which is forced rather than belt-and-braces: in node when it is set on
hot.client, and in the browser when it arrives on the query. An entry somebody wired by hand never passes through the node side, so without the browser warning it would deprecate silently.For the same reason the folding happens in the browser rather than in
clientQuery— node cannot translate a query it never sees.Why this shape
It is what the codebase already does everywhere:
overlay,token,cors,progressandtransportare each a scalar that may be an object. Neither of these reads as an exception.Notes for review
overrides.hotis not in the source for its regex to find. It collects them fromLEGACY_OPTIONSnow, with an assertion on that list's length so a regex that stops matching cannot quietly shrink the comparison./@property \{[^}]+\}/stops at the brace inside{ retries?: number }, soconnectlooked absent from the typedef. It matches one level of nesting now.hot=falsewith an unrecognisedlive-reload=false; with one option that scenario cannot exist, so it now asserts the property it was really about — an unrecognised value leaves the default in force.maincost more than it looked like it did. I resolved all eight conflicting hunks to the legacy side, which was right for the lines in conflict and wrong for the new lines that happened to sit in the same hunks: the README rows forapplyandconnect, the rewordedurlPrefixrow, and three tests covering the shapesconnecttakes. CodeRabbit caught two of the three; the tests were only found by auditing the branch against its pre-rebase commit. All restored infc5195e, and the client options table now reads live options first with the six deprecated names together at the end.1f37f40). They all land in one release, so the changelog is read by someone who only ever sees the final names — one entry even advertised a?webpack-dev-middleware-liveReload=falseparameter that no longer exists under any name.-hot=falseto-apply=nothing, but-hot=falseleft live reload on, so the build still reached the page, as a reload.nothingis the one mode where it does not arrive at all, and the test sat waiting for text that would never change. Mechanically renaming a query string is not safe when the options being replaced encoded a decision rather than a value.Verified
On
e92977b:npm run lint— clean (eslint, prettier, cspell,tsc, client types, schema-check)dependabot-auto-merge): the full Test matrix on ubuntu/macos/windows × Node 20/22/24/25, Lint, Client, CodeQL, Analyze, dependency-review, SocketFive browser tests cover the folding itself: each legacy combination, the precedence rule, and both warning channels.
🤖 Generated with Claude Code
https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
Summary by CodeRabbit
New Features
applymodes to control page behavior: apply updates with reload fallback, apply without fallback, reload on changes, or do nothing.connectoption to disable connections or configure retries and timeouts.Bug Fixes
Compatibility
Documentation
Generated by Claude Code