feat: enable isolatedDeclarations #615
Conversation
WalkthroughRefactors TypeScript typings across the repo: converts several value imports to type-only imports, adds explicit return/type annotations (including public APIs), adjusts exports, and enables an isolatedDeclarations compiler option. No runtime logic or control flow changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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 significantly refines the project's TypeScript configuration by enabling Highlights
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 TypeScript's isolatedDeclarations compiler option, which requires explicit type annotations for all exported values to ensure that type declarations can be generated in isolation without cross-file type inference. The changes systematically add explicit return type annotations to functions and variables, separate type imports using import type, and distinguish between type-only and value exports.
Key Changes
- Enabled
isolatedDeclarationsandverbatimModuleSyntaxin tsconfig.json - Added explicit return type annotations to all exported functions and constants
- Separated type imports from value imports using
import typesyntax for better tree-shaking and compliance with isolatedDeclarations
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Enabled isolatedDeclarations and verbatimModuleSyntax compiler options |
| vite.config.ts | Added explicit type annotation to config constant |
| test/utils.ts | Added return type annotations to utility functions |
| test/urllib.options.rejectUnauthorized-false.test.ts | Changed to type-only import for AddressInfo |
| test/options.timing.test.ts | Separated value and type imports |
| test/options.timeout.test.ts | Changed to type-only import for AddressInfo |
| test/fixtures/BufferStream.ts | Added void return types to Transform stream methods |
| test/fetch.test.ts | Separated value and type imports |
| test/diagnostics_channel.test.ts | Changed to type-only import for AddressInfo |
| test/HttpClient.test.ts | Separated value and type imports |
| src/utils.ts | Added return type annotations to utility functions and changed to type-only imports |
| src/symbols.ts | Added explicit type annotation to symbols constant and moved default export to end |
| src/index.ts | Separated value exports from type exports, added explicit type to urllib constant |
| src/fetch.ts | Separated value and type imports, added return type annotations to methods, added explicit type to fetch export |
| src/diagnosticsChannel.ts | Changed to type-only import and added void return type |
| src/HttpClient.ts | Separated value and type imports, added explicit types to constants, channels object, and return types |
| src/HttpAgent.ts | Separated value and type imports |
| src/FormData.ts | Added return type annotation to method |
| src/BaseAgent.ts | Changed to type-only import |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
commit: |
![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 the isolatedDeclarations TypeScript compiler option, which improves build performance and ensures better module isolation. The changes systematically add explicit types to functions, methods, and constants across the codebase, and convert imports and exports to be type-only where appropriate. This has been done correctly and consistently. The changes in tsconfig.json to enable isolatedDeclarations and verbatimModuleSyntax are appropriate. The refactoring of default exports in files like src/index.ts, src/symbols.ts, and vite.config.ts to use typed constants is a good pattern to satisfy the new compiler requirements. Overall, this is a solid and well-executed refactoring that improves the project's TypeScript configuration.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #615 +/- ##
==========================================
+ Coverage 93.93% 93.96% +0.02%
==========================================
Files 11 14 +3
Lines 1467 1474 +7
Branches 318 321 +3
==========================================
+ Hits 1378 1385 +7
Misses 85 85
Partials 4 4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
test/fixtures/BufferStream.ts (1)
14-71: Explicitvoidreturn types for stream methods look goodAdding
: voidtorealloc,_transform, and_flushmatches their existing behavior and theTransformcontract, improving declaration emit without changing runtime behavior. A future cleanup could narrowcallbacktoTransformCallbackinstead ofany, but that’s optional.If you decide to tighten the callback type, please verify against the Node
stream.Transformtypings for the Node version you target.src/symbols.ts (1)
1-45: Explicitly typedsymbolsmap improves clarityDefining a named
symbolsconstant with an explicit{ ...: symbol }type and exporting it as default keeps runtime behavior the same while making the available keys clearer at the type level, which helps with declaration emit. The only tradeoff is keeping the type shape and object literal in sync if new symbols are added, which is acceptable here.If you later expand this set, consider using
as constplustypeof symbols(orsatisfies) to reduce duplication between the type and the value.src/fetch.ts (2)
107-109: Type inconsistency:setDispatcheracceptsAgentbutgetDispatcherreturnsDispatcher.The parameter type
Agentis more restrictive than the return type ofgetDispatcher()which isDispatcher. This also differs fromHttpClient.setDispatcher(dispatcher: Dispatcher)which accepts anyDispatcher.Consider using
Dispatcherfor consistency:- setDispatcher(dispatcher: Agent): void { + setDispatcher(dispatcher: Dispatcher): void { this.#dispatcher = dispatcher; }
294-296: Same type inconsistency in static method.Apply the same fix for consistency with the instance method and
HttpClient:- static setDispatcher(dispatcher: Agent): void { + static setDispatcher(dispatcher: Dispatcher): void { FetchFactory.#instance.setDispatcher(dispatcher); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/BaseAgent.ts(1 hunks)src/FormData.ts(1 hunks)src/HttpAgent.ts(1 hunks)src/HttpClient.ts(8 hunks)src/diagnosticsChannel.ts(2 hunks)src/fetch.ts(6 hunks)src/index.ts(5 hunks)src/symbols.ts(2 hunks)src/utils.ts(7 hunks)test/HttpClient.test.ts(1 hunks)test/diagnostics_channel.test.ts(1 hunks)test/fetch.test.ts(1 hunks)test/fixtures/BufferStream.ts(2 hunks)test/options.timeout.test.ts(1 hunks)test/options.timing.test.ts(1 hunks)test/urllib.options.rejectUnauthorized-false.test.ts(1 hunks)test/utils.ts(3 hunks)tsconfig.json(1 hunks)vite.config.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/index.ts (3)
src/HttpClient.ts (3)
request(249-251)T(258-817)HEADER_USER_AGENT(115-115)src/Request.ts (1)
RequestURL(12-12)src/Response.ts (1)
HttpClientResponse(67-79)
src/utils.ts (3)
src/index.ts (2)
FixJSONCtlChars(127-127)SocketInfo(132-132)src/Request.ts (1)
FixJSONCtlChars(15-15)src/Response.ts (1)
SocketInfo(5-23)
⏰ 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: Agent
- GitHub Check: Test (windows-latest, 24)
- GitHub Check: Test (windows-latest, 22)
🔇 Additional comments (26)
test/urllib.options.rejectUnauthorized-false.test.ts (1)
4-4: Type-only import forAddressInfois appropriate
AddressInfois only used in a type assertion, so switching toimport typecleanly removes any unnecessary runtime dependency and works well withverbatimModuleSyntax/isolatedDeclarations.src/BaseAgent.ts (1)
5-25:FetchOpaquetype-only import matches its usage
FetchOpaqueis only referenced in type positions (generics and property types), so converting it toimport typeis safe and aligns withverbatimModuleSyntax/isolatedDeclarations. Thedispatchlogic and AsyncLocalStorage usage remain unchanged.Please double-check other modules importing
FetchOpaqueto ensure none rely on it as a runtime value.test/options.timeout.test.ts (1)
4-4: ConsistentAddressInfotype-only importUsing
import type { AddressInfo }matches its purely type-level usage and keeps the test runtime surface minimal, consistent with the other H2 tests.tsconfig.json (1)
9-11: EnablingisolatedDeclarations+verbatimModuleSyntaxis coherent with the typing cleanupThese options match the rest of the PR (type-only imports, explicit typings) and should improve declaration emit correctness. The main follow-up is ensuring all remaining type-only usages are marked with
import typeand that no file depends on TS rewriting/removing value imports.Please run a full
tscbuild (including declaration emit) to confirm there are no remaining diagnostics triggered specifically by these two options.src/FormData.ts (1)
9-38: Return typestring | undefinedmatches implementationGiven
contentDispositionis conditionally assigned and returned, annotating_getContentDispositionasstring | undefinedprecisely models the behavior and is a good fit for stricter declaration emit.Since this overrides
_FormDatainternals, please confirm the upstreamform-datatype declaration for_getContentDispositionis alsostring | undefined(or a compatible supertype) to avoid override-type mismatches.vite.config.ts (1)
2-16: Typedconfigconstant with default export is a solid patternTyping the Vite/Vitest config as
UserConfigand exporting the named constant as default keeps the external API identical while satisfyingisolatedDeclarationsand leveragingimport typecorrectly.If Vitest updates its
UserConfigshape, please re-run typechecking to ensure this annotation remains compatible withdefineConfig’s return type.src/HttpAgent.ts (1)
2-8: LGTM! Import separation correctly aligns with usage.The imports are correctly split into value and type-only imports based on their usage throughout the file.
src/utils.ts (3)
8-10: LGTM! Type-only imports correctly applied.The conversion to type-only imports for
IncomingHttpHeadersandSocketInfois appropriate as these are only used in type positions.
33-61: LGTM! Return type accurately reflects JSON parsing behavior.The explicit return type
: unknownforparseJSONis appropriate as the function parses arbitrary JSON data whose type cannot be determined statically.
63-67: LGTM! All return types are accurate.The added return types correctly describe the functions' outputs and will enable proper declaration file generation for
isolatedDeclarations.Also applies to: 73-137, 142-147, 149-151, 153-166, 168-203, 221-280
test/HttpClient.test.ts (1)
5-5: LGTM! Test imports correctly use type-only syntax.The type-only imports for
AddressInfoandRawResponseWithMetaare appropriate as these are only used in type assertions within the test file.Also applies to: 12-13
test/diagnostics_channel.test.ts (1)
5-5: LGTM! Type-only import correctly applied.The conversion of
AddressInfoto a type-only import is appropriate for its usage in type assertions.test/options.timing.test.ts (1)
6-7: LGTM! Import split correctly reflects usage patterns.The separation of
urllib(value import) andRawResponseWithMeta(type-only import) correctly matches their runtime vs. type-only usage.src/diagnosticsChannel.ts (2)
6-6: LGTM! Type-only import correctly applied.The conversion of
DiagnosticsChannelto a type-only import is appropriate as it's only used in type annotations throughout the file.
85-85: LGTM! Return type accurately reflects side-effect-only function.The explicit
: voidreturn type is correct forinitDiagnosticsChannelas it only subscribes to diagnostic channels and doesn't return a value.src/index.ts (3)
8-9: LGTM! Type-only imports correctly applied.The conversion to type-only imports for
RequestOptions,RequestURL, andHttpClientResponseis appropriate as these are only used in type positions.
67-89: LGTM! Return types accurately reflect async function behavior.The explicit return types
Promise<HttpClientResponse<T>>correctly describe the async functions' return values and enable proper type inference.
91-148: LGTM! Export structure enhancements improve public API.The changes expand the public API surface by:
- Re-exporting additional undici entities (
Response,Headers,FormData)- Organizing type exports into proper
export typeblocks- Adding a typed default export object
These changes improve type safety and API discoverability while maintaining backward compatibility.
test/utils.ts (1)
8-44: LGTM! All test utility return types are accurate.The explicit return types correctly describe each utility function's output:
readableToBytes:Promise<Buffer>readableToString:Promise<string>createTempfile: Properly typed object with cleanup functionnodeMajorVersion:numberisWindows:booleanThese additions enable proper type checking and declaration generation without changing runtime behavior.
test/fetch.test.ts (1)
8-10: LGTM! Correct separation of value and type-only imports.The diagnostic message types (
FetchDiagnosticsMessage,FetchResponseDiagnosticsMessage,RequestDiagnosticsMessage,ResponseDiagnosticsMessage) are correctly imported as types since they're only used for type annotations. Runtime values (fetch,FetchFactory) remain as value imports.src/HttpClient.ts (4)
2-2: LGTM! Type-only imports are correctly identified.All imported types (
Channel,LookupFunction,FetchOpaque,CheckAddressFunction,RequestURL,RequestOptions,HttpMethod,RequestMeta,RawResponseWithMeta,HttpClientResponse,SocketInfo) are correctly marked as type-only since they're used exclusively for type annotations.Also applies to: 6-6, 27-27, 29-34
43-43: LGTM! Explicit type annotations on exports are accurate.The explicit type annotations for
PROTO_RE,VERSION,HEADER_USER_AGENT, andchannelscorrectly satisfy isolatedDeclarations requirements and match their runtime values.Also applies to: 113-115, 137-147
69-71: LGTM! Helper function return types are accurate.Also applies to: 117-123, 125-127
215-256: LGTM! Method return types are correctly specified.The explicit return types on
getDispatcher(),setDispatcher(),getDispatcherPoolStats(),request<T>(), andcurl<T>()are accurate and properly propagate generic type parameters.src/fetch.ts (2)
13-16: LGTM! Type-only imports are correctly separated from value imports.Also applies to: 24-40
70-101: LGTM! Explicit return types on methods and thefetchexport are accurate.The explicit function type on the
fetchexport correctly satisfies isolatedDeclarations requirements.Also applies to: 103-105, 111-135, 137-143, 290-292, 303-303
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/fetch.ts(6 hunks)src/index.ts(5 hunks)test/utils.ts(3 hunks)tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/index.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/fetch.ts (2)
src/index.ts (8)
ClientOptions(109-109)Dispatcher(98-98)getGlobalDispatcher(100-100)Agent(97-97)FetchFactory(125-125)fetch(125-125)RequestInfo(106-106)Response(102-102)src/HttpClient.ts (2)
ClientOptions(75-111)PoolStat(163-176)
🔇 Additional comments (10)
tsconfig.json (1)
9-12: Public APIs already have explicit type annotations required by isolatedDeclarations.All exported functions, classes, and type definitions across the codebase include explicit type annotations. No changes are needed—the code is ready for this TypeScript 5.5 feature.
test/utils.ts (5)
8-8: LGTM!The explicit return type annotation is correct and aligns with the implementation.
17-17: LGTM!The explicit return type annotation is correct and matches the implementation.
22-24: LGTM!The explicit return type annotation correctly describes the returned object structure.
40-40: LGTM!The explicit return type annotation is correct for the
parseIntresult.
44-44: LGTM!The explicit return type annotation is correct for the boolean comparison result.
src/fetch.ts (4)
4-5: LGTM!The import splitting between value imports and type-only imports is correctly implemented, which supports both isolatedDeclarations and optimal tree-shaking.
Also applies to: 13-14, 16-29
35-51: LGTM!The new exported types (UrllibRequestInit, FetchDiagnosticsMessage, FetchResponseDiagnosticsMessage) properly formalize the API surface and diagnostic message structures for isolatedDeclarations compliance.
59-59: LGTM!All explicit return type annotations on FetchFactory methods are correct and match their implementations.
Also applies to: 92-92, 96-96, 100-100, 126-126, 130-130, 279-279, 283-283
292-292: LGTM!The explicit type annotation on the exported
fetchfunction correctly matches the FetchFactory.fetch implementation and properly uses the UrllibRequestInit interface.
| // https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-5.html#isolated-declarations | ||
| "isolatedDeclarations": true, | ||
| // verbatimModuleSyntax is incompatible with multi-dialect builds | ||
| // "verbatimModuleSyntax": true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check TypeScript version in package.json and installed packages
if [ -f package.json ]; then
echo "=== TypeScript in package.json ==="
jq '.devDependencies.typescript // .dependencies.typescript // "not found"' package.json
fi
echo ""
echo "=== Installed TypeScript version ==="
npm ls typescript 2>/dev/null | head -20 || echo "npm not available or typescript not installed"Repository: node-modules/urllib
Length of output: 189
Update TypeScript version to 5.5 or later.
The tsconfig.json enables isolatedDeclarations, which requires TypeScript 5.5 or later. However, package.json specifies TypeScript ^5.0.4, which does not meet this requirement. Update the TypeScript version in package.json to at least ^5.5.0 to ensure type-checking will succeed.
🤖 Prompt for AI Agents
In package.json (update needed because tsconfig.json lines 9-12 enable
"isolatedDeclarations" which requires TS >=5.5), the project currently depends
on TypeScript ^5.0.4; update the devDependency (or dependency) entry for
"typescript" to at least ^5.5.0 (or a compatible newer version) in package.json,
run npm/yarn/pnpm install to refresh lockfile, and verify tsc --version and a
full type-check to ensure the new compiler supports isolatedDeclarations.
[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
✏️ Tip: You can customize this high-level summary in your review settings.