feat: include @types/node type declarations out of the box
#31502
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (2)
⛔ Files ignored due to path filters (2)
You can disable this status message by setting the WalkthroughAdds extensive Node.js TypeScript declaration files (CTS) under Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
| "lib.deno.window.d.ts", | ||
| "lib.deno.worker.d.ts", | ||
| "lib.deno.shared_globals.d.ts", | ||
| "lib.deno.ns.d.ts", |
| } | ||
| } | ||
|
|
||
| fn process_node_types(out_dir: &Path) { |
There was a problem hiding this comment.
We should see about extracting the libs out to a separate crate so it can be done in parallel (I haven't measured if it's worth it yet though).
There was a problem hiding this comment.
I measured it and it was 873ms. It's probably worth extracting out to a separate crate after this lands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (5)
cli/tsc/dts/node/module.d.cts (1)
214-214: Minor typo in JSDoc comment.The comment says "f you want" instead of "If you want". This appears to be inherited from upstream
@types/node. Not blocking, but could be fixed if you're maintaining a fork.- * @param parentURL f you want to resolve `specifier` relative to a base + * @param parentURL If you want to resolve `specifier` relative to a basecli/tsc/dts/node/diagnostics_channel.d.cts (1)
349-354: Missing generic arguments onimplements TracingChannelCollection.The class should specify the generic parameters for proper type checking:
- class TracingChannel<StoreType = unknown, ContextType extends object = {}> implements TracingChannelCollection { + class TracingChannel<StoreType = unknown, ContextType extends object = {}> implements TracingChannelCollection<StoreType, ContextType> {Without the generic arguments, the channel properties may not be correctly typed relative to the class's own type parameters.
cli/tsc/dts/node/https.d.cts (1)
404-495: Gitleaks “generic API key” hits here are benign documentation examplesThe base64 strings and fingerprints in this JSDoc block are example values lifted from upstream HTTPS docs, not real credentials. If these are tripping CI, consider suppressing this path or these patterns in your Gitleaks configuration rather than changing the docs.
cli/tsc/dts/node/fs/promises.d.cts (1)
94-95: TODO comment for missing EventEmitter close functionality.There's a TODO indicating that EventEmitter close support is missing from the FileHandle interface. This may be important for proper resource cleanup event handling.
Do you want me to help implement the EventEmitter close event typings for FileHandle, or open an issue to track this?
cli/tsc/dts/node/compatibility/iterators.d.cts (1)
8-20: Keep the empty iterator placeholder interfaces as interfaces; adjust lint instead
IteratorObjectandAsyncIteratorObjectare intentionally empty placeholders so they can merge with the real TS ≥5.6 iterator interfaces when present. Changing them to type aliases (as Biome suggests) would break declaration merging and can conflict with the built‑in lib definitions.I’d leave these as interfaces and instead suppress
noEmptyInterfacefor this file (or this pattern), optionally adding a short comment noting that they’re mergeable shims for TS <5.6.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (15)
tests/specs/check/check_node_builtin_modules/mod.js.outis excluded by!**/*.outtests/specs/check/check_node_builtin_modules/mod.js.tsgo.outis excluded by!**/*.outtests/specs/check/check_node_builtin_modules/mod.ts.outis excluded by!**/*.outtests/specs/check/check_node_builtin_modules/mod.ts.tsgo.outis excluded by!**/*.outtests/specs/check/check_npm_install_diagnostics/main.outis excluded by!**/*.outtests/specs/check/check_npm_install_diagnostics/npm_install_diagnostics/main.outis excluded by!**/*.outtests/specs/check/lockfile_types_node/deno.lockis excluded by!**/*.locktests/specs/check/unknown_builtin_node_module/check.outis excluded by!**/*.outtests/specs/check/uses_types_node_pkg/check.outis excluded by!**/*.outtests/specs/check/uses_types_node_pkg/deno.lockis excluded by!**/*.locktests/specs/check/uses_types_node_pkg/node_modules/@types/node/index.d.tsis excluded by!**/node_modules/**tests/specs/check/uses_types_node_pkg/node_modules/@types/node/package.jsonis excluded by!**/node_modules/**tests/specs/publish/bare_node_builtins/bare_node_builtins.outis excluded by!**/*.outtests/specs/run/cts/cjs_import_cts/check.outis excluded by!**/*.outtests/specs/run/node_prefix_missing/node_globals_check.outis excluded by!**/*.out
📒 Files selected for processing (45)
.dprint.json(1 hunks)cli/build.rs(3 hunks)cli/factory.rs(0 hunks)cli/graph_util.rs(0 hunks)cli/lsp/config.rs(0 hunks)cli/lsp/diagnostics.rs(0 hunks)cli/lsp/documents.rs(0 hunks)cli/lsp/language_server.rs(2 hunks)cli/lsp/resolver.rs(1 hunks)cli/lsp/tsc.rs(2 hunks)cli/npm.rs(0 hunks)cli/schemas/config-file.v1.json(2 hunks)cli/tools/repl/session.rs(1 hunks)cli/tsc/00_typescript.js(3 hunks)cli/tsc/dts/lib.node.d.ts(1 hunks)cli/tsc/dts/node/LICENSE(1 hunks)cli/tsc/dts/node/README.md(1 hunks)cli/tsc/dts/node/assert.d.cts(1 hunks)cli/tsc/dts/node/assert/strict.d.cts(1 hunks)cli/tsc/dts/node/async_hooks.d.cts(1 hunks)cli/tsc/dts/node/buffer.buffer.d.cts(1 hunks)cli/tsc/dts/node/buffer.d.cts(1 hunks)cli/tsc/dts/node/child_process.d.cts(1 hunks)cli/tsc/dts/node/cluster.d.cts(1 hunks)cli/tsc/dts/node/compatibility/iterators.d.cts(1 hunks)cli/tsc/dts/node/console.d.cts(1 hunks)cli/tsc/dts/node/constants.d.cts(1 hunks)cli/tsc/dts/node/dgram.d.cts(1 hunks)cli/tsc/dts/node/diagnostics_channel.d.cts(1 hunks)cli/tsc/dts/node/dns.d.cts(1 hunks)cli/tsc/dts/node/dns/promises.d.cts(1 hunks)cli/tsc/dts/node/dom-events.d.cts(1 hunks)cli/tsc/dts/node/domain.d.cts(1 hunks)cli/tsc/dts/node/events.d.cts(1 hunks)cli/tsc/dts/node/fs/promises.d.cts(1 hunks)cli/tsc/dts/node/globals.d.cts(1 hunks)cli/tsc/dts/node/globals.typedarray.d.cts(1 hunks)cli/tsc/dts/node/http.d.cts(1 hunks)cli/tsc/dts/node/https.d.cts(1 hunks)cli/tsc/dts/node/index.d.cts(1 hunks)cli/tsc/dts/node/module.d.cts(1 hunks)cli/tsc/dts/node/net.d.cts(1 hunks)cli/tsc/dts/node/os.d.cts(1 hunks)cli/tsc/dts/node/path.d.cts(1 hunks)cli/tsc/dts/node/perf_hooks.d.cts(1 hunks)
💤 Files with no reviewable changes (6)
- cli/lsp/diagnostics.rs
- cli/factory.rs
- cli/npm.rs
- cli/lsp/config.rs
- cli/graph_util.rs
- cli/lsp/documents.rs
🧰 Additional context used
📓 Path-based instructions (3)
cli/tools/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
CLI tools should be implemented in
cli/tools/<tool>orcli/tools/<tool>/mod.rs
Files:
cli/tools/repl/session.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: For debugging Rust code, set breakpoints in IDE debuggers (VS Code with rust-analyzer, IntelliJ IDEA) or uselldbdirectly
Useeprintln!()ordbg!()macros for debug prints in Rust code
Files:
cli/tools/repl/session.rscli/build.rscli/lsp/tsc.rscli/lsp/resolver.rscli/lsp/language_server.rs
⚙️ CodeRabbit configuration file
Don't worry about coverage of Rust docstrings. Don't be nitpicky about it. Leave it to the author's judgement if such a documentation is necessary.
Files:
cli/tools/repl/session.rscli/build.rscli/lsp/tsc.rscli/lsp/resolver.rscli/lsp/language_server.rs
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: For JavaScript runtime debugging, enable V8 inspector with--inspect-brkflag and connect Chrome DevTools tochrome://inspect
Useconsole.log()for debug prints in JavaScript runtime code
Files:
cli/tsc/00_typescript.jscli/tsc/dts/lib.node.d.ts
🧠 Learnings (3)
📚 Learning: 2025-11-20T09:12:20.198Z
Learnt from: marvinhagemeister
Repo: denoland/deno PR: 31358
File: ext/node/polyfills/process.ts:759-766
Timestamp: 2025-11-20T09:12:20.198Z
Learning: In ext/node/polyfills/process.ts, the process.sourceMapsEnabled property should be defined with enumerable: true to match Node.js behavior (as seen in Node.js source: lib/internal/bootstrap/node.js).
Applied to files:
cli/tsc/dts/node/compatibility/iterators.d.ctscli/tsc/dts/node/index.d.ctscli/tsc/dts/node/module.d.cts
📚 Learning: 2025-11-24T16:19:37.808Z
Learnt from: CR
Repo: denoland/deno PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:19:37.808Z
Learning: Applies to cli/module_loader.rs : Module loading and resolution is handled in `cli/module_loader.rs`
Applied to files:
cli/lsp/language_server.rs
📚 Learning: 2025-11-24T16:19:37.808Z
Learnt from: CR
Repo: denoland/deno PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:19:37.808Z
Learning: Applies to **/*.{ts,tsx,js} : Use `console.log()` for debug prints in JavaScript runtime code
Applied to files:
cli/tsc/dts/node/console.d.cts
🧬 Code graph analysis (19)
cli/tsc/dts/node/cluster.d.cts (1)
ext/node/polyfills/net.ts (1)
Socket(1190-1278)
cli/tsc/dts/node/domain.d.cts (1)
ext/node/polyfills/domain.ts (1)
Domain(37-131)
cli/tsc/dts/node/dom-events.d.cts (1)
cli/tsc/dts/node/undici/patch.d.ts (5)
EventListener(26-28)EventListenerObject(22-24)AddEventListenerOptions(14-18)EventListenerOptions(10-12)EventInit(4-8)
cli/tsc/dts/node/http.d.cts (3)
ext/node/polyfills/_http_agent.mjs (1)
oncreate(315-327)ext/node/polyfills/net.ts (1)
Socket(1190-1278)ext/node/polyfills/http.ts (3)
ClientRequest(2292-2292)METHODS(2295-2295)maxHeaderSize(2288-2288)
cli/lsp/tsc.rs (2)
cli/tsc/99_main_compiler.js (1)
specifier(188-188)cli/util/path.rs (1)
specifier(73-77)
cli/lsp/resolver.rs (3)
ext/node/ops/v8.rs (1)
None(312-312)tests/util/server/src/lsp.rs (1)
None(989-989)cli/lsp/performance.rs (1)
None(259-259)
cli/tsc/dts/node/console.d.cts (1)
ext/node/polyfills/internal/console/constructor.mjs (1)
ignoreErrors(322-322)
cli/tsc/dts/node/child_process.d.cts (5)
cli/tsc/dts/node/events.d.cts (1)
Abortable(503-508)cli/tsc/dts/node/buffer.d.cts (1)
BufferEncoding(240-240)cli/tsc/dts/node/fs.d.cts (1)
ObjectEncodingOptions(39-41)ext/node/polyfills/internal/errors.ts (1)
ErrnoException(154-160)ext/node/polyfills/child_process.ts (4)
execFile(427-670)fork(71-186)execSync(731-748)execFileSync(813-835)
cli/tsc/dts/node/buffer.buffer.d.cts (1)
cli/tsc/dts/node/buffer.d.cts (4)
WithImplicitCoercion(234-237)BufferEncoding(240-240)NonSharedBuffer(124-124)AllowSharedBuffer(124-124)
cli/tsc/dts/node/async_hooks.d.cts (2)
ext/node/polyfills/async_hooks.ts (5)
asyncId(40-42)createHook(231-244)AsyncResource(29-93)AsyncLocalStorage(95-150)asyncWrapProviders(163-228)ext/node/polyfills/internal/async_hooks.ts (1)
AsyncHook(345-451)
cli/tsc/dts/node/fs/promises.d.cts (2)
cli/tsc/dts/node/fs.d.cts (6)
WatchEventType(3334-3334)Mode(44-44)OpenMode(43-43)ReadPosition(2578-2578)ObjectEncodingOptions(39-41)PathLike(30-30)cli/tsc/dts/node/buffer.d.cts (1)
BufferEncoding(240-240)
cli/tsc/dts/node/perf_hooks.d.cts (1)
ext/node/polyfills/async_hooks.ts (1)
AsyncResource(29-93)
cli/tsc/dts/node/https.d.cts (5)
cli/tsc/dts/node/http2.d.cts (1)
ServerOptions(1342-1347)ext/node/polyfills/_tls_wrap.js (1)
TLSSocket(81-295)ext/node/polyfills/internal/crypto/x509.ts (1)
issuer(164-166)ext/node/polyfills/stream.ts (1)
Duplex(171-171)ext/node/polyfills/http.ts (1)
ClientRequest(2292-2292)
cli/tsc/dts/node/net.d.cts (4)
ext/node/polyfills/internal/errors.ts (1)
ErrnoException(154-160)ext/node/polyfills/net.ts (7)
BlockList(2758-2758)Socket(1190-1278)SocketAddress(2758-2758)getDefaultAutoSelectFamily(1867-1869)setDefaultAutoSelectFamily(1872-1875)getDefaultAutoSelectFamilyAttemptTimeout(1878-1880)setDefaultAutoSelectFamilyAttemptTimeout(1883-1891)cli/tsc/dts/node/buffer.d.cts (1)
BufferEncoding(240-240)cli/tsc/dts/node/events.d.cts (1)
Abortable(503-508)
cli/tsc/dts/node/dns/promises.d.cts (1)
cli/tsc/dts/node/dns.d.cts (1)
TlsaRecord(295-300)
cli/tsc/dts/node/dgram.d.cts (3)
ext/node/polyfills/internal/dgram.ts (1)
SocketType(36-36)cli/tsc/dts/node/events.d.cts (1)
Abortable(503-508)ext/node/polyfills/internal/errors.ts (1)
ErrnoException(154-160)
cli/tsc/dts/node/buffer.d.cts (2)
ext/node/polyfills/internal/buffer.mjs (3)
isUtf8(2921-2941)isAscii(2943-2963)transcode(2965-3005)ext/node/polyfills/crypto.ts (1)
BinaryLike(443-443)
cli/tsc/dts/node/events.d.cts (1)
ext/node/polyfills/async_hooks.ts (1)
AsyncResource(29-93)
cli/tsc/dts/node/assert.d.cts (1)
ext/node/polyfills/assert.ts (14)
ok(1020-1020)notEqual(1018-1018)deepEqual(1007-1007)notDeepEqual(1016-1016)strictEqual(1023-1023)notStrictEqual(1019-1019)deepStrictEqual(1008-1008)notDeepStrictEqual(1017-1017)throws(1024-1024)doesNotThrow(1011-1011)ifError(1014-1014)rejects(1021-1021)doesNotReject(1010-1010)doesNotMatch(1009-1009)
🪛 Biome (2.1.2)
cli/tsc/dts/node/compatibility/iterators.d.cts
[error] 8-9: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 9-10: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
cli/tsc/dts/node/perf_hooks.d.cts
[error] 915-915: Shouldn't redeclare 'PerformanceObserver'. Consider to delete it or rename it.
'PerformanceObserver' is defined here:
(lint/suspicious/noRedeclare)
[error] 925-925: Shouldn't redeclare 'PerformanceObserverEntryList'. Consider to delete it or rename it.
'PerformanceObserverEntryList' is defined here:
(lint/suspicious/noRedeclare)
[error] 935-935: Shouldn't redeclare 'PerformanceResourceTiming'. Consider to delete it or rename it.
'PerformanceResourceTiming' is defined here:
(lint/suspicious/noRedeclare)
cli/tsc/dts/node/buffer.d.cts
[error] 1893-1893: Shouldn't redeclare 'atob'. Consider to delete it or rename it.
'atob' is defined here:
(lint/suspicious/noRedeclare)
[error] 1909-1909: Shouldn't redeclare 'btoa'. Consider to delete it or rename it.
'btoa' is defined here:
(lint/suspicious/noRedeclare)
🪛 Gitleaks (8.30.0)
cli/tsc/dts/node/https.d.cts
[high] 483-483: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 486-486: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 489-489: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (10)
- GitHub Check: test release linux-x86_64
- GitHub Check: build libs
- GitHub Check: test debug linux-x86_64
- GitHub Check: test debug windows-x86_64
- GitHub Check: test debug macos-aarch64
- GitHub Check: test debug linux-aarch64
- GitHub Check: test debug macos-x86_64
- GitHub Check: lint debug windows-x86_64
- GitHub Check: lint debug macos-x86_64
- GitHub Check: lint debug linux-x86_64
🔇 Additional comments (70)
cli/tsc/dts/node/README.md (1)
1-3: Documentation for generated Node type declarations is clear.The README appropriately documents the folder's purpose and regeneration process. The instruction to run
./tools/update_types_node.tsaligns with the PR's build script and follows standard conventions for auto-generated content.cli/tsc/dts/node/path.d.cts (1)
1-200: LGTM - Comprehensive and accurate path module type definitions.The type definitions correctly cover the entire Node.js path API including all methods, properties, and submodules. The structure properly supports both bare (
"path") and prefixed ("node:path") imports, as well as posix/win32 variants. The CommonJS export syntax is appropriate for the.d.ctsformat.cli/tsc/dts/node/module.d.cts (1)
1-893: Bundled@types/nodemodule declarations look correct.This declaration file provides comprehensive TypeScript types for the Node.js
moduleAPI, including the Module class, hook types for customization, SourceMap support, and proper global augmentations for CommonJS globals (__dirname,__filename,require, etc.) andImportMeta. The re-export pattern fornode:moduleis correctly implemented.This addresses the linked issue #30963 by ensuring Node.js types (including
processfrom other bundled files) are available during type checking without explicit imports.cli/tsc/dts/node/console.d.cts (1)
1-452: LGTM! Well-structured Node.js console type declarations.The file correctly defines types for both
"console"and"node:console"modules with comprehensive method signatures and JSDoc. The global augmentation pattern properly avoids conflicts with DOM types, constructor overloads match Node.js API, and version references align with @types/node@24.2.0.cli/tsc/dts/node/child_process.d.cts (1)
1-74: Well-structured Node.js type declarations.The file provides comprehensive TypeScript declarations for the
node:child_processmodule with thorough JSDoc documentation, proper event emitter overloads, and correct handling of encoding-based return type variations. The module augmentation pattern for supporting both"child_process"and"node:child_process"imports is correctly implemented.Also applies to: 1547-1549
cli/tsc/dts/node/perf_hooks.d.cts (2)
1-101: LGTM!The module documentation is comprehensive, and the foundational types (EntryType, NodeGCPerformanceDetail, PerformanceEntry) are well-defined with appropriate readonly modifiers and clear JSDoc comments.
900-944: LGTM! Static analysis warnings are false positives.The global augmentation pattern correctly provides Node.js-specific types to the global scope while respecting existing web platform types. The conditional type logic at lines 915-939 checks for web worker/browser context and falls back to Node.js implementations when needed. The static analysis warnings about redeclaration can be safely ignored as this is the standard pattern for Node.js type declarations.
cli/tsc/dts/node/cluster.d.cts (3)
1-59: LGTM!The module documentation is comprehensive, and the imports are appropriate for the cluster module functionality.
60-129: LGTM!The ClusterSettings and Address interfaces are well-defined with appropriate types and comprehensive documentation.
573-579: LGTM!The default export and
node:clusterre-export module declarations follow the correct pattern.cli/tsc/dts/node/diagnostics_channel.d.cts (3)
416-421: Return type inconsistency: JSDoc saysboolean, signature saysvoid.Same issue as
Channel.unsubscribe- the documentation says it returnstrueif all handlers were successfully unsubscribed, but the signature returnsvoid.This appears to be an upstream
@types/nodeinconsistency.
576-578: Module augmentation looks correct.Standard pattern for re-exporting under the
node:prefix.
1-26: Comprehensive type declarations fordiagnostics_channel.The bundled types provide complete coverage of the Node.js diagnostics_channel API including channels, tracing channels, store bindings, and all associated methods. Documentation is thorough and aligned with Node.js API docs.
cli/tsc/dts/node/domain.d.cts (2)
87-87: The type signature is correct and matches the official Node.js Domain API specification. The filedomain.d.ctsis a TypeScript declaration file (.d.ctsextension), which contains only type definitions—not runtime implementations. Declaration files describe types for compiled JavaScript code elsewhere. There is no implementation-signature mismatch to address.
25-165: Thedispose()method is correctly omitted.Node.js removed the
dispose()method as part of deprecation DEP0012. The method is no longer part of the domain API, so its absence from type declarations is intentional and accurate. These type definitions correctly reflect the current Node.js domain module specification.Likely an incorrect or invalid review comment.
cli/tsc/dts/node/http.d.cts (2)
2034-2042: Verify undici types are accessible via relative import path.Lines 2034-2042 import WebSocket and event types from
"./undici/index.d.ts"using a relative path. Confirm this file exists in the build output and is properly referenced.
203-238: Thehintsproperty exists on Node.jsdns.LookupOptionsas an optional number type, making the indexed type accessLookupOptions["hints"]valid. No issue exists with this line.Likely an incorrect or invalid review comment.
cli/tsc/dts/node/async_hooks.d.cts (4)
1-16: LGTM on file structure and documentation.The overall structure is well-organized with comprehensive JSDoc documentation. The warning about preferring alternatives to
async_hooks(lines 2-6) and the standard re-export pattern fornode:async_hooks(lines 621-623) follow Node.js conventions correctly.Also applies to: 621-623
263-322: Verify AsyncResource implementation matches type declarations.The type declarations indicate
AsyncResourceacceptstriggerAsyncIdas a constructor parameter and provides atriggerAsyncId()method (lines 273, 321). Confirm these are implemented inext/node/polyfills/async_hooks.ts. If the implementation is incomplete, code that type-checks will fail at runtime.
448-452: Verify AsyncLocalStorage.name property is implemented in runtime.The type declaration includes a
readonly name: stringproperty (since v24.0.0), but this needs verification against the runtime implementation inext/node/polyfills/async_hooks.tsto ensure the property is defined and accessible as expected.
561-619: Verify asyncWrapProviders constants alignment with Node.js runtime.The type declarations include provider constants that may not align with Node.js's actual async_hooks implementation. Constants like
QUIC_ENDPOINT,QUIC_LOGSTREAM,QUIC_PACKET,QUIC_SESSION,QUIC_STREAM, andQUIC_UDPare documented Node.js async_hooks providers but may need verification against the actual runtime list. Additionally,FIXEDSIZEBLOBCOPYappears to be embedder-specific (e.g., Bun) and its inclusion in Node.js type declarations warrants review.cli/tsc/dts/node/https.d.cts (1)
6-545: HTTPS typings align with existing node lib patternsThe module declarations, generics (
ServerOptions,Server<Request, Response>), and overloads forcreateServer,request, andgetare consistent with the surroundinghttp/http2node typings in this repo. I don’t see structural or compatibility issues in the surface you’ve modeled here..dprint.json (1)
38-38: LGTM!Excluding the node types directory from formatting is appropriate for vendored type declarations.
cli/tsc/dts/node/LICENSE (1)
1-21: LGTM!The MIT license with Microsoft copyright is appropriate for bundling the @types/node declarations.
cli/tools/repl/session.rs (1)
857-898: LGTM!The removal of synthetic
@types/nodeinjection is correct since node types are now bundled by default. The simplified logic only installs npm packages when there are actual npm imports.cli/build.rs (3)
124-124: LGTM!Adding
lib.node.d.tsto the compression list is consistent with other library declarations.
137-200: LGTM!The
process_node_typesfunction correctly:
- Discovers all
.tsand.ctsfiles recursively- Sorts paths for deterministic builds
- Conditionally compresses for release builds
- Generates the macro with paths relative to
tsc/dts- Handles Windows path separators
268-270: LGTM!Initializing
out_direarly and callingprocess_node_typesunconditionally is correct. The macro needs to be generated for both debug and release builds.cli/schemas/config-file.v1.json (2)
244-244: LGTM!Adding
"node"to the defaultlibachieves the PR objective of including Node.js type declarations by default. Users can still opt out by providing a customlibarray.
273-274: LGTM!Adding
"node"to the examples list helps users discover this option.cli/tsc/00_typescript.js (3)
11434-11434: LGTM - API export for cross-module usage.The export enables the function to be called from the program creation logic in hunk 3.
127436-127442: LGTM - deduplication logic is correct.The early return when
@types/nodeis detected prevents duplicate type definitions. The iteration through all file paths only occurs once perlib.node.d.tsprocessing, so performance impact is acceptable.
11589-11591: Verify path separator handling on Windows.The hardcoded forward slash in
path.includes("/@types/node/")may fail to detect@types/nodeon Windows systems that use backslashes, causinglib.node.d.tsto be injected even when@types/nodeis present. Consider normalizing paths to use forward slashes or use a platform-agnostic comparison method.cli/tsc/dts/node/fs/promises.d.cts (3)
1268-1280: LGTM - glob function overloads are well-typed.The glob function overloads correctly handle the different return types based on options (Dirent vs string) and pattern types.
1282-1284: LGTM - node:fs/promises re-export correctly implemented.The re-export pattern for the
node:prefixed module is correctly implemented.
300-303: Typo in JSDoc comment.Line 302 contains a typo: "objectsAdd commentMore actions" should be "objects".
- * the `hostname`. On success, the `Promise` is resolved with an array of objectsAdd commentMore actions + * the `hostname`. On success, the `Promise` is resolved with an array of objectsLikely an incorrect or invalid review comment.
cli/tsc/dts/node/dgram.d.cts (4)
1-27: LGTM - Module documentation and structure.The module documentation with usage example is helpful, and the source reference link is a nice addition for users wanting to see the implementation.
28-64: LGTM - Interface definitions are complete.
RemoteInfo,BindOptions,SocketType, andSocketOptionsinterfaces are well-defined and match the Node.js dgram API. TheSocketOptionscorrectly extendsAbortablefor abort signal support.
546-595: LGTM - Event listener typings are comprehensive.The typed event listener overloads provide excellent type safety for the dgram Socket events (close, connect, error, listening, message). The
Symbol.asyncDisposesupport for async disposal is a nice modern addition.
597-599: LGTM - node:dgram re-export correctly implemented.cli/tsc/dts/node/dns/promises.d.cts (2)
458-499: LGTM - Resolver class is comprehensive.The Resolver class properly declares all instance methods matching the module-level functions, plus the
cancel()andsetLocalAddress()methods specific to resolver instances.
501-503: LGTM - node:dns/promises re-export correctly implemented.cli/tsc/dts/node/dom-events.d.cts (1)
1-77: File appears truncated - missing global augmentation.The comment at line 77 states "Merge conditional interfaces into global scope, and conditionally declare global constructors" but no global declaration block follows. The conditional type aliases
__Event,__CustomEvent, and__EventTargetare defined but never used in a global augmentation.Typically this pattern requires a
declare global { ... }block that utilizes these conditional types to merge into the global scope.cli/tsc/dts/node/buffer.buffer.d.cts (1)
1-463: Missingnode:buffermodule re-export may require verification.The file
buffer.buffer.d.ctsmay be missing a re-export for thenode:buffermodule alias. If other declaration files in this project include re-exports like:declare module "node:buffer" { export * from "buffer"; }then this file should include the equivalent pattern to ensure imports like
import { Buffer } from "node:buffer"resolve correctly.cli/tsc/dts/lib.node.d.ts (1)
1-4: LGTM!Clean entry point for the bundled Node.js type declarations. The
no-default-lib="true"directive and the reference to the index file follow the expected lib pattern.cli/tsc/dts/node/index.d.cts (1)
1-52: LGTM!Good structure with proper licensing, TypeScript version compatibility shims, and comprehensive coverage of Node.js modules. The lib references for es2020, esnext.disposable, and esnext.float16 are appropriate.
Also applies to: 55-94
cli/lsp/language_server.rs (2)
578-584: LGTM!Correctly disables the automatic
@types/nodeversion lookup since Node types are now bundled directly. TheNpmVersionResolverno longer needs to track a types version requirement.
833-843: LGTM!Consistent change in
update_global_cache()to match the new bundled types approach. The TODO comment about workspace-specific npm_search_api is noted but unrelated to this PR.cli/tsc/dts/node/assert/strict.d.cts (1)
1-8: LGTM!Standard pattern for Node.js type declarations, correctly providing both the bare
"assert/strict"and"node:assert/strict"module paths that re-export from the main assert module.cli/lsp/tsc.rs (2)
4728-4737:op_is_node_filenow correctly classifies bundled node assetsNormalizing the path then treating anything under
asset:///node/as a node file in addition toin_node_moduleslooks appropriate for the new bundled node libs, and the error path still safely returnsfalse. No issues spotted.
4746-4755: FilteringLAZILY_LOADED_STATIC_ASSETSbyis_libis soundRestricting
op_libsto entries wherevalue.is_libis true avoids exposing non-lib assets while preserving the existing lib name normalization. This is a clean way to surface the new"node"lib without pulling in unrelated assets.cli/tsc/dts/node/globals.typedarray.d.cts (1)
1-21: NodeJS.TypedArray / ArrayBufferView augmentation looks correctForcing module scope with
export {}and then augmentingglobal/NodeJSwith the genericTypedArrayandArrayBufferViewunions is consistent with modern TS typed‑array definitions; I don’t see any issues here.cli/lsp/resolver.rs (1)
943-947: Verify no unsafe unwraps oftypes_node_version_reqexist in call sitesSetting
types_node_version_reqtoNoneis structurally sound since the field is typed asOption<VersionReq>indeno_npm. This aligns with bundling node types instead of pulling@types/node. However, ensure all consumers ofNpmVersionResolveruse safe extraction methods (e.g.,get_types_node_version_req()) rather than unchecked unwraps.cli/tsc/dts/node/buffer.d.cts (4)
1-4: Conditional type aliases look good.The pattern using
typeof globalThis extends { onmessage: any; ... }correctly avoids type conflicts whenlib.dom.d.tsorlib.webworker.d.tsis loaded.
232-233: Static analysis warnings foratob/btoaredeclarations are expected.Biome flags lines 1893 and 1909 as redeclarations. This is intentional and follows the
@types/nodepattern—theexport importon lines 232-233 makes them module exports, while the global function declarations on lines 1893/1909 make them available as globals. This dual availability is correct for Node.js compatibility.Also applies to: 1893-1909
51-124: Module exports and API surface are comprehensive.The exports for
isUtf8,isAscii,transcode,resolveObjectURL, constants,Blob, andFileclasses align with Node.js buffer API. TheBinaryLikeimport fromnode:cryptofor theBlobconstructor is appropriate.
238-256: GlobalBufferEncodingaugmentation is correctly structured.The global namespace augmentation for
NodeJS.BufferEncodingand the globalBufferEncodingtype alias cover all standard encodings including the newerbase64url.cli/tsc/dts/node/os.d.cts (1)
10-496: Comprehensive os module declaration.The API surface correctly covers all Node.js
osmodule functions, interfaces, and constants including platform-specific errno codes (both POSIX and WindowsWSA*variants). The function overloads foruserInfoandsetPriorityare correctly typed.cli/tsc/dts/node/globals.d.cts (4)
1-30: Module export and conditional types are well-designed.The
export {}ensures this is treated as a module. The conditional type aliases correctly detect the presence of DOM types viaonmessage(for fetch types) andonabort(for Storage) to avoid conflicts while providing fallbacks to undici types.
94-168: Global augmentations for process, console, Error, and gc are correct.The ErrorConstructor extension with
captureStackTrace,prepareStackTrace, andstackTraceLimitproperly exposes V8-specific APIs. The optionalgcfunction with--expose-gcflag documentation is appropriate.
170-262: NodeJS namespace interfaces are comprehensive.CallSite, ErrnoException, ReadableStream, WritableStream, RefCounted, Dict, GCFunction/GCOptions, and the Iterator/AsyncIterator interfaces provide a solid foundation for Node.js type definitions.
268-315: AbortController, Storage, and fetch API interfaces are correctly structured.The interfaces extend their conditional type aliases, ensuring they're either empty (when DOM types are present) or populated with the undici-based definitions.
cli/tsc/dts/node/events.d.cts (3)
102-118: Generic EventEmitter type system is well-designed.The
EventMap,DefaultEventMap, and helper types (Args,Key,Listener, etc.) enable type-safe event handling when a specific event map is provided while maintaining backward compatibility with the defaultstring | symbolevent names.
499-586: EventEmitterAsyncResource integration is correct.The
EventEmitterAsyncResourceclass properly extendsEventEmitterand exposesasyncId,triggerAsyncId,asyncResource, andemitDestroy()methods for async context tracking. TheEventEmitterReferencingAsyncResourceinterface correctly references back to the emitter.
588-924: Global NodeJS.EventEmitter interface is comprehensive.All standard EventEmitter instance methods are properly typed with the generic event map support, including
on,once,emit,removeListener,listeners,rawListeners,eventNames,prependListener,prependOnceListener,setMaxListeners,getMaxListeners, andlistenerCount.cli/tsc/dts/node/assert.d.cts (4)
6-51: Assert module and AssertionError class are correctly declared.The main
assertfunction withasserts valuetype guard and theAssertionErrorclass with proper properties (actual,expected,operator,generatedMessage,code) follow Node.js API exactly.
52-184: CallTracker class correctly marked as deprecated.The
@deprecatedJSDoc tag is properly applied. The method overloads forcalls()handle all use cases including the genericFuncparameter.
460-460: Type assertion functions use proper type predicates.
strictEqual<T>returnsasserts actual is TanddeepStrictEqual<T>returnsasserts actual is T, enabling TypeScript to narrow types after successful assertions. This is correct and useful for type-safe testing.Also applies to: 492-492
1014-1049: Strict assertion mode is well-typed.The
strictconst usesOmitto exclude legacy methods and then re-adds them with strict equivalents (equal→strictEqual,deepEqual→deepStrictEqual, etc.). The comment about mapped types and assertion functions incompatibility (lines 1040-1042) correctly explains why explicit property assignments are needed.cli/tsc/dts/node/dns.d.cts (1)
1-918: LGTM - Comprehensive DNS module type declarations.This file provides complete TypeScript type declarations for the Node.js
dnsmodule, including all lookup/resolve functions, record types, error codes, and theResolverclass. The structure correctly mirrors the official@types/nodepackage.One minor note: Line 363 uses
address: CaaRecord[](singular) while other array callbacks useaddresses(plural). This is inherited from upstream@types/nodeand doesn't affect functionality.
There was a problem hiding this comment.
There's a comment in this file that says this:
// todo(dsherret): add support for injecting this in the graph so
// we don't need this special code here.
// This could occur when resolving npm:@types/node when it is
// injected and not part of the graph
We should investigate if that code is still necessary after this PR. I would suspect not.
1 participant
This adds a new "node" lib that's included by default and can be excluded by providing a custom
"lib": [...]typescript compiler option without the"node"value.If it detects a
@types/nodepackage in the files being type checked it will skip injecting thelib.node.d.tsfile.npm:@types/node@24.2.0Closes #30963