fix(third-party-dts-extractor): use createRequire for require.resolve in ESM build - #5014
Conversation
… in ESM build tsup's ESM output (dist/index.mjs) shims bare `require` to a stub whose .resolve is undefined, since require does not exist natively in ESM. getPackageRootDir() and resolvePackageJson() called require.resolve() directly, so both threw a TypeError under the ESM build; the error was swallowed by inferPkgDir()'s try/catch, leaving this.pkgs empty and copyDts() a silent no-op — extractThirdParty produced no output and no warning. Bind a real require via createRequire(import.meta.url) at module scope in both files, matching the existing precedent in packages/rsbuild-plugin/src/utils/ssr.ts. The CJS build is unaffected since createRequire works identically there. Closes module-federation#5007
…rdPartyExtractor inferPkgDir() has its own bare require.resolve() call, hit by the same tsup ESM shim issue as utils.ts. Same fix: bind require via createRequire(import.meta.url) at module scope. Relates to module-federation#5007
🦋 Changeset detectedLatest commit: 975f9bf The changes in this PR will be included in the next version bump. This PR includes changesets to release 48 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 975f9bfbae
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| import { getTypedName, getPackageRootDir, resolvePackageJson } from './utils'; | ||
|
|
||
| // See utils.ts: bare `require` is not a real require in tsup's ESM output. | ||
| const require = createRequire(import.meta.url); |
There was a problem hiding this comment.
Avoid import.meta.url in the CommonJS build
When consumers load the package through its documented require export, tsup.config.ts emits this source as CommonJS, where esbuild replaces unsupported import.meta with an empty object. This consequently calls createRequire(undefined) and throws ERR_INVALID_ARG_VALUE during module initialization, breaking the entire CommonJS entry point; use a build-format-safe filename/URL or configure a CommonJS shim while retaining import.meta.url for ESM.
AGENTS.md reference: AGENTS.md:L289-L289
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed — verified by building this exact source in isolation with tsup 7.3.0. Without shims, the cjs output does exactly what you describe: import.meta becomes {}, so createRequire(import_meta.url) is createRequire(undefined) and throws ERR_INVALID_ARG_VALUE at module load, breaking require() for every consumer.
Enabled tsup's shims: true option in tsup.config.ts (see latest commit), which injects a proper import.meta.url polyfill (new URL("file:" + __filename).href) for the cjs build. Re-verified both outputs directly: cjs and esm now both resolve correctly with no crash.
Thanks for catching this before it shipped.
…rks in the cjs build
createRequire(import.meta.url), added in the previous commit, works
correctly in the esm build but breaks the cjs one: esbuild leaves
import.meta as an empty object under the cjs target, so
createRequire(import_meta.url) is really createRequire(undefined),
which throws ERR_INVALID_ARG_VALUE at module load time -- turning a
silent ESM-only failure into a hard crash on require() for every
consumer.
tsup's shims option injects a proper import.meta.url polyfill
(new URL("file:" + __filename).href) for the cjs build, so
createRequire receives a real file URL in both formats. Verified by
building this exact source with tsup 7.3.0 in isolation and running
both the cjs and esm outputs directly: both now resolve correctly,
and neither crashes.
Thanks to the automated review on this PR for catching this before it
shipped.
Current Behavior
With
@module-federation/vitein a"type": "module"project,dts.generateTypes.extractThirdPartynever copies anything into@mf-types/node_modules, with no error or warning — the build reports success.Root Cause
getPackageRootDir()andresolvePackageJson()inpackages/third-party-dts-extractor/src/utils.tscall the barerequire.resolve(). This package is built by tsup into both a CJS (dist/index.js) and ESM (dist/index.mjs) bundle. In the ESM output, tsup shims barerequireto a stub whose.resolveisundefined(sincerequiredoesn't natively exist in ESM), so both calls throw aTypeError.That error is caught by the
try/catchinThirdPartyExtractor.inferPkgDir()(which has the same barerequire.resolve()call itself), sothis.pkgsstays empty andcopyDts()silently does nothing. The CJS bundle is unaffected sincerequireis real there.Fix
Bind a real
requireviacreateRequire(import.meta.url)at module scope in bothutils.tsandThirdPartyExtractor.ts, instead of relying on the ambient global. This matches the existing precedent inpackages/rsbuild-plugin/src/utils/ssr.ts:createRequireworks identically under both the CJS and ESM builds, so this doesn't change behavior for the CJS bundle at all — it only fixes the ESM path.Included a changeset for
@module-federation/third-party-dts-extractor(patch).Verification
Reasoned through and matched against the reporter's own minimal repro (
require.resolveundernode --input-type=modulevs. plainnode -e) and the existingcreateRequire(import.meta.url)pattern already shipping inpackages/rsbuild-plugin. I wasn't able to run a fullpnpm install/build of this monorepo locally to execute the existingThirdPartyExtractor.spec.tssuite end-to-end — happy to iterate on CI feedback.Closes #5007