fix: strip CJS-bridge artifact in ESM resolve to unblock Node 24 require(esm) under tsImport()#802
Open
ryanbonial wants to merge 1 commit into
Conversation
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.
Summary
tsImport()fails on Node 24 withERR_MODULE_NOT_FOUNDwhen a CommonJS-context.tsfile imports an ESM dependency. The same code works on Node 22.Closes #801
Repro: https://github.com/ryanbonial/tsx-pkg-utils-node24-repro
Observed in the wild while building Sanity packages that consume
@sanity/pkg-utils, whoseloadConfig()callstsImport(pathToFileURL(configFile).toString(), import.meta.url)for a projectpackage.config.tsthat imports from@sanity/pkg-utils.Why
.tslives in a CJS context, so it routes through the CJS bridge.src/cjs/api/module-resolve-filename/preserve-query.tsappends?namespace=<id>to the resolved file path for per-tsImport()Module._cacheisolation.import {x} from 'esm-pkg'intorequire('esm-pkg').require(esm)stays inside CJS code paths and the?namespace=query never leaks into ESM resolve.require(esm)dispatches through the module customization hooks.pathToFileURL()encodes the?as%3Finside the URL pathname. The ESMresolvehook does not recognize this as a tsx ESM URL (notsx-commonjs-virtual-query=1marker) and forwards it tonextResolve, whichstats a literalindex.js?namespace=<id>path and throws.What
Add
stripCjsBridgeArtifact()insrc/esm/hook/resolve.tsand call it onspecifierandcontext.parentURLat the entry of bothcreateResolveandcreateResolveSync. It detects afile:URL whose pathname contains a percent-encoded?and which lacks thetsx-commonjs-virtual-query=1marker, then truncates the pathname back to the real file. The CJS bridge keeps its ownModule._cachekeys keyed off the namespace, so nothing else needs the query at this point.Tests
New test in
tests/specs/api.ts:tsImport() > requires a CJS dependency from CJS-context .ts under namespace.Fails on
masteracross the Node matrix and passes with this change. The test usesrequire()of a.cjsfile (notrequire(esm)) because that path exercises the exact same%3Fnamespace=artifact on every supported Node version, giving stronger regression coverage than a Node-24-only ESM-from-CJS case.Full suite (
pnpm test) green on the local.nvmrcNode 24.15.0.Risk
Scoped to
file:URLs whose pathname already contains a percent-encoded?. Real tsx ESM URLs use real?query strings (not encoded), so they take the existing fast-path return. URLs with the explicittsx-commonjs-virtual-query=1marker bypass the strip entirely.Made with Cursor