Repository navigation
Conversation
…ss overloads A method declared on the bun-types Process merge hides the one Process inherits. On @types/node 25.0.0 through 25.0.9, Process inherits every event method from InternalEventEmitter<ProcessEventMap>, so the memoryPressure overloads rejected every other event name (#39807): error TS2345: Argument of type '"SIGINT"' is not assignable to parameter of type '"memoryPressure"'. The off/removeListener fallbacks from #40004 merge ahead of the @types/node overloads when bun-types is loaded through its index.d.ts (the default `types: ["bun"]` and every other order tried), which made those listener arguments `any` on 24, 25 and 26. Declare memoryPressure as an entry of ProcessEventMap through a node:process augmentation and remove the method declarations from the merge. Every event method is typed on 25.0.0 through 26.x. On 24, which has no event map, memoryPressure is an unknown event name. Add two bun-types checks: one pinned to @types/node@25.0.0 for each event method, and one that type-checks the whole fixture against the @types/node the repo pins. Fixes #39807
|
Status: reproduced by type-checking |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review. WalkthroughChangesProcess event types
Suggested reviewers: Merge Risk: ⚪ Minimal · up to This localized type-declaration change is merge-ready after normal checks and review; no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The changes directly address issue Full details: Description checkExplanation The description explains the problem, the fix, compatibility trade-offs, linked issue, and verification steps. It covers the template requirements, although it uses equivalent headings instead of the exact template headings. Comment |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because packages/bun-types/ is CODEOWNER-owned and the change carries an explicit trade-off for @ types/node 24 users (emit/addListener/prepend* now reject "memoryPressure" there), a human sign-off on that trade-off is still worthwhile.
What was reviewed:
- The
ProcessEventMapaugmentation shape — tuple entry matches how @ types/node 25+ maps event methods, and the JSDoc is preserved. - Removed overloads vs. the new augmentation — confirmed no other Bun-specific process event overloads remain on the merge that would still shadow inherited signatures.
- New tests follow the file's existing pattern (spawned tsc,
bun add @ types/node@<spec>, stdout/stderr asserted before exit code); theNotAny<T>guard correctly rejects the(...args: any[])fallback.
Extended reasoning...
Overview
This PR touches two files: packages/bun-types/overrides.d.ts (removes eight memoryPressure method overloads and two generic fallbacks from the NodeJS.Process interface merge, replaces them with a declare module "node:process" augmentation adding memoryPressure: [level: "warning" | "critical"] to ProcessEventMap) and test/integration/bun-types/bun-types.test.ts (refactors the existing @ types/node@ 24 process-event check into a shared helper, adds a pinned @ types/node@ 25.0.0 check with a NotAny<T> guard covering all eight event methods, and adds a whole-fixture check against the repo's own package.json#resolutions pin). This is a pure type-declaration change with no runtime code affected.
Security risks
None. The change is entirely in ambient TypeScript declarations and test fixtures that spawn tsc in temp directories. No auth, crypto, filesystem, or network-facing runtime code is touched.
Level of scrutiny
Medium, driven by ownership rather than risk. The declaration change itself is small and mechanically sound — augmenting ProcessEventMap is the idiomatic way to add process events on @ types/node 25+, and removing the shadowing overloads fixes real reported breakage (#39807, #40003). However, /packages/bun-types/ and *.d.ts are CODEOWNER-gated to alii, and the PR description explicitly flags a behavioral regression for @ types/node 24 users (memoryPressure becomes an unknown event name — accepted untyped by on/once/off/removeListener but rejected by emit/addListener/prepend*) and notes a prior PR asked for a CODEOWNER call on this exact trade-off without receiving one. That is precisely the kind of API-surface decision the CODEOWNERS entry exists to gate.
Other factors
The new tests are well-constructed: the NotAny<T> guard ensures listener arguments are actually typed rather than falling through to the (...args: any[]) overload, the version-prefix assertion guards against resolution drift, and the repo-pin check catches the case where the fixture's latest resolution masks breakage on the version Bun itself uses. The tests use bun add @ types/node@<spec> which contacts the registry — that pattern was already present in this file before this PR (the pre-existing @ types/node@ 24 check did the same), so it is not a newly introduced hermeticity concern. The skipLibCheck: false TS2671 warning on @ types/node 24 mentioned in the PR notes is consistent with existing behavior of the node:tls augmentation in the same file.
|
Updated 4:26 PM PT - Sep 2nd, 2026
✅ @robobun, your commit f55402ed5df391e902b7bf42c31e42e20859e95e passed in 🧪 To try this PR locally: bunx bun-pr 41208That installs a local version of the PR into your bun-41208 --bun |
Problem
error TS2345: Argument of type '"SIGINT"' is not assignable to parameter of type '"memoryPressure"'. That is New memoryPressure event type signature deletes all type signatures for other process events #39807, whose triage resolved@types/node@25to a later 25.x. The repo's own pin (25.0.0) hits it.packages/bun-types/overrides.d.ts:110declaresmemoryPressureoverloads ofon/once/emit/... on theNodeJS.Processmerge. A method declared on the merge hides the oneProcessinherits, and on those releasesProcessinherits all of them.off/removeListenerfallbacks from bun-types: keep generic process.removeListener and off signatures visible with @types/node 24 #40004 merge ahead of the @types/node overloads, soprocess.off("exit", code => ...)hascode: anyon 24, 25 and 26.Fix
memoryPressureas an entry ofProcessEventMapthrough adeclare module "node:process"augmentation, and remove the overloads and the fallbacks from the merge. Every event method on @types/node 25 and 26 keys off that map.memoryPressureis an unknown event name there.on/once/off/removeListeneraccept it untyped.emit/addListener/prepend*reject it. bun-types targets 25+.test/integration/bun-types/bun-types.test.ts. One check pins @types/node@25.0.0 (17 errors on main). One type-checks the whole fixture against the repo's pin (the 4 errors above on main).Background
interface Processagain and TypeScript merges it with @types/node's. Overloads both sides declare are combined, later declaration first. A method only one side declares hides what the other inherits.ProcessEventMap(@types/node 25+) maps each process event to its listener argument tuple. Every event method is generic over it, so one entry types the event on all of them.Fixes #39807
Notes
Where @types/node declares the process event methods, and what the old overloads did there:
Process,off/removeListenerinherited fromEventEmitteroff/removeListenerreject every other event (#40003)InternalEventEmitter<ProcessEventMap>ProcessEventMaponProcessoff/removeListenerlistenersanyProbe: a file that calls each event method with a Node event and with
memoryPressure, and fails if a listener argument isany(type NotAny<T> = 0 extends 1 & T ? never : T), checked withskipLibCheck: falseagainst the packed bun-types and each of @types/node 25.0.0, 25.0.1, 25.0.9, 25.0.10, 25.1.0, 25.9.5, 26.4.1, with TypeScript 7.0.2 and 6.0.2. All clean with this change. Also checkedprocess.ProcessEventMap["memoryPressure"],import type { ProcessEventMap } from "node:process", andlisteners("memoryPressure")on each.Overload order: bun-types'
index.d.tsreferencesnodebeforeoverrides.d.ts, so @types/node'sProcessis the earlier declaration and the merge's overloads come first. Checked on main withtypesset to["bun"],["bun-types"],["node", "bun"],["node", "bun-types"]and["bun-types", "node"]:process.off("exit", code => ...)isanyin all of them on 25.0.10 and 26.4.1.On @types/node 24.0.0 and 24.13.3 with the default
skipLibCheck: true: Node events typed on every method, and the #40003 cases pass. WithskipLibCheck: false, the augmentation adds oneTS2671: Cannot augment module 'node:process' because it resolves to a non-module entityto the three errors thenode:tlsaugmentation already produces there (ConnectionOptions,KeyObject,TLSSocket). #37790 asked for a CODEOWNER decision on this same trade-off and got no answer.Alternative tried: re-declaring the
(event: string | symbol, ...)fallbacks for the other six methods, like #40004 did for two. The merge's overloads come first, so every Node event listener becomesanyon every @types/node release, including 26. Keeping the #40004 fallbacks next to the map entry has the same effect onoff/removeListener.Repo effect:
tsc --noEmit -p scripts/build/tsconfig.jsonresolves the pinned @types/node 25.0.0 and reports threememoryPressureerrors inscripts/build/ci.tson main. With this change, none. Four unrelatednoUncheckedIndexedAccesserrors remain there. #37790 covers those, plus a source lint and a CI step for that typecheck.The whole-fixture check overrides
fixture/package.json#resolutionsfor @types/node, because that field otherwise wins over an explicitbun add @types/node@<pin>.Suites:
bun bd test test/integration/bun-types/bun-types.test.ts(9 pass, 14 skip on debug), the same file with a release build (23 pass),tsc -p packages/bun-types/tsconfig.json.[human-review] gate passed · iteration 0 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file