fix: resolve dynamic import() and bare require/module in TypeScript configs - #5686
Open
gololdf1sh wants to merge 1 commit into
Open
fix: resolve dynamic import() and bare require/module in TypeScript configs#5686gololdf1sh wants to merge 1 commit into
gololdf1sh wants to merge 1 commit into
Conversation
…onfigs
A `.ts` config is transpiled to a temp `.mjs` and its import tree is transpiled
with it, but two things were missed, and both surface only at runtime.
Dynamic `import('./module')` was invisible to the transpiler: the dependency
scan and the rewrite pass matched `from '...'` and `require('...')` only, so a
lazily imported module was never emitted and the specifier still pointed at a
`.ts` path — ERR_MODULE_NOT_FOUND. Static and dynamic specifiers now share one
resolver, so both follow the same ESM resolution.
The CommonJS shim was gated on `require(` and `module.exports`, so the standard
`if (require.main === module)` entrypoint idiom got no shim and the transpiled
file threw "require is not defined in ES module scope". Detection now counts
bare `require` / `module` identifiers, ignores quoted occurrences such as
`from 'module'`, and skips files that declare their own binding — which also
stops the shim redeclaring a user's own `const require = createRequire(...)`.
Unit suite: 769 -> 771 passing, 0 failing.
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
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.
Motivation/Description of the PR
A
.tsconfig is not executed as TypeScript —lib/config.jstranspiles it to a temp.mjsandtranspileTypeScriptwalks and transpiles its import tree along with it. Two gaps in that walk make perfectly ordinary TypeScript fail, and both only surface at runtime, with errors that point away from the real cause.1. Dynamic
import()is invisible to the transpiler. The dependency scan and the rewrite pass matchedfrom '...'andrequire('...')only. A module reached throughawait import('./module')was therefore never emitted, and the specifier survived transpilation still pointing at an extensionless.tspath:The same import written statically at the top of the config works, which makes the failure look like an ESM-vs-
tsxresolution problem in the runner rather than a gap in the transpiler. In our suite this cost a real workaround: both lifecycle hooks shelled out toexecSync('npx tsx <script>')for a year, with a code comment blaming the worker ESM context.Static and dynamic specifiers now share one resolver, so both follow identical ESM resolution — path aliases,
index.ts,.js-that-is-really-.ts, and the extension-append fallback included.2. A bare
require/moduleidentifier gets no CommonJS shim. Shim injection was gated on/\brequire\s*\(/and/\b(module\.exports|exports\.)/, so the standard entrypoint idiom matched neither:Detection now counts bare
require/moduleidentifiers. Two refinements keep it from over-firing:import { createRequire } from 'module'is not mistaken for a reference;const require = createRequire(import.meta.url)and callingrequire('x')used to get a secondconst requireinjected, i.e. aSyntaxErrorfrom the shim itself.Verification
New fixture
test/data/typescript-config-dynamic-import/covers both paths in one realistic shape: a config that dynamically imports a lifecycle module, which in turn statically imports a third file and carries arequire.main === moduleguard. Two unit tests assert the module tree is fully emitted and reachable, that the guarded file imports without throwing, and that the guard stays dormant on import.Unit suite before this change: 769 passing, 11 pending, 0 failing. After: 771 passing, 11 pending, 0 failing.
eslintclean on the touched files.Also checked against the real-world case that prompted this, a 10-file config import tree in a private suite:
await import('./src/…/globalTeardown')in a.tsconfigERR_MODULE_NOT_FOUNDApplicable helpers:
Applicable plugins:
Type of change
Checklist:
test/unit/utils/typescript_test.js)npx eslint lib/utils/typescript.js test/unit/utils/typescript_test.js)mocha test/unit --recursive: 771 passing, 0 failing)