Repository navigation
chore: migrate net-stubbing, network, network-tools and socket to @packages/eslint-config - #35173
Merged
Merged
Conversation
…ckages/eslint-config Move @packages/net-stubbing, @packages/network, @packages/network-tools and @packages/socket to the shared ESLint 9 flat config, with jsonConfig so package.json keeps being linted. - Add eslint.config.ts (baseConfig + jsonConfig) to each package and drop .eslintignore and the nested test/.eslintrc.json files. Build output, *.d.ts handling, package-lock.json and tsconfig*.json skips are central, and the tests import their globals from vitest. Two ignores remain: socket's `browser/` build output, which the central ignores don't cover, and net-stubbing's in-place `build-prod` output (lib/**/*.js). - net-stubbing: narrow tsconfig include to lib/**/*.ts. `build-prod` compiles in place with no outDir, so the root `*.ts` glob would emit an eslint.config.js that ESLint 9 picks over the .ts config. - Add @packages/eslint-config, eslint and jiti devDependencies, a plain `eslint` lint script and a per-package lint-staged key. The first `--fix` reordered package.json top-level keys, as jsonConfig expects. - Add the four packages to the root lint:fix scope. - ban-ts-comment: give every @ts-expect-error a reason and turn @ts-ignore into @ts-expect-error. Five suppressed nothing once checked against a built tree and were deleted (network agent.ts x2 and agent.spec.ts, network-tools cors.ts, socket utils.ts). - socket: socket.io-parser's typings don't declare hasBinary, which our patch adds to the runtime builds only. Widen the namespace type in the spec instead of disabling import-x/namespace. - Tick the packages off in guides/eslint-migration.md, adding network-tools, which the checklist was missing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrAyaJg6bSZMjP63vUEFpy
- socket: drop the comment above the hasBinary type widening. - network: correct two @ts-expect-error reasons in agent.spec.ts so they name the actual cause (an untyped `new Promise`, and createConnection's options type not declaring `_agentKey`). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrAyaJg6bSZMjP63vUEFpy
Five @ts-expect-error directives only covered test data declared too loosely, so type the data and drop them: - socket: give the circular test objects a type that allows the circular property added after creation (socket.spec.ts x3, utils.spec.ts x1). - network: give the `new Promise` in agent.spec.ts the response type the test reads, instead of letting it resolve to unknown. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrAyaJg6bSZMjP63vUEFpy
…socket # Conflicts: # guides/eslint-migration.md # package.json
- network agent.spec.ts: `_agentKey` is declared as Optional<symbol>; its void member is what can't index the session cache. - net-stubbing handle-intercept-request.ts: the error is the headers mismatch (IncomingHttpHeaders allows undefined values). - socket cdp-socket.ts and cdp-browser.ts: the async emit returns a Promise where the base class's emit returns boolean or this. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JumBwZ8434duzGtAh57S8a
…g errors - Type the three test packets as socket.io-parser's Packet, which already declares `attachments`, so resetting it needs no suppression. - Fake `window` with vi.stubGlobal rather than assigning to `global`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JumBwZ8434duzGtAh57S8a
AGENTS.md asks for an explanation on every `as` assertion that stays. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JumBwZ8434duzGtAh57S8a
…error Add `destroy` with Object.assign and make allowDestroy generic, so it returns the caller's server type plus `destroy` rather than a plain net.Server. Runtime behavior is unchanged: the same method is set on the same server object. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JumBwZ8434duzGtAh57S8a
cypress
|
||||||||||||||||||||||||||||||||||||||||
| Project |
cypress
|
| Branch Review |
eslint/r9-network-socket
|
| Run status |
|
| Run duration | 15m 39s |
| Commit |
|
| Committer | Claude |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
13
|
|
|
1036
|
|
|
0
|
|
|
30094
|
| View all changes introduced in this branch ↗︎ | |
UI Coverage
67.12%
|
|
|---|---|
|
|
24
|
|
|
49
|
Accessibility
98.98%
|
|
|---|---|
|
|
0 critical
3 serious
2 moderate
0 minor
|
|
|
18
|
…socket # Conflicts: # guides/eslint-migration.md # package.json
develop enabled prefer-const in the shared flat config (#35169). The eslint --fix pass turns the never-reassigned `let`s into `const`s. driver-events.ts destructures `value`, which is reassigned, alongside `type`, which is not, so it is split into a `const` and a `let`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JumBwZ8434duzGtAh57S8a
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes 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 eca718b. Configure here.
The root eslintrc turned no-restricted-properties and no-restricted-syntax off for **/test/**, so specs could use sync fs and process.geteuid(). baseConfig enables both everywhere, so carry the exemption over with a test/** override in each package, as config and scaffold-config do. This clears the 49 sync-fs warnings in network's specs, and makes the bare eslint-disable for geteuid in agent.spec.ts unused, so it is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JumBwZ8434duzGtAh57S8a
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
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.

guides/eslint-migration.md.Additional details
This moves
@packages/net-stubbing,@packages/network,@packages/network-toolsand@packages/socketonto the shared flat config (@packages/eslint-config, ESLint 9), following the guide and the earlier batches (#35146, #35148).Per package
eslint.config.tsthat spreadsbaseConfigthenjsonConfig, withtsconfigRootDir: __dirname.package.json: added the@packages/eslint-config,eslintandjitidevDependencies, setlinttoeslintand added thelint-stagedkey. The first--fixwithjsonConfigreordered the top-level keys, as the guide expects..eslintignoreand the nestedtest/.eslintrc.jsonfiles. There wasn't much to port:baseConfigalready covers the test globals, and the old ignore paths either no longer exist or are handled centrally.lint:fixscope and ticked them off in the guide.network-toolswasn't in the checklist yet, so I added it.Package-specific overrides
no-restricted-propertiesandno-restricted-syntaxoff fortest/**. This carries over the root.eslintrc.jsexemption for**/test/**, the same wayconfigandscaffold-configdo, so specs can still use syncfsandprocess.geteuid().socketignoresbrowser/**, the output oftsconfig.browser.json, which the shared config's build-output ignores don't cover.net-stubbingignoreslib/**/*.js, becausebuild-prodcompiles in place. For the same reason itstsconfig.jsoninclude drops the root*.tsglob. Otherwisebuild-prodwould emit aneslint.config.js, and ESLint 9 would pick that up over the.tsconfig.ban-ts-comment(33 sites)@ts-ignorethat still hides a real error is now// @ts-expect-error - <reason>, and each reason names the actual compiler error.@ts-ignores no longer suppressed anything, so I deleted them: two in network'sagent.ts, one in its spec, one in network-tools'cors.tsand one in socket'sutils.ts.Packet-typed test packetsvi.stubGlobal('window', {})new Promisein network'sagent.spec.tsallowDestroy(network) now addsdestroywithObject.assignand is generic, so callers get back their own server type plusdestroyinstead of a plainnet.Server. Runtime behavior is unchanged. It has callers in server, https-proxy and proxy, and all of them still type-check.import-x/namespaceerror on socket'shasBinaryis handled with a narrowed type and a "Safe because" comment. The export comes from our ownpatches/socket.io-parser+4.2.6.patch, which doesn't touch the type declarations.prefer-constdevelopturned onprefer-constin the shared config (chore: enable prefer-const in the shared ESLint flat config #35169) while this PR was open. Merging it surfaced 18 violations in net-stubbing and network.eslint --fixturned 17 never-reassignedlets intoconsts. The last one, in net-stubbing'sdriver-events.ts, destructured a reassignedvaluetogether with a never-reassignedtype, so I split it into aconstand alet.Intentionally left alone
tslintscripts stay, matching howlauncher,proxyandnetwork-interceptionwere migrated.All four packages lint with 0 errors and 0 warnings.
Steps to test
All of these pass locally on the branch with the latest
developmerged in. One exception is@packages/network, which has 54 failures in an unprivileged container. Those are port and proxy errors from the environment, and I compared the run againstdevelop: the failing set is identical, with no new failures. I also rancheck-tsforserver,https-proxyandproxybecause of theallowDestroytype change, along with https-proxy's tests (24/24).How has the user experience changed?
No change. This is lint configuration plus type-only edits.
PR Tasks
cypress-documentation?type definitions?Generated by Claude Code
Note
Low Risk
Tooling and type-only edits in network-related packages; no intended runtime behavior changes beyond lint compliance.
Overview
Moves
@packages/net-stubbing,@packages/network,@packages/network-tools, and@packages/socketonto the shared@packages/eslint-configflat config (ESLint 9): neweslint.config.tsfiles,lint-staged, updatedlintscripts, removal of legacy.eslintignore/test/.eslintrc.json, and inclusion in the rootlint:fixscope. The migration guide marks these packages (andnetwork-tools) complete.Package-specific lint ignores:
net-stubbingskips in-placelib/**/*.jsbuild output and tightenstsconfig.jsonsoeslint.config.tsis not emitted beside sources;socketignoresbrowser/**frombuild:browser.Lint and
ban-ts-commentcleanup across the four packages:@ts-ignore→ documented@ts-expect-error, removal of stale suppressions, stronger test typings, and small formatting fixes.allowDestroyin@packages/networknow attachesdestroyviaObject.assignwith a generic return type (runtime behavior unchanged; callers in server/https-proxy/proxy still type-check).Reviewed by Cursor Bugbot for commit 6a38528. Bugbot is set up for automated code reviews on this repo. Configure here.