From e8367f4900b1b53d171bf0faf2d55259ff8b81b9 Mon Sep 17 00:00:00 2001 From: Benny Neugebauer Date: Tue, 29 Sep 2026 16:04:27 +0200 Subject: [PATCH] fix: Respect package.json exports when adding extensions to package imports Package imports got a file extension whenever a matching file existed on disk, without looking at the package's "exports" field. For a package that exports "./sub" and also ships sub.js, "pkg/sub" became "pkg/sub.js", which Node.js rejects with ERR_PACKAGE_PATH_NOT_EXPORTED. Package imports are now checked with TypeScript's module resolver in NodeNext ESM mode. An import that resolves as written stays untouched, and an extension is only added when the import doesn't resolve without it but does with it. This also covers package roots like "@playwright/test" (#81), so the isNodeModuleRoot special case is gone. --- .gitignore | 3 ++ src/converter/convertFile.test.ts | 4 ++ .../node_modules/@scope/pkg/index.js | 1 + .../node_modules/@scope/pkg/package.json | 1 + .../node_modules/legacy/dir/index.js | 1 + .../node_modules/legacy/omit.js | 1 + .../node_modules/legacy/package.json | 1 + .../node_modules/modern/package.json | 1 + .../node_modules/modern/sub.js | 1 + .../fixtures/package-exports/src/main.snap.ts | 9 ++++ src/test/fixtures/package-exports/src/main.ts | 9 ++++ .../fixtures/package-exports/tsconfig.json | 11 ++++ src/util/PathUtil.ts | 8 --- src/util/replaceModulePath.ts | 54 +++++++++++++++---- 14 files changed, 86 insertions(+), 19 deletions(-) create mode 100644 src/test/fixtures/package-exports/node_modules/@scope/pkg/index.js create mode 100644 src/test/fixtures/package-exports/node_modules/@scope/pkg/package.json create mode 100644 src/test/fixtures/package-exports/node_modules/legacy/dir/index.js create mode 100644 src/test/fixtures/package-exports/node_modules/legacy/omit.js create mode 100644 src/test/fixtures/package-exports/node_modules/legacy/package.json create mode 100644 src/test/fixtures/package-exports/node_modules/modern/package.json create mode 100644 src/test/fixtures/package-exports/node_modules/modern/sub.js create mode 100644 src/test/fixtures/package-exports/src/main.snap.ts create mode 100644 src/test/fixtures/package-exports/src/main.ts create mode 100644 src/test/fixtures/package-exports/tsconfig.json diff --git a/.gitignore b/.gitignore index 0366cc2..ad3b3b9 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,6 @@ dist/ lerna-debug.log node_modules/ npm-debug.log* + +# Fake packages used as test fixtures +!src/test/fixtures/*/node_modules/ diff --git a/src/converter/convertFile.test.ts b/src/converter/convertFile.test.ts index e1e3777..ec86f39 100644 --- a/src/converter/convertFile.test.ts +++ b/src/converter/convertFile.test.ts @@ -39,6 +39,10 @@ describe('convertFile', () => { await testFileConversion('dynamic-imports'); }); + it('adds file extensions to package imports only when needed to resolve as ESM', async () => { + await testFileConversion('package-exports'); + }); + it('handles index files referenced with a trailing slash', async () => { await testFileConversion('trailing-slash'); }); diff --git a/src/test/fixtures/package-exports/node_modules/@scope/pkg/index.js b/src/test/fixtures/package-exports/node_modules/@scope/pkg/index.js new file mode 100644 index 0000000..f053ebf --- /dev/null +++ b/src/test/fixtures/package-exports/node_modules/@scope/pkg/index.js @@ -0,0 +1 @@ +module.exports = {}; diff --git a/src/test/fixtures/package-exports/node_modules/@scope/pkg/package.json b/src/test/fixtures/package-exports/node_modules/@scope/pkg/package.json new file mode 100644 index 0000000..857d7ad --- /dev/null +++ b/src/test/fixtures/package-exports/node_modules/@scope/pkg/package.json @@ -0,0 +1 @@ +{"main": "index.js", "name": "@scope/pkg"} diff --git a/src/test/fixtures/package-exports/node_modules/legacy/dir/index.js b/src/test/fixtures/package-exports/node_modules/legacy/dir/index.js new file mode 100644 index 0000000..f053ebf --- /dev/null +++ b/src/test/fixtures/package-exports/node_modules/legacy/dir/index.js @@ -0,0 +1 @@ +module.exports = {}; diff --git a/src/test/fixtures/package-exports/node_modules/legacy/omit.js b/src/test/fixtures/package-exports/node_modules/legacy/omit.js new file mode 100644 index 0000000..f053ebf --- /dev/null +++ b/src/test/fixtures/package-exports/node_modules/legacy/omit.js @@ -0,0 +1 @@ +module.exports = {}; diff --git a/src/test/fixtures/package-exports/node_modules/legacy/package.json b/src/test/fixtures/package-exports/node_modules/legacy/package.json new file mode 100644 index 0000000..2ec4a0b --- /dev/null +++ b/src/test/fixtures/package-exports/node_modules/legacy/package.json @@ -0,0 +1 @@ +{"main": "index.js", "name": "legacy"} diff --git a/src/test/fixtures/package-exports/node_modules/modern/package.json b/src/test/fixtures/package-exports/node_modules/modern/package.json new file mode 100644 index 0000000..75b2b4c --- /dev/null +++ b/src/test/fixtures/package-exports/node_modules/modern/package.json @@ -0,0 +1 @@ +{"exports": {"./sub": "./sub.js"}, "name": "modern", "type": "module"} diff --git a/src/test/fixtures/package-exports/node_modules/modern/sub.js b/src/test/fixtures/package-exports/node_modules/modern/sub.js new file mode 100644 index 0000000..ff8b4c5 --- /dev/null +++ b/src/test/fixtures/package-exports/node_modules/modern/sub.js @@ -0,0 +1 @@ +export default {}; diff --git a/src/test/fixtures/package-exports/src/main.snap.ts b/src/test/fixtures/package-exports/src/main.snap.ts new file mode 100644 index 0000000..e7a1cb0 --- /dev/null +++ b/src/test/fixtures/package-exports/src/main.snap.ts @@ -0,0 +1,9 @@ +// Legacy package without "exports" gets file extensions +import omit from 'legacy/omit.js'; +import dir from 'legacy/dir/index.js'; +// Package with "exports" stays untouched +import sub from 'modern/sub'; +// Package root stays untouched +import pkg from '@scope/pkg'; + +console.log(omit, dir, sub, pkg); diff --git a/src/test/fixtures/package-exports/src/main.ts b/src/test/fixtures/package-exports/src/main.ts new file mode 100644 index 0000000..4045e02 --- /dev/null +++ b/src/test/fixtures/package-exports/src/main.ts @@ -0,0 +1,9 @@ +// Legacy package without "exports" gets file extensions +import omit from 'legacy/omit'; +import dir from 'legacy/dir'; +// Package with "exports" stays untouched +import sub from 'modern/sub'; +// Package root stays untouched +import pkg from '@scope/pkg'; + +console.log(omit, dir, sub, pkg); diff --git a/src/test/fixtures/package-exports/tsconfig.json b/src/test/fixtures/package-exports/tsconfig.json new file mode 100644 index 0000000..d20568f --- /dev/null +++ b/src/test/fixtures/package-exports/tsconfig.json @@ -0,0 +1,11 @@ +{ + "compilerOptions": { + "esModuleInterop": true, + "forceConsistentCasingInFileNames": true, + "module": "Node16", + "skipLibCheck": true, + "strict": true, + "target": "es2016" + }, + "include": ["src"] +} diff --git a/src/util/PathUtil.ts b/src/util/PathUtil.ts index 15c1dae..5abb5bb 100644 --- a/src/util/PathUtil.ts +++ b/src/util/PathUtil.ts @@ -1,5 +1,4 @@ import path from 'node:path'; -import fs from 'node:fs'; import type {ModuleInfo} from '../parser/InfoParser.js'; /** @@ -55,10 +54,3 @@ export function hasRelativePath(path: string) { } return path.startsWith('./') || path.startsWith('../'); } - -export function isNodeModuleRoot(directory: string): boolean { - const isInNodeModules = directory.includes(`node_modules${path.sep}`); - const packageJsonPath = path.join(directory, 'package.json'); - const packageJsonExists = fs.existsSync(packageJsonPath); - return isInNodeModules && packageJsonExists; -} diff --git a/src/util/replaceModulePath.ts b/src/util/replaceModulePath.ts index eff24cb..19f3b48 100644 --- a/src/util/replaceModulePath.ts +++ b/src/util/replaceModulePath.ts @@ -1,8 +1,8 @@ -import {SourceFile, StringLiteral} from 'ts-morph'; +import {SourceFile, StringLiteral, ts} from 'ts-morph'; import {ModuleInfo, parseInfo} from '../parser/InfoParser.js'; import {ProjectUtil} from './ProjectUtil.js'; import {toImport, toImportAttribute} from '../converter/ImportConverter.js'; -import {getNormalizedPath, isNodeModuleRoot} from './PathUtil.js'; +import {getNormalizedPath} from './PathUtil.js'; import path from 'node:path'; import {PathFinder} from './PathFinder.js'; @@ -49,24 +49,56 @@ function createReplacementPath({ return toImportAttribute(info); } + if (isNodeModulesPath) { + return createPackageImportPath(info); + } + // If an import does not have a file extension or isn't an extension recognized here and can't be found locally (perhaps // file had . in name), try to find a matching file by traversing through all valid TypeScript source file extensions. - let baseFilePath = comesFromPathAlias + const baseFilePath = comesFromPathAlias ? getNormalizedPath(projectDirectory, info, paths) : path.join(info.directory, info.normalized); - if (isNodeModulesPath) { - baseFilePath = path.join(projectDirectory, 'node_modules', info.normalized); - } const foundPath = PathFinder.findPath(baseFilePath, info.extension); if (foundPath) { - // TODO: Write test case for this condition, mock "path" and "fs" calls if necessary - if (foundPath.extension === '/index.js' && isNodeModuleRoot(baseFilePath)) { - // @fixes https://github.com/bennycode/ts2esm/issues/81#issuecomment-2437503011 - return null; - } return toImport({...info, extension: foundPath.extension}); } } return null; } + +const ESM_RESOLUTION_OPTIONS: ts.CompilerOptions = { + allowJs: true, + module: ts.ModuleKind.NodeNext, + moduleResolution: ts.ModuleResolutionKind.NodeNext, +}; + +// Resolves a specifier the way Node.js resolves it from an ES module, which respects "exports" in package.json +function resolvesAsESM(specifier: string, containingFile: string) { + const {resolvedModule} = ts.resolveModuleName( + specifier, + containingFile, + ESM_RESOLUTION_OPTIONS, + ts.sys, + undefined, + undefined, + ts.ModuleKind.ESNext + ); + return !!resolvedModule; +} + +// Imports from packages only get an extension when they don't resolve as written but do with the extension. +// This keeps imports of packages with "exports" (i.e. "firebase-functions/v1/https") untouched, +// while legacy packages (i.e. "lodash/omit") become "lodash/omit.js". +// @see https://github.com/bennycode/ts2esm/issues/128 +function createPackageImportPath(info: ModuleInfo) { + if (resolvesAsESM(info.normalized, info.sourceFilePath)) { + return null; + } + for (const extension of ['.js', '/index.js']) { + if (resolvesAsESM(`${info.normalized}${extension}`, info.sourceFilePath)) { + return toImport({...info, extension}); + } + } + return null; +}