Repository navigation
Unwrap tsc's interop helpers around builtin modules in packages - #1683
Merged
Merged
Conversation
tsc wraps every CommonJS import in its helpers: `const events_1 =
__importDefault(require("events"))`, `const path =
__importStar(require("path"))`. cjs-tsc-interop.ts reads such bindings of
program modules as plain require bindings, so the helper declarations go
dead, but a builtin module is not a program module: its binding kept the
helpers live, and their probe of the module's top-level `this`
(`(this && this.__importDefault) ||`) failed at load ("property access
through top-level 'this'"). That is the shape of hundreds of tsc-compiled
files in a typical dependency tree.
A builtin carries no `__esModule`, so `events_1.default` is the module and
the namespace copy reads the module's members. When every reference has
that form, a package-file stage now respells the binding as the plain
require() of the builtin and `x.default` as `x`.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tsc wraps every CommonJS import in its interop helpers:
const events_1 = __importDefault(require("events")),const path = __importStar(require("path")). For program modules,cjs-tsc-interop.tsreads such a binding as a plain require binding, so the helper declarations go dead. A builtin module is not a program module, though, so its binding kept the helpers live. Their probe of the module's top-levelthis,var __importDefault = (this && this.__importDefault) || …, then failed at load ("property access through top-level 'this'"). That shape appears in hundreds of tsc-compiled files in a typical dependency tree.A builtin carries no
__esModule, soevents_1.defaultis the module and the namespace copy reads the module's members. When every reference has that form, a package-file stage (npm-static-tsc-helpers.ts) now respells the binding as the plainrequire()of the builtin andx.defaultasx. Any other reference (the binding used as a value,ns.defaultof a namespace copy, a program module) keeps the text unchanged. Dropping the probe instead would be wrong:cjs-tsc-interop.tsrecognizes the helpers by that probe.Why this PR
This is salvaged from #1584, which was merged only into #1558's branch (
opt/npm-cjs-var-export), not main. Under the scope-B plan, #1558's bundled-CommonJS emulation goes to Node islands and is closed. The tsc helpers stay native because typed, tsc-emitted packages need them. This PR carries only the tsc-helper commit (d60d1f2, with the formatting fixes #1584 later applied to the same files) and its fixtures. It leaves out #1584's bundled-ESM namespace work (bundler interop, D1) and #1610 (exports object asthis, a CommonJS export form, D2).Evidence (Linux sandbox, against main 0237d70)
tsc-builtin-interop: real tsc 5.6 output (module: commonjs,esModuleInterop) of a class extendingevents.default.EventEmitterand a namespace import ofpath. Its output is byte-identical to Node's; before this change it failed at load. Its order baseline is recorded.npm-static-tsc-helpers.test.tsruns rewritten modules against their originals under Node, and checks which shapes keep their text.packages/compiler/src/frontendunit tests: 891 passed.tests/harness/npm.test.ts -t tsc: 3 passed.npm-static.test.tsandpackage-shims.test.ts: 138 passed.pnpm lint,pnpm format:checkandpnpm test:ts7 --baselines-onlypass. The self-hosted--emit=irbuild ofpackages/compiler/src/native/cli.tsreports no diagnostics.fs-extra-package, whose IR embeds the fs-extra shim's absolute checkout path (it differs between any two checkouts). Compile time: bench geomean +0.53% [95% CI -0.84%, +1.87%], self-hosted +0.75%, both within tolerance.🤖 Generated with Claude Code