Skip to content

Commit 3df00f4

Browse files
fix(resolver): prioritize requested path in dependencies
fixes #617
1 parent 1c3fd22 commit 3df00f4

9 files changed

+168
-69
lines changed

src/cjs/api/module-extensions.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import { isESM } from '../../utils/es-module-lexer.js';
77
import { shouldApplySourceMap, inlineSourceMap } from '../../source-map.js';
88
import { parent } from '../../utils/ipc/client.js';
99
import { fileMatcher } from '../../utils/tsconfig.js';
10-
import { implicitlyResolvableExtensions } from './resolve-implicit-extensions.js';
1110
import type { LoaderState } from './types.js';
1211

1312
const typescriptExtensions = [
@@ -24,6 +23,12 @@ const transformExtensions = [
2423
'.mjs',
2524
] as const;
2625

26+
const implicitlyResolvableExtensions = [
27+
'.ts',
28+
'.tsx',
29+
'.jsx',
30+
] as const;
31+
2732
const safeSet = <T extends Record<string, unknown>>(
2833
object: T,
2934
property: keyof T,

src/cjs/api/module-resolve-filename.ts

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,19 @@ import Module from 'node:module';
33
import { fileURLToPath } from 'node:url';
44
import { mapTsExtensions } from '../../utils/map-ts-extensions.js';
55
import type { NodeError } from '../../types.js';
6-
import { isRelativePath, fileUrlPrefix, tsExtensionsPattern } from '../../utils/path-utils.js';
6+
import {
7+
isRelativePath,
8+
isFilePath,
9+
fileUrlPrefix,
10+
tsExtensionsPattern,
11+
isDirectoryPattern,
12+
nodeModulesPath,
13+
} from '../../utils/path-utils.js';
714
import { tsconfigPathsMatcher, allowJs } from '../../utils/tsconfig.js';
815
import { urlSearchParamsStringify } from '../../utils/url-search-params-stringify.js';
916
import type { ResolveFilename, SimpleResolve, LoaderState } from './types.js';
1017
import { createImplicitResolver } from './resolve-implicit-extensions.js';
1118

12-
const nodeModulesPath = `${path.sep}node_modules${path.sep}`;
13-
1419
const getOriginalFilePath = (
1520
request: string,
1621
) => {
@@ -53,8 +58,11 @@ const resolveTsFilename = (
5358
parent: Module.Parent | undefined,
5459
) => {
5560
if (
56-
!(parent?.filename && tsExtensionsPattern.test(parent.filename))
57-
&& !allowJs
61+
isDirectoryPattern.test(request)
62+
|| (
63+
!(parent?.filename && tsExtensionsPattern.test(parent.filename))
64+
&& !allowJs
65+
)
5866
) {
5967
return;
6068
}
@@ -82,7 +90,7 @@ const resolveTsFilename = (
8290
const resolveRequest = (
8391
request: string,
8492
parent: Module.Parent | undefined,
85-
resolve: SimpleResolve,
93+
nextResolve: SimpleResolve,
8694
) => {
8795
// Support file protocol
8896
if (request.startsWith(fileUrlPrefix)) {
@@ -102,25 +110,27 @@ const resolveRequest = (
102110
const possiblePaths = tsconfigPathsMatcher(request);
103111

104112
for (const possiblePath of possiblePaths) {
105-
const tsFilename = resolveTsFilename(resolve, possiblePath, parent);
113+
const tsFilename = resolveTsFilename(nextResolve, possiblePath, parent);
106114
if (tsFilename) {
107115
return tsFilename;
108116
}
109117

110118
try {
111-
return resolve(possiblePath);
119+
return nextResolve(possiblePath);
112120
} catch {}
113121
}
114122
}
115123

116-
// If extension exists
117-
const resolvedTsFilename = resolveTsFilename(resolve, request, parent);
118-
if (resolvedTsFilename) {
119-
return resolvedTsFilename;
124+
// It should only try to resolve TS extensions first if it's a local file (non dependency)
125+
if (isFilePath(request)) {
126+
const resolvedTsFilename = resolveTsFilename(nextResolve, request, parent);
127+
if (resolvedTsFilename) {
128+
return resolvedTsFilename;
129+
}
120130
}
121131

122132
try {
123-
return resolve(request);
133+
return nextResolve(request);
124134
} catch (error) {
125135
const nodeError = error as NodeError;
126136

@@ -133,7 +143,7 @@ const resolveRequest = (
133143
const isExportsPath = nodeError.message.match(/^Cannot find module '([^']+)'$/);
134144
if (isExportsPath) {
135145
const exportsPath = isExportsPath[1];
136-
const tsFilename = resolveTsFilename(resolve, exportsPath, parent);
146+
const tsFilename = resolveTsFilename(nextResolve, exportsPath, parent);
137147
if (tsFilename) {
138148
return tsFilename;
139149
}
@@ -142,7 +152,7 @@ const resolveRequest = (
142152
const isMainPath = nodeError.message.match(/^Cannot find module '([^']+)'. Please verify that the package.json has a valid "main" entry$/);
143153
if (isMainPath) {
144154
const mainPath = isMainPath[1];
145-
const tsFilename = resolveTsFilename(resolve, mainPath, parent);
155+
const tsFilename = resolveTsFilename(nextResolve, mainPath, parent);
146156
if (tsFilename) {
147157
return tsFilename;
148158
}
@@ -222,17 +232,16 @@ export const createResolveFilename = (
222232
return nextResolve(request, parent, ...restOfArgs);
223233
}
224234

225-
let _nextResolve = nextResolve;
226-
if (namespace) {
227-
/**
228-
* When namespaced, the loaders are registered to the extensions in a hidden way
229-
* so Node's built-in resolver will not try those extensions
230-
*
231-
* To support implicit extensions, we need to enhance the resolver with our own
232-
* re-implementation of the implicit extension resolution
233-
*/
234-
_nextResolve = createImplicitResolver(_nextResolve);
235-
}
235+
/**
236+
* Custom implicit resolver to resolve .ts over .js extensions
237+
*
238+
* To support implicit extensions, we need to enhance the resolver with our own
239+
* re-implementation of the implicit extension resolution
240+
*
241+
* Also, when namespaced, the loaders are registered to the extensions in a hidden way
242+
* so Node's built-in resolver will not try those extensions
243+
*/
244+
const _nextResolve = createImplicitResolver(nextResolve);
236245

237246
const resolve: SimpleResolve = request_ => _nextResolve(
238247
request_,

src/cjs/api/resolve-implicit-extensions.ts

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,17 @@
11
import path from 'node:path';
22
import type { NodeError } from '../../types.js';
3-
import { isBarePackageNamePattern } from '../../utils/path-utils.js';
3+
import { isDirectoryPattern } from '../../utils/path-utils.js';
4+
import { mapTsExtensions } from '../../utils/map-ts-extensions.js';
45
import type { ResolveFilename } from './types.js';
56

6-
export const implicitlyResolvableExtensions = [
7-
'.ts',
8-
'.tsx',
9-
'.jsx',
10-
] as const;
11-
127
const tryExtensions = (
138
resolve: ResolveFilename,
149
...args: Parameters<ResolveFilename>
1510
) => {
16-
for (const extension of implicitlyResolvableExtensions) {
11+
const tryPaths = mapTsExtensions(args[0]);
12+
for (const tryPath of tryPaths) {
1713
const newArgs = args.slice() as Parameters<ResolveFilename>;
18-
newArgs[0] += extension;
14+
newArgs[0] = tryPath;
1915

2016
try {
2117
return resolve(...newArgs);
@@ -29,13 +25,40 @@ export const createImplicitResolver = (
2925
request,
3026
...args
3127
) => {
28+
if (request === '.') {
29+
request = './';
30+
}
31+
32+
/**
33+
* Currently, there's an edge case where it doesn't resolve index.ts over index.js
34+
* if the request doesn't end with a slash. e.g. `import './dir'`
35+
* Doesn't handle '.' either
36+
*/
37+
if (isDirectoryPattern.test(request)) {
38+
// If directory, can be index.js, index.ts, etc.
39+
let joinedPath = path.join(request, 'index');
40+
41+
/**
42+
* path.join will remove the './' prefix if it exists
43+
* but it should only be added back if it was there before
44+
* (e.g. not package directory imports)
45+
*/
46+
if (request.startsWith('./')) {
47+
joinedPath = `./${joinedPath}`;
48+
}
49+
50+
const resolved = tryExtensions(resolve, joinedPath, ...args);
51+
if (resolved) {
52+
return resolved;
53+
}
54+
}
55+
3256
try {
3357
return resolve(request, ...args);
3458
} catch (_error) {
3559
const nodeError = _error as NodeError;
3660
if (
3761
nodeError.code === 'MODULE_NOT_FOUND'
38-
&& !isBarePackageNamePattern.test(request)
3962
) {
4063
const resolved = (
4164
tryExtensions(resolve, request, ...args)

src/esm/hook/load.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { fileURLToPath } from 'node:url';
2+
import path from 'node:path';
23
import type { LoadHook } from 'node:module';
34
import { readFile } from 'node:fs/promises';
45
import type { TransformOptions } from 'esbuild';
@@ -121,7 +122,11 @@ export const load: LoadHook = async (
121122
code,
122123
filePath,
123124
{
124-
tsconfigRaw: fileMatcher?.(filePath) as TransformOptions['tsconfigRaw'],
125+
tsconfigRaw: (
126+
path.isAbsolute(filePath)
127+
? fileMatcher?.(filePath) as TransformOptions['tsconfigRaw']
128+
: undefined
129+
),
125130
},
126131
);
127132

src/esm/hook/resolve.ts

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
fileUrlPrefix,
1515
tsExtensionsPattern,
1616
isDirectoryPattern,
17-
isBarePackageNamePattern,
17+
isRelativePath,
1818
} from '../../utils/path-utils.js';
1919
import type { TsxRequest } from '../types.js';
2020
import {
@@ -61,7 +61,7 @@ const resolveExtensions = async (
6161
nextResolve: NextResolve,
6262
throwError?: boolean,
6363
) => {
64-
const tryPaths = mapTsExtensions(url, true);
64+
const tryPaths = mapTsExtensions(url);
6565
if (!tryPaths) {
6666
return;
6767
}
@@ -93,16 +93,18 @@ const resolveBase: ResolveHook = async (
9393
context,
9494
nextResolve,
9595
) => {
96-
const isBarePackageName = isBarePackageNamePattern.test(specifier);
97-
98-
// Typescript gives .ts, .cts, or .mts priority over actual .js, .cjs, or .mjs extensions
99-
//
100-
// If `allowJs` is set in `tsconfig.json`, then we'll apply the same resolution logic
101-
// to files without a TypeScript extension.
96+
/**
97+
* Only prioritize TypeScript extensions for file paths (no dependencies)
98+
* TS aliases are pre-resolved so they're file paths
99+
*
100+
* If `allowJs` is set in `tsconfig.json`, then we'll apply the same resolution logic
101+
* to files without a TypeScript extension.
102+
*/
102103
if (
103-
// Ignore if there's no subpath to test extensions against
104-
!isBarePackageName
105-
&& (
104+
(
105+
specifier.startsWith(fileUrlPrefix)
106+
|| isRelativePath(specifier)
107+
) && (
106108
tsExtensionsPattern.test(context.parentURL!)
107109
|| allowJs
108110
)
@@ -139,10 +141,18 @@ const resolveDirectory: ResolveHook = async (
139141
context,
140142
nextResolve,
141143
) => {
144+
if (specifier === '.') {
145+
specifier = './';
146+
}
147+
142148
if (isDirectoryPattern.test(specifier)) {
149+
const urlParsed = new URL(specifier, context.parentURL);
150+
143151
// If directory, can be index.js, index.ts, etc.
152+
urlParsed.pathname = path.join(urlParsed.pathname, 'index');
153+
144154
return (await resolveExtensions(
145-
`${specifier}index`,
155+
urlParsed.toString(),
146156
context,
147157
nextResolve,
148158
true,

src/utils/map-ts-extensions.ts

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,21 @@
11
import path from 'node:path';
2+
import { isFilePath, fileUrlPrefix, nodeModulesPath } from './path-utils.js';
23

3-
const noExtension = ['.js', '.json', '.ts', '.tsx', '.jsx'];
4+
const implicitJsExtensions = ['.js', '.json'];
5+
const implicitTsExtensions = ['.ts', '.tsx', '.jsx'];
46

7+
// Guess extension
8+
const localExtensions = [...implicitTsExtensions, ...implicitJsExtensions];
9+
10+
/**
11+
* If dependency, prioritize .js extensions over .ts
12+
*
13+
* .js is more likely to behave correctly than the .ts file
14+
* https://github.com/evanw/esbuild/releases/tag/v0.20.0
15+
*/
16+
const dependencyExtensions = [...implicitJsExtensions, ...implicitTsExtensions];
17+
18+
// Swap extension
519
const tsExtensions: Record<string, string[]> = Object.create(null);
620
tsExtensions['.js'] = ['.ts', '.tsx', '.js', '.jsx'];
721
tsExtensions['.jsx'] = ['.tsx', '.ts', '.jsx', '.js'];
@@ -10,28 +24,47 @@ tsExtensions['.mjs'] = ['.mts'];
1024

1125
export const mapTsExtensions = (
1226
filePath: string,
13-
handleMissingExtension?: boolean,
1427
) => {
1528
const splitPath = filePath.split('?');
16-
let [pathname] = splitPath;
29+
const pathQuery = splitPath[1] ? `?${splitPath[1]}` : '';
30+
const [pathname] = splitPath;
1731
const extension = path.extname(pathname);
1832

19-
let tryExtensions = tsExtensions[extension];
33+
const tryPaths: string[] = [];
34+
35+
const tryExtensions = tsExtensions[extension];
2036
if (tryExtensions) {
21-
pathname = pathname.slice(0, -extension.length);
22-
} else {
23-
if (!handleMissingExtension) {
24-
return;
25-
}
37+
const extensionlessPath = pathname.slice(0, -extension.length);
2638

27-
tryExtensions = noExtension;
39+
tryPaths.push(
40+
...tryExtensions.map(
41+
extension_ => (
42+
extensionlessPath
43+
+ extension_
44+
+ pathQuery
45+
),
46+
),
47+
);
2848
}
2949

30-
return tryExtensions.map(
31-
tsExtension => (
32-
pathname
33-
+ tsExtension
34-
+ (splitPath[1] ? `?${splitPath[1]}` : '')
50+
const guessExtensions = (
51+
(
52+
!(filePath.startsWith(fileUrlPrefix) || isFilePath(pathname))
53+
|| pathname.includes(nodeModulesPath)
54+
|| pathname.includes('/node_modules/') // For file:// URLs on Windows
55+
)
56+
? dependencyExtensions
57+
: localExtensions
58+
);
59+
tryPaths.push(
60+
...guessExtensions.map(
61+
extension_ => (
62+
pathname
63+
+ extension_
64+
+ pathQuery
65+
),
3566
),
3667
);
68+
69+
return tryPaths;
3770
};

0 commit comments

Comments
 (0)