Skip to content

typings: fix internal type resolution - #66512

Open
leah-1ee wants to merge 2 commits into
nodejs:mainfrom
leah-1ee:typings/fix-internal-type-resolution
Open

leah-1ee wants to merge 2 commits into
nodejs:mainfrom
leah-1ee:typings/fix-internal-type-resolution

Conversation

@leah-1ee

@leah-1ee leah-1ee commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

With TypeScript 5.9.3, the root project reports 51 TS2451 diagnostics because builtin files such as primordials.js and realm.js are treated as scripts. Their wrapper-local declarations consequently collide with standard library globals. The ICU declaration also reports TS2580 because its Buffer return type has no local type definition.

Set moduleDetection: "force" to separate file scopes. This alone would remove the shared internalBinding type information, so add an ambient let that reuses the existing generic function type and binding map. This preserves binding inference without exposing a globalThis.internalBinding property.

Define the ICU Buffer alias as Uint8Array, following the existing byte-view convention in internal binding declarations. No runtime code is changed.

Validation:

  • The root project passes with TypeScript 5.9.3 and --noEmit --skipLibCheck false, removing all 52 diagnostics.
  • All 567 input files and the return types of 420 existing internalBinding references are preserved.

Refs: #49742
Refs: #59176

Assisted-by: Codex

@nodejs-github-bot nodejs-github-bot added the typings Issues and PRs related to internal TypeScript declarations. label Oct 4, 2026
Comment thread typings/internalBinding/icu.d.ts Outdated
@@ -1,3 +1,5 @@
type Buffer = Uint8Array;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type Buffer = Uint8Array;
import { FastBuffer as Buffer } from 'internal/buffer';

Comment thread tsconfig.json Outdated
"target": "ESNext",
"module": "CommonJS",
"moduleDetection": "force",
"baseUrl": ".",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"baseUrl": ".",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review and suggestions! I've applied all three suggestions. I also updated the JSDoc reference in realm.js and converted five type imports in resolve.js and package_json_reader.js to relative paths to preserve their type information.

Comment thread typings/globals.d.ts Outdated
Comment on lines +111 to +115
declare function internalBinding<T extends InternalBindingKeys>(binding: T): InternalBindingMap[T]

declare global {
// Supplied to internal modules by the builtin function wrapper.
let internalBinding: typeof import('./globals').internalBinding;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just move the function definition, there's no need to juggle this now that the global scope clash with internal/bootstrap/realm is removed by the moduleDetection setting.

Suggested change
declare function internalBinding<T extends InternalBindingKeys>(binding: T): InternalBindingMap[T]
declare global {
// Supplied to internal modules by the builtin function wrapper.
let internalBinding: typeof import('./globals').internalBinding;
declare global {
// Supplied to internal modules by the builtin function wrapper.
function internalBinding<T extends InternalBindingKeys>(binding: T): InternalBindingMap[T]

Treat builtin JavaScript files as modules to avoid script-scope
collisions with standard library declarations. Explicitly provide the
existing internalBinding type so binding inference is preserved.

Define the ICU Buffer alias using the existing Uint8Array convention.

Assisted-by: Codex
Signed-off-by: leah-1ee <selee3196@gmail.com>
Use FastBuffer for ICU and move internalBinding into declare global.
Remove baseUrl and update JSDoc references to preserve type links.

Assisted-by: Codex
Signed-off-by: leah-1ee <selee3196@gmail.com>
@leah-1ee
leah-1ee force-pushed the typings/fix-internal-type-resolution branch from dcbaadc to 4a82eb2 Compare October 5, 2026 14:55
@codecov

codecov Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.39%. Comparing base (bbd566d) to head (4a82eb2).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66512      +/-   ##
==========================================
- Coverage   92.74%   90.39%   -2.35%     
==========================================
  Files         422      791     +369     
  Lines      193170   275991   +82821     
  Branches    29783    52971   +23188     
==========================================
+ Hits       179160   249494   +70334     
- Misses      13682    16903    +3221     
- Partials      328     9594    +9266     
Files with missing lines Coverage Δ
lib/internal/bootstrap/realm.js 97.09% <100.00%> (+0.77%) ⬆️
lib/internal/modules/esm/resolve.js 99.02% <100.00%> (+12.52%) ⬆️
lib/internal/modules/package_json_reader.js 99.28% <100.00%> (+12.38%) ⬆️

... and 496 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

typings Issues and PRs related to internal TypeScript declarations.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants