chore: enable oxfmt #602
chore: enable oxfmt #602
Conversation
|
Warning Rate limit exceeded@fengmk2 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 53 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (6)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRepository-wide formatting, CI, and tooling updates plus small API typing and one-loop behavior change: CI workflow normalization and a new typecheck job, oxfmt config and scripts added, pnpm lockfile now versioned, many import/format adjustments, one early-exit Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Summary of ChangesHello @fengmk2, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing code consistency and maintainability by integrating oxfmt, a new code formatter, into the project's development workflow. The changes include adding oxfmt configuration, updating package.json scripts to incorporate formatting and type-checking commands, and applying oxfmt's stylistic rules across the entire codebase. This initiative aims to standardize the code style, making it easier for developers to read and contribute to the project. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR enables oxfmt as the code formatter for the project, applying consistent formatting across all TypeScript, JavaScript, JSON, and Markdown files. The changes include configuration setup, CI integration, and automatic formatting of the entire codebase.
- New formatter configuration with 120 character line width, single quotes, and automatic import sorting
- CI workflow updated to enforce formatting checks on all pull requests
- Package manager explicitly set to pnpm@10.24.0 with lock file management changes
Reviewed changes
Copilot reviewed 78 out of 82 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
.oxfmtrc.json |
New formatter configuration with import sorting and style rules |
package.json |
Added oxfmt dependency, format scripts, and packageManager field; removed lint from CI script |
.github/workflows/nodejs.yml |
Added dedicated typecheck job with lint, format check, and build validation |
.gitignore |
Removed pnpm-lock.yaml from ignore list to commit lock files |
src/**/*.ts |
Applied formatting: import reordering, consistent spacing, line breaks, trailing commas |
test/**/*.ts |
Applied formatting: import reordering, multi-line function calls, consistent indentation |
examples/**/*.cjs |
Applied formatting: promise chains, arrow functions, consistent spacing |
vite.config.ts, tsconfig.json, etc. |
Applied formatting: single-line arrays, trailing commas in objects |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #602 +/- ##
==========================================
- Coverage 94.95% 93.93% -1.02%
==========================================
Files 11 11
Lines 1387 1467 +80
Branches 316 318 +2
==========================================
+ Hits 1317 1378 +61
- Misses 66 85 +19
Partials 4 4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enables oxfmt and applies its formatting across the codebase. The changes are mostly stylistic and improve code consistency. I've noticed that the ci script in package.json no longer includes a linting step. I've added a comment to suggest re-adding it, along with the new format and type checking scripts, to maintain code quality in the CI pipeline. Overall, the changes look good and align with the goal of introducing a code formatter.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
test/urllib.options.rejectUnauthorized-false.test.ts (1)
63-63: Resolve TypeScript error: unsafe property access on union type.The non-null assertion
!removes null/undefined from the union but doesn't narrowstring | AddressInfo. Whenserver.address()returns a string (Unix domain socket), accessing.portfails. Use an explicit type assertion or type guard.Apply one of these fixes:
Option 1: Type assertion (if you know it will always be AddressInfo):
- const url = `https://localhost:${server.address()!.port}`; + const url = `https://localhost:${(server.address() as AddressInfo).port}`;You'll also need to import
AddressInfo:-import { createSecureServer } from 'node:http2'; +import { createSecureServer } from 'node:http2'; +import type { AddressInfo } from 'node:net';Option 2: Type guard (safer):
+ const address = server.address(); + if (typeof address === 'string') { + throw new Error('Server listening on Unix socket, expected TCP address'); + } - const url = `https://localhost:${server.address()!.port}`; + const url = `https://localhost:${address.port}`;test/fixtures/socket_server.ts (1)
15-15: Remove unused parameter or prefix with underscore.The TypeScript compiler reports that the
reqparameter is declared but never used. Consider either using the parameter or prefixing it with an underscore (_req) to indicate it's intentionally unused.Apply this diff to fix the issue:
- unixSocketServer.on('request', (req, res) => { + unixSocketServer.on('request', (_req, res) => {test/options.timeout.test.ts (1)
84-116: Fix TypeScript strict mode error onserver.address().portand add missing server cleanupThe non-null assertion
!onserver.address()is insufficient in strict mode. Sinceserver.address()returnsstring | AddressInfo | null, andstring(unix domain sockets) has no.portproperty, TypeScript will fail the typecheck. Additionally, the server resource is not closed after the test, which can leave connections open.Apply this fix:
-import { createSecureServer } from 'node:http2'; +import type { AddressInfo } from 'node:net'; +import { createSecureServer } from 'node:http2'; @@ - const url = `https://localhost:${server.address()!.port}`; + const { port } = server.address() as AddressInfo; + const url = `https://localhost:${port}`; await assert.rejects( async () => { await httpClient.request(url, { timeout: 10, }); }, (err: any) => { // console.error(err); assert.equal(err.name, 'HttpClientRequestTimeoutError'); assert.equal(err.message, 'Request timeout for 10 ms'); assert.equal(err.cause.name, 'InformationalError'); assert.equal(err.cause.message, 'HTTP/2: "stream timeout after 10"'); assert.equal(err.cause.code, 'UND_ERR_INFO'); assert.equal(err.res.status, -1); assert(err.res.rt > 10, `actual ${err.res.rt}`); assert.equal(typeof err.res.rt, 'number'); return true; }, ); + server.close(); + await once(server, 'close');The type assertion
as AddressInfois required for strict TypeScript checking since the server is listening on TCP (port 0). Closing the server after the test prevents resource leaks and matches the cleanup pattern used elsewhere in the test suite.test/HttpClient.test.ts (3)
160-160: Fix TypeScript error: Handle null case for server.address().The pipeline reports that
server.address()can returnstring | AddressInfo | null, but the code accesses.portwithout null checking.Apply this diff to fix the type safety issue:
- const url = `https://localhost:${server.address()!.port}`; + const address = server.address(); + if (!address || typeof address === 'string') { + throw new Error('Failed to get server address'); + } + const url = `https://localhost:${address.port}`;
223-223: Fix TypeScript error: Handle null case for server.address().Same issue as Line 160 -
server.address()requires null and type checking.Apply this diff:
- const url = `https://localhost:${server.address()!.port}`; + const address = server.address(); + if (!address || typeof address === 'string') { + throw new Error('Failed to get server address'); + } + const url = `https://localhost:${address.port}`;
414-416: Remove unused parameter.The
addressparameter is declared but never used, triggering a TypeScript warning.Apply this diff:
- checkAddress(address, family) { + checkAddress(_address, family) { return family !== 6; },test/diagnostics_channel.test.ts (1)
220-220: Fix TypeScript error: Handle null case for server.address().The pipeline reports two errors:
server.address()can returnnull- It can return
string | AddressInfo, but the code assumesAddressInfowith aportpropertyApply this diff to fix the type safety issues:
- _url = `https://localhost:${server.address().port}`; + const address = server.address(); + if (!address || typeof address === 'string') { + throw new Error('Failed to get server address'); + } + _url = `https://localhost:${address.port}`;src/diagnosticsChannel.ts (1)
34-54: Fix localsymbolsshadowing the importedsymbolsmodule informatSocket.Inside
formatSocket, the localconst symbols = Object.getOwnPropertySymbols(socket);shadows the importedsymbolsmodule. As a result, expressions likesocket[symbols.kSocketLocalAddress]use the localsymbol[]instead of the symbol constants module, solocalAddress/localPortwill always resolve viasocket[undefined]and stayundefinedin logs.Renaming the local variable avoids the shadowing and restores correct logging of symbol-backed properties:
function formatSocket(socket: SocketExtend) { if (!socket) return socket; if (!kSocketReset) { - const symbols = Object.getOwnPropertySymbols(socket); - for (const symbol of symbols) { + const socketSymbols = Object.getOwnPropertySymbols(socket); + for (const symbol of socketSymbols) { if (symbol.description === 'reset') { kSocketReset = symbol; break; } } } return { localAddress: socket[symbols.kSocketLocalAddress], localPort: socket[symbols.kSocketLocalPort],test/fixtures/server.ts (2)
15-32: Fix TS7053 onreq.socket[requestsPerSocket]by using a typed socket alias.The CI errors on Lines 24 and 30 come from indexing
Socketwith aunique symbol. A small local typing change fixes both the type error and clarifies intent:const requestsPerSocket = Symbol('requestsPerSocket'); export async function startServer(options?: { keepAliveTimeout?: number; https?: boolean; }): Promise<{ server: Server; url: string; urlWithDns: string; closeServer: any }> { let server: Server; const requestHandler = async (req: IncomingMessage, res: ServerResponse) => { const startTime = Date.now(); - req.socket[requestsPerSocket] = (req.socket[requestsPerSocket] || 0) + 1; + const socket = req.socket as Socket & { [requestsPerSocket]?: number }; + socket[requestsPerSocket] = (socket[requestsPerSocket] ?? 0) + 1; @@ - res.setHeader('x-requests-persocket', req.socket[requestsPerSocket]); - res.setHeader('x-requests-socket-port', req.socket.remotePort!); + res.setHeader('x-requests-persocket', socket[requestsPerSocket] ?? 0); + res.setHeader('x-requests-socket-port', socket.remotePort!);The
connections: Socket[] = []andserver.on('connection', (connection) => { ... })part is already correctly typed and works well with this change.Also applies to: 432-459
317-355: Add explicit types forresult.filesandresult.formto resolve TS7053.
result.files[name] = ...andresult.form[name] = valcurrently index into{}-typed objects, triggering TS7053 in strict mode. Givingresultan explicit shape with string index signatures fixes both errors and documents the structure:- const result = { - method: req.method, - url: req.url, - href: urlObject.href, - headers: req.headers, - files: {}, - form: {}, - }; + const result: { + method: string | undefined; + url: string | undefined; + href: string; + headers: IncomingMessage['headers']; + files: Record< + string, + { + filename: string; + encoding: string; + mimeType: string; + size: number; + } + >; + form: Record<string, any>; + } = { + method: req.method, + url: req.url, + href: urlObject.href, + headers: req.headers, + files: {}, + form: {}, + }; @@ - result.files[name] = { + result.files[name] = { filename, encoding, mimeType, size, }; @@ - result.form[name] = val; + result.form[name] = val;This keeps runtime behavior identical while satisfying the compiler and making the multipart response structure explicit.
🧹 Nitpick comments (1)
src/HttpClient.ts (1)
82-87: UpdatecheckAddressJSDoc to reflect the actual function signature.
CheckAddressFunctionis defined as(ip: string, family: number | string, hostname: string) => boolean, but the comment here says it receives “two arguments (ip and family)”. Consider updating the description to mention the thirdhostnameparameter so users don’t miss it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (81)
.github/workflows/nodejs-16.yml(2 hunks).github/workflows/nodejs.yml(1 hunks).github/workflows/release.yml(1 hunks).gitignore(0 hunks).oxfmtrc.json(1 hunks).oxlintrc.json(2 hunks)README.md(1 hunks)examples/h2-other-side-closed-exit-0-fetch.cjs(2 hunks)examples/h2-other-side-closed-exit-0.cjs(2 hunks)examples/httpclient.cjs(1 hunks)examples/longruning.cjs(1 hunks)examples/search_github.cjs(1 hunks)examples/timing.cjs(1 hunks)package.json(4 hunks)renovate.json(1 hunks)scripts/replace_urllib_version.js(1 hunks)src/BaseAgent.ts(1 hunks)src/FetchOpaqueInterceptor.ts(1 hunks)src/FormData.ts(2 hunks)src/HttpAgent.ts(2 hunks)src/HttpClient.ts(16 hunks)src/HttpClientError.ts(1 hunks)src/IncomingHttpHeaders.ts(1 hunks)src/Request.ts(3 hunks)src/Response.ts(1 hunks)src/diagnosticsChannel.ts(6 hunks)src/fetch.ts(6 hunks)src/index.ts(2 hunks)src/utils.ts(6 hunks)test/HttpClient.connect.rejectUnauthorized.test.ts(2 hunks)test/HttpClient.events.test.ts(5 hunks)test/HttpClient.test.ts(8 hunks)test/diagnostics_channel.test.ts(3 hunks)test/esm/index.js(1 hunks)test/fetch-head-request-should-keepalive.test.ts(1 hunks)test/fetch.test.ts(6 hunks)test/fixtures/server.ts(9 hunks)test/fixtures/socket_server.ts(2 hunks)test/fixtures/ts-cjs-es2021/tsconfig.json(1 hunks)test/fixtures/ts-esm/tsconfig.json(1 hunks)test/fixtures/ts/tsconfig.json(1 hunks)test/formData-with-BufferStream.test.ts(1 hunks)test/head-request-should-keepalive.test.ts(1 hunks)test/index.test.ts(7 hunks)test/keep-alive-header.test.ts(4 hunks)test/mts/src/index.mts(1 hunks)test/mts/tsconfig.json(1 hunks)test/non-ascii-request-header.test.ts(2 hunks)test/options.auth.test.ts(1 hunks)test/options.compressed.test.ts(3 hunks)test/options.content.test.ts(1 hunks)test/options.data.test.ts(16 hunks)test/options.dataType.test.ts(3 hunks)test/options.digestAuth.test.ts(1 hunks)test/options.dispatcher.test.ts(3 hunks)test/options.files.test.ts(8 hunks)test/options.fixJSONCtlChars.test.ts(2 hunks)test/options.followRedirect.test.ts(3 hunks)test/options.gzip.test.ts(1 hunks)test/options.headers.test.ts(1 hunks)test/options.method.test.ts(1 hunks)test/options.opaque.test.ts(2 hunks)test/options.reset.test.ts(1 hunks)test/options.retry.test.ts(1 hunks)test/options.signal.test.ts(2 hunks)test/options.socketErrorRetry.test.ts(3 hunks)test/options.socketPath.test.ts(1 hunks)test/options.stream.test.ts(3 hunks)test/options.streaming.test.ts(3 hunks)test/options.timeout.test.ts(3 hunks)test/options.timing.test.ts(1 hunks)test/options.type.test.ts(1 hunks)test/options.writeStream.test.ts(3 hunks)test/response-charset-gbk.test.ts(1 hunks)test/urllib.options.allowH2.test.ts(1 hunks)test/urllib.options.rejectUnauthorized-false.test.ts(1 hunks)test/user-agent.test.ts(1 hunks)test/utils.test.ts(1 hunks)test/utils.ts(1 hunks)tsconfig.json(1 hunks)vite.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- .gitignore
🧰 Additional context used
🧬 Code graph analysis (17)
test/options.streaming.test.ts (3)
test/cjs/index.js (1)
assert(2-2)src/utils.ts (1)
isReadable(153-166)test/utils.ts (1)
readableToBytes(8-15)
test/HttpClient.connect.rejectUnauthorized.test.ts (1)
src/HttpClient.ts (1)
HttpClient(180-811)
examples/h2-other-side-closed-exit-0-fetch.cjs (1)
src/index.ts (2)
setGlobalDispatcher(95-95)Agent(93-93)
test/options.timeout.test.ts (1)
src/HttpClientError.ts (1)
HttpClientRequestTimeoutError(16-23)
test/options.dataType.test.ts (1)
test/utils.ts (1)
nodeMajorVersion(38-40)
test/HttpClient.events.test.ts (1)
test/cjs/index.js (1)
assert(2-2)
src/diagnosticsChannel.ts (1)
src/utils.ts (1)
performanceTime(149-151)
test/keep-alive-header.test.ts (1)
test/cjs/index.js (1)
assert(2-2)
test/fetch.test.ts (3)
src/HttpClient.ts (2)
RequestDiagnosticsMessage(142-146)ResponseDiagnosticsMessage(148-154)src/index.ts (4)
RequestDiagnosticsMessage(111-111)ResponseDiagnosticsMessage(112-112)FetchFactory(130-130)fetch(130-130)src/fetch.ts (6)
FetchDiagnosticsMessage(47-50)FetchResponseDiagnosticsMessage(52-58)FetchFactory(60-297)fetch(141-284)fetch(294-296)fetch(299-299)
examples/h2-other-side-closed-exit-0.cjs (1)
src/index.ts (2)
setGlobalDispatcher(95-95)Agent(93-93)
src/HttpClient.ts (3)
src/HttpAgent.ts (1)
CheckAddressFunction(8-8)src/Request.ts (2)
RequestURL(12-12)RequestOptions(19-164)src/utils.ts (2)
isReadable(153-166)digestAuthHeader(73-137)
src/Request.ts (1)
src/index.ts (1)
Request(97-97)
examples/search_github.cjs (1)
examples/httpclient.cjs (1)
urllib(11-11)
src/fetch.ts (2)
src/HttpClient.ts (1)
request(242-244)src/index.ts (1)
request(66-79)
test/options.compressed.test.ts (2)
test/cjs/index.js (1)
assert(2-2)examples/httpclient.cjs (1)
urllib(11-11)
src/HttpAgent.ts (2)
src/index.ts (1)
CheckAddressFunction(125-125)src/BaseAgent.ts (1)
BaseAgentOptions(7-9)
test/options.opaque.test.ts (2)
test/cjs/index.js (1)
assert(2-2)examples/httpclient.cjs (1)
urllib(11-11)
🪛 Biome (2.1.2)
.oxlintrc.json
[error] 126-127: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🪛 GitHub Actions: CI
test/options.timeout.test.ts
[error] 96-96: TS2339: Property 'port' does not exist on type 'string | AddressInfo'.
[error] 174-174: TS2339: Property 'name' does not exist on type '{}'.
test/diagnostics_channel.test.ts
[error] 220-220: TS2531: Object is possibly 'null'.
[error] 220-220: TS2339: Property 'port' does not exist on type 'string | AddressInfo'.
test/options.stream.test.ts
[error] 85-85: TS2339: Property 'file' does not exist on type 'FormStream'.
[error] 86-86: TS2339: Property 'field' does not exist on type 'FormStream'.
[error] 92-92: TS2740: Type 'FormStream' is missing the following properties from type 'Readable': readableAborted, readable, readableDidRead, readableEncoding, and 52 more.
[error] 93-93: TS2339: Property 'headers' does not exist on type 'FormStream'.
test/urllib.options.rejectUnauthorized-false.test.ts
[error] 63-63: TS2339: Property 'port' does not exist on type 'string | AddressInfo'.
test/HttpClient.test.ts
[error] 160-160: TS2339: Property 'port' does not exist on type 'string | AddressInfo'.
[error] 223-223: TS2339: Property 'port' does not exist on type 'string | AddressInfo'.
[error] 414-414: TS6133: 'address' is declared but its value is never read.
test/mts/src/index.mts
[error] 1-1: TS2307: Cannot find module 'urllib' or its corresponding type declarations.
[error] 1-1: Cannot find module 'urllib' or its corresponding type declarations.
test/fixtures/server.ts
[error] 24-24: TS7053: Element implicitly has an 'any' type because expression of type 'unique symbol' can't be used to index type 'Socket'. Property '[requestsPerSocket]' does not exist on type 'Socket'.
[error] 30-30: TS7053: Element implicitly has an 'any' type because expression of type 'unique symbol' can't be used to index type 'Socket'. Property '[requestsPerSocket]' does not exist on type 'Socket'.
[error] 338-338: TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{}'. No index signature with a parameter of type 'string' was found on type '{}'.
[error] 348-348: TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{}'. No index signature with a parameter of type 'string' was found on type '{}'.
test/fixtures/socket_server.ts
[error] 15-15: TS6133: 'req' is declared but its value is never read.
🪛 LanguageTool
README.md
[style] ~60-~60: To form a complete sentence, be sure to include a subject.
Context: ...ng - Request method, defaults to GET. Could be GET, POST, DELETE or PUT. Al...
(MISSING_IT_THERE)
[uncategorized] ~61-~61: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...a_** Object - Data to be sent. Will be stringify automatically. - content String...
(AUTO_HYPHEN)
[uncategorized] ~64-~64: Possible missing preposition found.
Context: ...ps://nodejs.org/api/stream.html#stream_class_stream_writable) - A writable stream t...
(AI_HYDRA_LEO_MISSING_TO)
[grammar] ~64-~64: Possible agreement error. The noun ‘phase’ seems to be countable.
Context: ...am.html#stream_class_stream_writable) - A writable stream to be piped by the response...
(CD_NN)
[style] ~66-~66: To form a complete sentence, be sure to include a subject or ‘there’.
Context: ...tType_** String - Type of request data. Could be json (Notes: not use `applicat...
(MISSING_IT_THERE)
[style] ~67-~67: To form a complete sentence, be sure to include a subject.
Context: ...Type_** String - Type of response data. Could be text or json. If it's text, th...
(MISSING_IT_THERE)
[misspelling] ~70-~70: Use ‘every thing’ if you want to emphasize that the things are separate. Usually, you can use “everything”.
Context: ...rs_** Object - Request headers. - timeout Number | Array - Request timeout in milli...
(EVERYTHING)
[uncategorized] ~70-~70: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...timeout: 5000` to tell urllib use same timeout on two phase or set them separately such as ...
(AUTO_HYPHEN)
[misspelling] ~75-~75: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...- maxRedirects Number - The maximum number of redirects to follow, defaults...
(EN_A_VS_AN)
[uncategorized] ~78-~78: Possible missing article found.
Context: ...n - lets you get the res object when request connected, default false. alias `cust...
(AI_HYDRA_LEO_MISSING_A)
[uncategorized] ~81-~81: Loose punctuation mark.
Context: ...t is true. - socketPath String | null - request a unix socket service, ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~81-~81: Loose punctuation mark.
Context: ...String | null - request a unix socket service, default is null. - **_highWate...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (111)
test/options.gzip.test.ts (1)
1-7: Formatting looks good.The blank lines added around import groups align with standard formatting conventions and the oxfmt tool's style. No functional changes have been introduced—the test logic remains intact.
scripts/replace_urllib_version.js (1)
9-9: Formatting changes approved.Both formatting updates align well with oxfmt standards and maintain code clarity:
- Line 9: Single-line array declaration is more compact without sacrificing readability.
- Line 14: Parentheses around arrow function parameters follow modern JavaScript conventions and improve consistency.
The script logic and functionality remain intact across these formatting adjustments.
Also applies to: 14-14
test/formData-with-BufferStream.test.ts (1)
1-9: Clean import organization following standard conventions.The import reorganization groups imports logically (node built-ins → external packages → local modules) with appropriate blank-line separators for improved readability. This aligns well with the oxfmt configuration objectives.
test/urllib.options.rejectUnauthorized-false.test.ts (1)
1-10: Import organization looks good.The imports are properly grouped by source (stdlib, third-party, local) with clear separation, which aligns with oxfmt conventions.
test/options.streaming.test.ts (2)
4-6: LGTM! Consistent import spacing.The blank lines between import groups improve readability and align with the oxfmt formatting standards.
92-96: Formatting looks good, but verify error handling in pipeline callbacks.The multi-line formatting of the pipeline calls improves readability. However, the empty callbacks don't handle potential pipeline errors. In test code, unhandled errors could cause tests to hang or pass incorrectly.
Consider handling errors in the pipeline callbacks:
bytes = await readableToBytes( - pipeline(response.res, decoder, () => { - return; - }), + pipeline(response.res, decoder, (err) => { + if (err) throw err; + }), );This pattern appears in both locations (lines 92-96 and 117-121).
Also applies to: 117-121
test/mts/src/index.mts (2)
4-4: IsAny conditional type change is purely formattingDropping the parentheses in
0 extends 1 & T ? Y : Ndoesn’t change the conditional type’s semantics here;1 & Tis still the right-hand side ofextends. This looks like a safe, style-only tweak aligned with the formatter.Please re-run the new typecheck task over this file to confirm there are no additional TS diagnostics beyond the known import-resolution issue.
1-2: Remove unsupported TS2307 claim; this file is not checked by CI typecheckThe CI workflow runs
pnpm run typecheck(which istsc --noEmitat the root level), but the roottsconfig.jsondoes not includetest/mts/**/*files. Thetest/mts/directory has its owntsconfig.jsonthat is only used when explicitly runningbuild:mts:testortsc -p test/mts/tsconfig.json, neither of which are in the CI pipeline. The referenced TS2307 error does not occur in the actual CI typecheck job.Likely an incorrect or invalid review comment.
test/head-request-should-keepalive.test.ts (1)
1-8: LGTM! Formatting improvements align with oxfmt standards.The added blank lines separate import groups for better readability. No functional changes.
test/options.retry.test.ts (1)
1-9: LGTM! Import grouping improves readability.Blank lines added to separate import groups consistently with the rest of the test suite.
test/fetch-head-request-should-keepalive.test.ts (1)
1-8: LGTM! Consistent import formatting.Formatting aligns with the oxfmt standardization across the test suite.
test/urllib.options.allowH2.test.ts (1)
1-6: LGTM! Formatting improvements.Import spacing standardized per oxfmt configuration.
test/utils.test.ts (1)
1-6: LGTM! Import formatting standardized.Consistent blank line spacing applied.
renovate.json (1)
1-4: LGTM! JSON formatting normalized.The extends array formatting is now single-line, which is cleaner for a single-item array. No semantic change.
test/user-agent.test.ts (1)
1-7: LGTM! Import spacing standardized.Consistent formatting applied across the test suite.
src/Response.ts (1)
1-4: LGTM! Import formatting improved.Blank line added to separate external import from local imports, improving readability.
test/esm/index.js (1)
1-5: Formatting-only change; no functional impact.The blank line added after the initial import improves readability and aligns with oxfmt formatting rules.
test/options.headers.test.ts (1)
1-6: Formatting-only change; no functional impact.Blank lines added to separate import groups by source (built-in, third-party, project imports), improving readability per oxfmt standards.
test/options.method.test.ts (1)
1-6: Formatting-only change; no functional impact.Blank lines added to separate import groups, improving readability and consistency per oxfmt standards.
test/mts/tsconfig.json (1)
2-8: Formatting-only change; no functional impact.Indentation normalized to spaces and trailing commas added to JSON structure, improving consistency per oxfmt formatting standards. Configuration behavior remains unchanged.
test/options.timing.test.ts (1)
1-7: Formatting-only change; no functional impact.Blank lines added to separate import groups by source, improving readability and consistency per oxfmt standards.
test/options.type.test.ts (1)
1-6: Formatting-only change; no functional impact.Blank lines added to separate import groups, improving readability and consistency per oxfmt standards.
test/options.digestAuth.test.ts (1)
1-6: Formatting-only change; no functional impact.Blank lines added to separate import groups, improving readability and consistency per oxfmt standards.
test/options.reset.test.ts (1)
1-6: Formatting-only change; no functional impact.Blank lines added to separate import groups, improving readability and consistency per oxfmt standards.
.oxlintrc.json (1)
6-6: LGTM!The single-line formatting of the extends array is consistent with the new oxfmt configuration.
src/BaseAgent.ts (1)
1-4: LGTM!The import consolidation and spacing align with the new oxfmt formatting rules. No functional changes.
test/options.content.test.ts (1)
4-6: LGTM!The import reordering and spacing are consistent with the new oxfmt formatting configuration.
test/keep-alive-header.test.ts (1)
3-5: LGTM!The formatting improvements enhance readability, particularly for the multi-line assertions. All changes are consistent with the oxfmt configuration.
Also applies to: 93-96, 140-143, 176-176
vite.config.ts (1)
5-5: LGTM!The single-line array formatting is consistent with the oxfmt configuration and maintains readability for these short arrays.
Also applies to: 8-8, 11-11
test/response-charset-gbk.test.ts (1)
2-2: LGTM!The blank lines improve import grouping and align with the new oxfmt formatting rules.
Also applies to: 4-4
examples/timing.cjs (1)
22-30: LGTM!The multi-line formatting with trailing comma improves readability and follows JavaScript best practices.
test/fixtures/ts/tsconfig.json (1)
9-11: LGTM!The trailing comma addition is consistent with TypeScript configuration best practices and the oxfmt formatting rules.
test/options.socketPath.test.ts (1)
1-4: Import spacing change is purely stylisticAdded blank lines between node/vitest/local imports; behavior and test semantics are unchanged. Looks good.
examples/search_github.cjs (1)
5-14: Reformatted promise chain keeps behavior identicalThe multi-line
.request(...).then(...)formatting preserves the same options and logging; no functional change.test/options.signal.test.ts (1)
24-40: assert.rejects blocks reformatted without changing logicBoth abort-related tests still create the same requests, trigger abort, await the same promises, and validate error shape via the custom predicate. Changes are cosmetic only.
Also applies to: 44-61
tsconfig.json (1)
8-9: tsconfig change is formatting-onlyOnly a trailing comma was added to
compilerOptions; no compiler behavior or options were modified.test/options.auth.test.ts (1)
1-4: Import formatting aligns with new styleSpacing between node/vitest/project imports is updated only; the auth test behavior remains identical.
examples/h2-other-side-closed-exit-0-fetch.cjs (1)
3-7: H2 dispatcher and main promise chain are unchanged semanticallyThe multiline
setGlobalDispatcher(new Agent({ allowH2: true }))andmain().then(...).catch(...)are equivalent to the prior single-line forms; runtime behavior is preserved.Also applies to: 26-34
test/options.compressed.test.ts (1)
1-5: Compressed-options tests keep the same assertionsImport spacing, the
assert(!requestHeaders['accept-encoding'], ...)reflow, and bothassert.rejectsblocks for invalid gzip/brotli remain logically equivalent to the previous version; no behavior change.Also applies to: 41-44, 173-190, 194-214
test/options.fixJSONCtlChars.test.ts (1)
1-4: JSON control-character test assertion is structurally refactored onlyThe
assert.rejectscall still exercises the same failing request and validates the same error properties; only formatting and layout changed.Also applies to: 34-49
examples/longruning.cjs (1)
24-32: LGTM! Promise chain formatting looks clean.The multi-line formatting improves readability while preserving the original error handling logic.
test/fixtures/ts-esm/tsconfig.json (1)
9-11: LGTM! Trailing comma added for cleaner diffs.The formatting change follows JSON best practices without affecting the configuration.
test/fixtures/ts-cjs-es2021/tsconfig.json (1)
9-11: LGTM! Consistent formatting with other tsconfig files.Trailing comma added for better maintainability.
examples/h2-other-side-closed-exit-0.cjs (2)
3-7: LGTM! Multi-line formatting improves readability.The setGlobalDispatcher call is now easier to read with one argument per line.
26-34: LGTM! Promise chain formatting is consistent.The error handling logic remains unchanged while improving code clarity.
test/options.dataType.test.ts (3)
2-4: LGTM! Import spacing normalized.Consistent blank line spacing improves code organization.
104-134: LGTM! Test assertion formatting improved.The assert.rejects call is more readable with multi-line formatting while preserving all error validation logic.
147-161: LGTM! Consistent assertion formatting.The test logic remains unchanged with improved readability.
test/options.followRedirect.test.ts (3)
3-5: LGTM! Import spacing normalized.Consistent formatting across test files.
82-82: LGTM! Array formatting consolidated.Single-line format is cleaner for simple arrays.
94-94: LGTM! Consistent array formatting.Matches the formatting style applied throughout the file.
test/non-ascii-request-header.test.ts (2)
2-4: LGTM! Import spacing normalized.Consistent with the formatting applied to other test files.
23-40: LGTM! Test assertion formatting improved.Multi-line formatting enhances readability while preserving the error validation logic.
README.md (1)
59-82: LGTM! Documentation formatting updated consistently.The change from bold to italic emphasis for option names provides a cleaner, more consistent documentation style across the arguments list.
test/options.dispatcher.test.ts (1)
1-7: LGTM - Formatting changes align with oxfmt standardization.Import reordering and blank line additions are consistent with the formatting tool configuration.
test/index.test.ts (2)
1-15: LGTM - Import reordering and formatting standardization.The import reorganization follows the expected pattern: node modules first, then external packages, then local imports with appropriate blank line separators.
207-247: LGTM - Improved mock intercept readability with fluent chaining.The reformatted
mockPool.intercept(...).reply(...)pattern is more readable than the previous style.src/index.ts (1)
90-129: LGTM - Export declarations reformatted for clarity.One export per line improves readability and produces cleaner diffs when exports are added or removed.
test/options.stream.test.ts (1)
111-128: LGTM - assert.rejects formatting standardized.The multi-line callback style for
assert.rejectsis consistent with the formatting applied across other test files.src/FormData.ts (1)
28-35: LGTM - Quote style normalization.The change from escaped single quotes to double quotes produces identical output while improving readability. The Content-Disposition header construction remains correct per RFC 6266.
test/options.writeStream.test.ts (2)
1-12: LGTM - Import reordering and formatting standardization.Import organization follows the consistent pattern established across the test suite.
87-101: LGTM - assert.rejects formatting consistent with test suite.The multi-line callback style aligns with the formatting applied across other test files in this PR.
test/options.files.test.ts (2)
1-11: LGTM - Import reordering follows formatting standards.The import organization is consistent with the patterns established across the test suite.
43-56: LGTM - assert.rejects formatting standardized.The multi-line callback style is consistent with other test files in this PR.
src/Request.ts (3)
2-5: LGTM! Import reorganization aligns with oxfmt.The import statements have been reorganized for consistency. The changes maintain the same functionality while improving code style.
44-50: LGTM! Multiline formatting improves readability.The
filesoption type has been reformatted to a clearer multiline structure, making the union type easier to read without changing its behavior.
176-176: LGTM! Consistent type annotation punctuation.The trailing semicolon aligns with TypeScript best practices and the formatting standards being applied across the codebase.
test/fixtures/socket_server.ts (2)
7-10: LGTM! Consistent type annotation style.The type annotation has been updated to use semicolons instead of commas, aligning with the formatting standards being applied across the codebase.
21-27: LGTM! Consistent arrow function parameter formatting.The explicit parentheses around arrow function parameters improve consistency and align with the formatting standards.
test/options.data.test.ts (1)
1-555: LGTM! Consistent formatting improvements throughout the file.The formatting changes improve readability by:
- Adding appropriate blank lines after imports
- Reformatting multiline assertions and object literals for clarity
- Standardizing array and object spacing
All changes are non-functional and align with the oxfmt formatting standards being applied across the codebase.
test/fetch.test.ts (2)
4-10: LGTM! Import organization improvements.The imports have been consolidated and reorganized for better clarity, including the addition of the
Requestimport from undici which is used later in the test.
30-139: LGTM! Consistent formatting for arrow functions and arrays.The changes standardize:
- Arrow function parameters with explicit parentheses
- Array spacing to be more compact
- Multiline assertion formatting for better readability
All changes maintain the same functionality while improving code consistency.
src/utils.ts (3)
1-8: LGTM! Improved import organization.The imports have been reorganized to:
- Group related imports (Blob and File from node:buffer)
- Make explicit the import of IncomingHttpHeaders
- Improve overall clarity
All changes maintain the same functionality.
49-165: LGTM! Consistent multiline formatting.The multiline formatting of error messages and the
isReadablereturn statement improves readability without changing behavior. These changes align with the formatting standards being applied across the codebase.
207-279: LGTM! Minor spacing and formatting adjustments.The spacing adjustments in loops and function definitions maintain consistency with the formatting standards. All changes are non-functional.
src/HttpAgent.ts (3)
3-4: LGTM! Import formatting improvement.The imports have been formatted for better consistency.
13-13: LGTM! Consistent type annotation style.The type annotation has been updated to use a semicolon, aligning with TypeScript best practices and the formatting standards being applied across the codebase.
53-59: LGTM! Improved address validation logic.The changes include:
- Consistent type annotation formatting with semicolons
- Addition of a
breakstatement after detecting an invalid addressThe
breakstatement is a functional improvement that prevents unnecessary iterations once an invalid address is found, making the validation more efficient.package.json (4)
27-29: LGTM! New formatting scripts added.The addition of
fmt,typecheck, andfmtcheckscripts supports the oxfmt formatting initiative. These scripts provide essential tooling for maintaining code quality and consistency.
77-77: LGTM! oxfmt dependency added.The
oxfmtformatter has been added as a dev dependency to support the formatting initiative described in the PR objectives.
119-119: LGTM! Package manager specified.The
packageManagerfield has been added to specify pnpm version 10.24.0, which helps ensure consistent package management across different environments.
46-46: LGTM! CI script updated.The CI script has been appropriately streamlined for its publishing validation purpose. Linting is still covered in the CI pipeline—it runs in the GitHub Actions workflow (
.github/workflows/nodejs.yml) and is also included as part of thetestscript.src/FetchOpaqueInterceptor.ts (1)
2-3: LGTM! Improved import spacing.The addition of a blank line after the import improves readability and aligns with the formatting standards being applied across the codebase.
.github/workflows/release.yml (1)
3-5: Workflow branch filter normalization looks goodSwitching to
branches: [master]is a style-only change and keeps the trigger behavior identical..github/workflows/nodejs-16.yml (1)
3-7: CI trigger and step formatting are consistentStandardizing to
branches: [master]and the minor step formatting change do not affect behavior and keep this workflow aligned with the others.Also applies to: 22-22
src/IncomingHttpHeaders.ts (1)
1-7: Import reordering is safeGrouping the type-only
Exceptimport after the Node built-in import is purely stylistic; theIncomingHttpHeaderstype remains unchanged.test/utils.ts (1)
1-7: Import order cleanup onlyReordering the imports here doesn’t change runtime behavior; the helpers continue to work as before.
src/HttpClientError.ts (1)
1-14: Explicit type import for response metadata is correctImporting
RawResponseWithMetaandSocketInfoas types fromResponse.jsmatches their usage onHttpClientRequestErrorand keeps runtime dependencies unchanged..oxfmtrc.json (1)
1-18: oxfmt configuration is coherent with repo styleThe schema reference, print width, single-quote preference, ignore patterns, and import-sorting groups all look consistent with the formatting changes in source and test files.
test/HttpClient.connect.rejectUnauthorized.test.ts (1)
28-41: Reformattedassert.rejectsblocks preserve behaviorThe reshaped
assert.rejectscalls still assert onerr.name,err.message,err.code, anderr.res.statusas before. Only style changed; behavior remains correct.Also applies to: 47-60
test/options.timeout.test.ts (6)
3-8: New imports for HTTP/2 and timeout error type are appropriateImporting
createSecureServerandHttpClientRequestTimeoutErrormatches their usage in the h2 timeout test and the last timeout test.
29-49: HeadersTimeout test formatting change is safeReflowing this
assert.rejectsblock doesn’t alter the checks onerr.name,err.message,err.cause.*, anderr.restiming; behavior is unchanged.
52-73: String timeout test remains valid after reformatThe test still verifies that a string
timeout: '10'is handled correctly and produces the expectedHttpClientRequestTimeoutErrorwithHeadersTimeoutErroras the cause.
120-138: BodyTimeout test reformat is behavior‑preservingThe
assert.rejectsblock still checks forHttpClientRequestTimeoutErrorwith optionalBodyTimeoutErrorcause and status200. Only formatting changed.
142-157: Array timeout[400, 500]test remains unchanged semanticallyThe refactored
assert.rejectsblock continues to assert the correct timeout message, status, andBodyTimeoutErrorcause when present.
160-177: Change callback parameter type toanyto fix TS2339 error on line 174The callback in
assert.rejectstries to accesserr.cause.name, butHttpClientRequestTimeoutErrordoesn't explicitly declare acauseproperty. Match the pattern used in all other timeout tests in this file by changing the parameter type:- (err: HttpClientRequestTimeoutError) => { + (err: any) => { // console.log(err); assert.equal(err.name, 'HttpClientRequestTimeoutError'); assert.equal(err.message, 'Request timeout for 100 ms'); assert.equal(err.res!.status, -1); assert(err.headers); assert.equal(err.status, -1); err.cause && assert.equal(err.cause.name, 'HeadersTimeoutError'); return true; },.github/workflows/nodejs.yml (1)
11-46: LGTM! Excellent addition of comprehensive pre-test checks.The new
typecheckjob adds valuable quality gates including linting, dependency deduplication checks, TypeScript type checking, formatting verification, and build validation before running tests. This will catch issues earlier in the CI pipeline.test/HttpClient.events.test.ts (1)
1-135: LGTM! Formatting improvements for consistency.The changes apply consistent formatting across the file, including standardized arrow function parameter syntax and improved readability of assertion blocks. No behavioral changes.
test/options.opaque.test.ts (1)
1-54: LGTM! Consistent formatting applied.Formatting improvements align with the repository-wide oxfmt standardization. No functional changes.
test/options.socketErrorRetry.test.ts (1)
1-98: LGTM! Formatting standardization applied.Consistent formatting improvements throughout the file. No behavioral changes to the test logic.
examples/httpclient.cjs (1)
1-28: LGTM! Formatting standardization.Adds consistent spacing after the
functionkeyword throughout the file. No functional changes.test/HttpClient.test.ts (1)
1-29: LGTM! Formatting improvements.The PerformanceObserver callback and array formatting updates improve readability. These are formatting-only changes.
test/diagnostics_channel.test.ts (1)
1-15: LGTM! Import organization improvements.The import reordering and separation of type imports improve code organization. These are formatting-only changes.
src/fetch.ts (3)
1-38: LGTM! Import organization improvements.The import consolidation and reorganization improve code structure and readability. No functional changes.
115-116: LGTM! Improved loop clarity.The explicit destructuring in the for-of loop makes the code more readable. The type assertion formatting is also clearer.
263-270: LGTM! Debug call formatting improvement.Breaking the debug call across multiple lines improves readability without changing behavior.
src/diagnosticsChannel.ts (2)
2-3: TypedSocketusage and connect error instrumentation look consistent.Importing
Socketfromnode:net, annotatingSocketExtend, and wiringSocket.prototype.destroyto attachsymbols.kErrorSocketpreserves existing behavior and improves type safety. The updatedundici:client:connectErrorhandler, includingsocket: SocketExtendin the cast and using the sameSocketExtendfor the fallback fromerror[symbols.kErrorSocket], is coherent and keeps the error/socket correlation clear.Also applies to: 59-68, 122-154
108-120: Structured debug logging withformatSocketis a good improvement.The multi-line
debug(...)calls for request creation, header sending, and response headers, plus switching toformatSocket(socket)/formatSocket(sock)instead of logging the raw socket, make diagnostics more readable and avoid dumping huge socket objects while still surfacing key connection metadata.Also applies to: 186-205, 233-253
test/fixtures/server.ts (1)
68-86: Formatting changes to JSON responses, headers, and server lifecycle look good.The multi-line
res.end(JSON.stringify(...))calls, normalized digest authwww-authenticate/x-www-authenticateheaders, properSet-Cookiecasing for multiple cookies, and the Promise-wrappedstartServer/closeServerstructure all preserve behavior while improving readability and consistency.Also applies to: 88-119, 121-152, 163-185, 203-207, 317-357, 442-464
src/HttpClient.ts (2)
216-238: Improved typing for undici pool stats looks correct.Casting
clientsfromundiciSymbols.kClientstoMap<string, WeakRef<Pool>> | undefinedand normalizingrefviatypeof ref.deref === 'function' ? ref.deref() : ref(then checking bothpool.statsandpool.dispatcher.stats) safely handles both the older and undici@v7 shapes. UsingPoolStatwithsatisfiesis a nice touch to keep the mapping structurally checked.
251-257: Request pipeline, upload handling, redirects, and digest auth changes are semantically sound.The refactoring of
#requestInternalto multi-line parameters, thefiles/FormDataupload handling usingFormStreamcompatibility, the nested querystring support viaqs, the more detailed debug logs (including timing and socket info), the digest-auth retry block, and the socket-error retry debug are all consistent with the existing behavior and improve readability without introducing new logic.Also applies to: 462-512, 526-562, 575-587, 596-625, 643-651, 712-719, 751-757, 771-775
commit: |
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/fixtures/server.ts (2)
1-32: startServer wiring, per-socket tracking, and HTTPS/closeServer behavior all look soundImports (including
Socket, compression helpers, andselfsigned), thestartServeroptions/return type, per-socket request counting via aSymbol, and the HTTPS server creation/closeServerhelper are consistent with Node’s HTTP APIs and preserve the previous behavior while keeping Node 16+ connection shutdown logic intact. If you ever want tighter typing, you could narrow thecloseServerproperty fromanyto() => Promise<void>in the return type, but it’s not required for these test fixtures.Also applies to: 414-460
318-357: Multipart and form-url-encoded parsing remain correct; typing tweaks are fine but could be strongerThe
/multipartbusboy setup still only records file metadata plus fields and responds with the aggregatedresult, and theapplication/x-www-form-urlencodedbranch still buildsrequestBodyfrom eitherqs.parseorURLSearchParamsas before. The newfiles/formassertions toRecord<string, any>are acceptable for tests; optionally, you could introduce explicit interfaces for file entries and form fields to avoidanyand catch shape mismatches at compile time.Also applies to: 371-386
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.github/workflows/nodejs.yml(1 hunks)src/formstream.d.ts(1 hunks)test/HttpClient.test.ts(9 hunks)test/diagnostics_channel.test.ts(4 hunks)test/fixtures/server.ts(9 hunks)test/fixtures/socket_server.ts(1 hunks)test/mts/src/index.mts(1 hunks)test/options.stream.test.ts(4 hunks)test/options.timeout.test.ts(3 hunks)test/urllib.options.rejectUnauthorized-false.test.ts(2 hunks)tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- test/options.timeout.test.ts
- .github/workflows/nodejs.yml
- test/options.stream.test.ts
- test/HttpClient.test.ts
- tsconfig.json
🧰 Additional context used
🧬 Code graph analysis (2)
test/diagnostics_channel.test.ts (2)
test/cjs/index.js (1)
assert(2-2)examples/httpclient.cjs (1)
urllib(11-11)
test/fixtures/server.ts (1)
test/fixtures/socket_server.ts (1)
startServer(6-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test (windows-latest, 22)
- GitHub Check: Test (macos-latest, 18)
- GitHub Check: Test (windows-latest, 18)
🔇 Additional comments (8)
test/mts/src/index.mts (2)
1-3: Import and top-level await with@ts-expect-errorlook correctDirective syntax is valid with a message, import style matches the formatter, and using top-level
awaitin this.mtstest context is appropriate. No issues from a typing or runtime perspective here.
5-5:IsAnyconditional type change is a no-op and remains canonicalDropping the parentheses in
0 extends 1 & Tdoesn’t change semantics; the helper still behaves as intended in the assertions below. Good cleanup.test/urllib.options.rejectUnauthorized-false.test.ts (1)
4-7: Typed port extraction fromserver.address()looks correctUsing
AddressInfoforserver.address()and interpolatinghttps://localhost:${(server.address() as AddressInfo).port}after awaiting the'listening'event is type-safe and matches the expected Nodehttp2server typings; behavior is unchanged and clearer than a non-null assertion.Also applies to: 11-11, 64-64
test/diagnostics_channel.test.ts (3)
4-9: Import re-org and type-only import are consistent and safeReordered imports, separating the
import type { … }from the value import and addingAddressInfo, align with modern TS + Node style and don’t change runtime behavior.Also applies to: 11-11, 15-15
220-222: UsingAddressInfofor H2 server URL construction is appropriateAssigning
_url = \https://localhost:${(server.address() as AddressInfo).port}`afterlisten(0)and'listening'` ensures a non-null, correctly typed port from the HTTP/2 secure server; behavior matches the previous implementation while improving typing clarity.
390-414: Updatedassert.rejectsvalidators keep error semantics intactWrapping the
urllib.requestcalls in async functions and using validator callbacks that assert onlastError, error name/message, codes, and socket references (thenreturn true) is a correct use ofassert.rejects; it preserves the original expectations while making the checks more explicit and thorough.Also applies to: 443-464
test/fixtures/server.ts (1)
69-208: Auth JSON handlers and cookie behavior are preserved despite formatting changesThe
/auth,/hello/json, and various digest auth endpoints still emit the same JSON payloads andwww-authenticate/x-www-authenticateheader values, and changing/set-two-cookieto use'Set-Cookie'casing remains HTTP-equivalent since Node normalizes header names, so these edits are effectively formatting-only.test/fixtures/socket_server.ts (1)
6-30: Unix-socket startServer formatting and typing adjustments look goodThe semicolon-separated return type, Promise executor parentheses, and renaming the unused
reqparameter to_reqare all stylistic; the server still listens on a unique Unix socket path, responds with the same JSON payload, and exposes acloseServerthat wrapsunixSocketServer.closein a Promise, matching the HTTP fixture’s behavior.
[skip ci] ## 4.9.0 (2025-12-13) * feat: enable isolatedDeclarations (#615) ([374402e](374402e)), closes [#615](#615) [hi#level](https://github.com/hi/issues/level) * chore: enable oxfmt (#602) ([948f05d](948f05d)), closes [#602](#602) [hi#level](https://github.com/hi/issues/level) * chore: enable tsgo on typecheck (#605) ([41f6277](41f6277)), closes [#605](#605) [hi#level](https://github.com/hi/issues/level) * chore: fix auto release ([8f13384](8f13384)) * chore(deps): update actions/checkout action to v6 (#600) ([75685a1](75685a1)), closes [#600](#600) * chore(deps): update actions/setup-node action to v6 (#596) ([d4fd7cf](d4fd7cf)), closes [#596](#596) * chore(deps): update codecov/codecov-action digest to 671740a (#608) ([2734504](2734504)), closes [#608](#608) * chore(deps): update dependency @tsconfig/node18 to v18.2.6 (#603) ([23ffca2](23ffca2)), closes [#603](#603) * chore(deps): update dependency @tsconfig/strictest to v2.0.8 (#604) ([0fc042a](0fc042a)), closes [#604](#604) * chore(deps): update dependency @types/node to v22.19.2 (#610) ([a9ace96](a9ace96)), closes [#610](#610) * chore(deps): update dependency @typescript/native-preview to v7.0.0-dev.20251210.1 (#609) ([dc1ed3e](dc1ed3e)), closes [#609](#609) * chore(deps): update dependency @typescript/native-preview to v7.0.0-dev.20251211.1 (#613) ([8f6fdaa](8f6fdaa)), closes [#613](#613) * chore(deps): update dependency @typescript/native-preview to v7.0.0-dev.20251213.1 (#619) ([11b3bec](11b3bec)), closes [#619](#619) * chore(deps): update dependency cross-env to v10.1.0 (#612) ([0f68029](0f68029)), closes [#612](#612) * chore(deps): update dependency form-data to v4.0.5 (#606) ([ab3d22f](ab3d22f)), closes [#606](#606) * chore(deps): update dependency oxlint to v1.32.0 (#614) ([8118abb](8118abb)), closes [#614](#614) * chore(deps): update dependency typescript to v5.9.3 (#607) ([cda0e13](cda0e13)), closes [#607](#607) * chore(deps): update dependency undici to v7.16.0 (#617) ([ca378bc](ca378bc)), closes [#617](#617) * chore(deps): update pnpm to v10.25.0 (#618) ([708b985](708b985)), closes [#618](#618)
|
🎉 This PR is included in version 4.9.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary by CodeRabbit
New Features
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.