chore(apps, napi): restrict global types to NodeJS in tsconfig#20064
chore(apps, napi): restrict global types to NodeJS in tsconfig#20064overlookmotel wants to merge 1 commit intomainfrom
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
github-actions
bot
added
A-linter
github-actions
bot
added
the
C-cleanup
There was a problem hiding this comment.
Pull request overview
Adds explicit TypeScript types scoping for several Node-focused packages to prevent unintended ambient @types/* leakage into compilation, improving type-check determinism across the workspace.
Changes:
- Add
"types": ["node"]toapps/oxfmt/tsconfig.jsonandapps/oxlint/tsconfig.json. - Add
"types": ["node"]to N-API package tsconfigs:napi/minify,napi/parser, andnapi/transform.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| napi/transform/tsconfig.json | Restricts auto-included global types to Node.js only. |
| napi/parser/tsconfig.json | Restricts auto-included global types to Node.js only. |
| napi/minify/tsconfig.json | Restricts auto-included global types to Node.js only. |
| apps/oxlint/tsconfig.json | Restricts auto-included global types to Node.js only (helps avoid transitive ambient type leaks). |
| apps/oxfmt/tsconfig.json | Restricts auto-included global types to Node.js only (helps avoid transitive ambient type leaks). |
tsgolint restricts types to |
No idea! But I can conform that it was happening. If you neglected to import types like |
|
Hmm I still don't understand this change - if i add types: node to the tsconfig, i still get Furthermore is having node in the globals correct? We can't guarantee that these packages are always running in node - they might be running in the browser ect. |
|
@overlookmotel I think we want this instead: #20071 |
|
👍🏻 for #20071 |
|
I don't really understand this tsconfig business. I didn't write it - this was part of Sapphi's PR #19675. I've just split that PR up into 4 PRs to make it easier to disentangle and look at each piece in isolation. And yeah you're right, it doesn't seem to stop e.g. The rest of the stack works without this anyway. I'm going to remove this PR from the stack and continue revising the rest. |
overlookmotel
deleted the
om/03-06-chore_apps_napi_restrict_global_types_to_nodejs_in_tsconfig
branch
Replaces #20064 The `dom` lib was implicitly being injected, this means stuff like `Token` and `Comment` were in the global scope, which was incorrect. cc @overlookmotel
Add
"types": ["node"]totsconfig.jsoninapps/oxfmt,apps/oxlint,napi/minify,napi/parser, andnapi/transform.This restricts auto-included global type declarations to
@types/nodeonly, preventing unintended ambient types from transitive@types/*packages from leaking into the type-checking scope.This change was broken out from #19675.