Skip to content

Commit

Permalink
fix(resolver): prioritize requested path in dependencies
Browse files Browse the repository at this point in the history
fixes #617
  • Loading branch information
privatenumber authored Jul 30, 2024
1 parent 1c3fd22 commit 3df00f4
Show file tree
Hide file tree
Showing 9 changed files with 168 additions and 69 deletions.
7 changes: 6 additions & 1 deletion src/cjs/api/module-extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { isESM } from '../../utils/es-module-lexer.js';
import { shouldApplySourceMap, inlineSourceMap } from '../../source-map.js';
import { parent } from '../../utils/ipc/client.js';
import { fileMatcher } from '../../utils/tsconfig.js';
import { implicitlyResolvableExtensions } from './resolve-implicit-extensions.js';
import type { LoaderState } from './types.js';

const typescriptExtensions = [
Expand All @@ -24,6 +23,12 @@ const transformExtensions = [
'.mjs',
] as const;

const implicitlyResolvableExtensions = [
'.ts',
'.tsx',
'.jsx',
] as const;

const safeSet = <T extends Record<string, unknown>>(
object: T,
property: keyof T,
Expand Down
61 changes: 35 additions & 26 deletions src/cjs/api/module-resolve-filename.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,19 @@ import Module from 'node:module';
import { fileURLToPath } from 'node:url';
import { mapTsExtensions } from '../../utils/map-ts-extensions.js';
import type { NodeError } from '../../types.js';
import { isRelativePath, fileUrlPrefix, tsExtensionsPattern } from '../../utils/path-utils.js';
import {
isRelativePath,
isFilePath,
fileUrlPrefix,
tsExtensionsPattern,
isDirectoryPattern,
nodeModulesPath,
} from '../../utils/path-utils.js';
import { tsconfigPathsMatcher, allowJs } from '../../utils/tsconfig.js';
import { urlSearchParamsStringify } from '../../utils/url-search-params-stringify.js';
import type { ResolveFilename, SimpleResolve, LoaderState } from './types.js';
import { createImplicitResolver } from './resolve-implicit-extensions.js';

const nodeModulesPath = `${path.sep}node_modules${path.sep}`;

const getOriginalFilePath = (
request: string,
) => {
Expand Down Expand Up @@ -53,8 +58,11 @@ const resolveTsFilename = (
parent: Module.Parent | undefined,
) => {
if (
!(parent?.filename && tsExtensionsPattern.test(parent.filename))
&& !allowJs
isDirectoryPattern.test(request)
|| (
!(parent?.filename && tsExtensionsPattern.test(parent.filename))
&& !allowJs
)
) {
return;
}
Expand Down Expand Up @@ -82,7 +90,7 @@ const resolveTsFilename = (
const resolveRequest = (
request: string,
parent: Module.Parent | undefined,
resolve: SimpleResolve,
nextResolve: SimpleResolve,
) => {
// Support file protocol
if (request.startsWith(fileUrlPrefix)) {
Expand All @@ -102,25 +110,27 @@ const resolveRequest = (
const possiblePaths = tsconfigPathsMatcher(request);

for (const possiblePath of possiblePaths) {
const tsFilename = resolveTsFilename(resolve, possiblePath, parent);
const tsFilename = resolveTsFilename(nextResolve, possiblePath, parent);
if (tsFilename) {
return tsFilename;
}

try {
return resolve(possiblePath);
return nextResolve(possiblePath);
} catch {}
}
}

// If extension exists
const resolvedTsFilename = resolveTsFilename(resolve, request, parent);
if (resolvedTsFilename) {
return resolvedTsFilename;
// It should only try to resolve TS extensions first if it's a local file (non dependency)
if (isFilePath(request)) {
const resolvedTsFilename = resolveTsFilename(nextResolve, request, parent);
if (resolvedTsFilename) {
return resolvedTsFilename;
}
}

try {
return resolve(request);
return nextResolve(request);
} catch (error) {
const nodeError = error as NodeError;

Expand All @@ -133,7 +143,7 @@ const resolveRequest = (
const isExportsPath = nodeError.message.match(/^Cannot find module '([^']+)'$/);
if (isExportsPath) {
const exportsPath = isExportsPath[1];
const tsFilename = resolveTsFilename(resolve, exportsPath, parent);
const tsFilename = resolveTsFilename(nextResolve, exportsPath, parent);
if (tsFilename) {
return tsFilename;
}
Expand All @@ -142,7 +152,7 @@ const resolveRequest = (
const isMainPath = nodeError.message.match(/^Cannot find module '([^']+)'. Please verify that the package.json has a valid "main" entry$/);
if (isMainPath) {
const mainPath = isMainPath[1];
const tsFilename = resolveTsFilename(resolve, mainPath, parent);
const tsFilename = resolveTsFilename(nextResolve, mainPath, parent);
if (tsFilename) {
return tsFilename;
}
Expand Down Expand Up @@ -222,17 +232,16 @@ export const createResolveFilename = (
return nextResolve(request, parent, ...restOfArgs);
}

let _nextResolve = nextResolve;
if (namespace) {
/**
* When namespaced, the loaders are registered to the extensions in a hidden way
* so Node's built-in resolver will not try those extensions
*
* To support implicit extensions, we need to enhance the resolver with our own
* re-implementation of the implicit extension resolution
*/
_nextResolve = createImplicitResolver(_nextResolve);
}
/**
* Custom implicit resolver to resolve .ts over .js extensions
*
* To support implicit extensions, we need to enhance the resolver with our own
* re-implementation of the implicit extension resolution
*
* Also, when namespaced, the loaders are registered to the extensions in a hidden way
* so Node's built-in resolver will not try those extensions
*/
const _nextResolve = createImplicitResolver(nextResolve);

const resolve: SimpleResolve = request_ => _nextResolve(
request_,
Expand Down
43 changes: 33 additions & 10 deletions src/cjs/api/resolve-implicit-extensions.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,17 @@
import path from 'node:path';
import type { NodeError } from '../../types.js';
import { isBarePackageNamePattern } from '../../utils/path-utils.js';
import { isDirectoryPattern } from '../../utils/path-utils.js';
import { mapTsExtensions } from '../../utils/map-ts-extensions.js';
import type { ResolveFilename } from './types.js';

export const implicitlyResolvableExtensions = [
'.ts',
'.tsx',
'.jsx',
] as const;

const tryExtensions = (
resolve: ResolveFilename,
...args: Parameters<ResolveFilename>
) => {
for (const extension of implicitlyResolvableExtensions) {
const tryPaths = mapTsExtensions(args[0]);
for (const tryPath of tryPaths) {
const newArgs = args.slice() as Parameters<ResolveFilename>;
newArgs[0] += extension;
newArgs[0] = tryPath;

try {
return resolve(...newArgs);
Expand All @@ -29,13 +25,40 @@ export const createImplicitResolver = (
request,
...args
) => {
if (request === '.') {
request = './';
}

/**
* Currently, there's an edge case where it doesn't resolve index.ts over index.js
* if the request doesn't end with a slash. e.g. `import './dir'`
* Doesn't handle '.' either
*/
if (isDirectoryPattern.test(request)) {
// If directory, can be index.js, index.ts, etc.
let joinedPath = path.join(request, 'index');

/**
* path.join will remove the './' prefix if it exists
* but it should only be added back if it was there before
* (e.g. not package directory imports)
*/
if (request.startsWith('./')) {
joinedPath = `./${joinedPath}`;
}

const resolved = tryExtensions(resolve, joinedPath, ...args);
if (resolved) {
return resolved;
}
}

try {
return resolve(request, ...args);
} catch (_error) {
const nodeError = _error as NodeError;
if (
nodeError.code === 'MODULE_NOT_FOUND'
&& !isBarePackageNamePattern.test(request)
) {
const resolved = (
tryExtensions(resolve, request, ...args)
Expand Down
7 changes: 6 additions & 1 deletion src/esm/hook/load.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { fileURLToPath } from 'node:url';
import path from 'node:path';
import type { LoadHook } from 'node:module';
import { readFile } from 'node:fs/promises';
import type { TransformOptions } from 'esbuild';
Expand Down Expand Up @@ -121,7 +122,11 @@ export const load: LoadHook = async (
code,
filePath,
{
tsconfigRaw: fileMatcher?.(filePath) as TransformOptions['tsconfigRaw'],
tsconfigRaw: (
path.isAbsolute(filePath)
? fileMatcher?.(filePath) as TransformOptions['tsconfigRaw']
: undefined
),
},
);

Expand Down
34 changes: 22 additions & 12 deletions src/esm/hook/resolve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
fileUrlPrefix,
tsExtensionsPattern,
isDirectoryPattern,
isBarePackageNamePattern,
isRelativePath,
} from '../../utils/path-utils.js';
import type { TsxRequest } from '../types.js';
import {
Expand Down Expand Up @@ -61,7 +61,7 @@ const resolveExtensions = async (
nextResolve: NextResolve,
throwError?: boolean,
) => {
const tryPaths = mapTsExtensions(url, true);
const tryPaths = mapTsExtensions(url);
if (!tryPaths) {
return;
}
Expand Down Expand Up @@ -93,16 +93,18 @@ const resolveBase: ResolveHook = async (
context,
nextResolve,
) => {
const isBarePackageName = isBarePackageNamePattern.test(specifier);

// Typescript gives .ts, .cts, or .mts priority over actual .js, .cjs, or .mjs extensions
//
// If `allowJs` is set in `tsconfig.json`, then we'll apply the same resolution logic
// to files without a TypeScript extension.
/**
* Only prioritize TypeScript extensions for file paths (no dependencies)
* TS aliases are pre-resolved so they're file paths
*
* If `allowJs` is set in `tsconfig.json`, then we'll apply the same resolution logic
* to files without a TypeScript extension.
*/
if (
// Ignore if there's no subpath to test extensions against
!isBarePackageName
&& (
(
specifier.startsWith(fileUrlPrefix)
|| isRelativePath(specifier)
) && (
tsExtensionsPattern.test(context.parentURL!)
|| allowJs
)
Expand Down Expand Up @@ -139,10 +141,18 @@ const resolveDirectory: ResolveHook = async (
context,
nextResolve,
) => {
if (specifier === '.') {
specifier = './';
}

if (isDirectoryPattern.test(specifier)) {
const urlParsed = new URL(specifier, context.parentURL);

// If directory, can be index.js, index.ts, etc.
urlParsed.pathname = path.join(urlParsed.pathname, 'index');

return (await resolveExtensions(
`${specifier}index`,
urlParsed.toString(),
context,
nextResolve,
true,
Expand Down
63 changes: 48 additions & 15 deletions src/utils/map-ts-extensions.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,21 @@
import path from 'node:path';
import { isFilePath, fileUrlPrefix, nodeModulesPath } from './path-utils.js';

const noExtension = ['.js', '.json', '.ts', '.tsx', '.jsx'];
const implicitJsExtensions = ['.js', '.json'];
const implicitTsExtensions = ['.ts', '.tsx', '.jsx'];

// Guess extension
const localExtensions = [...implicitTsExtensions, ...implicitJsExtensions];

/**
* If dependency, prioritize .js extensions over .ts
*
* .js is more likely to behave correctly than the .ts file
* https://github.com/evanw/esbuild/releases/tag/v0.20.0
*/
const dependencyExtensions = [...implicitJsExtensions, ...implicitTsExtensions];

// Swap extension
const tsExtensions: Record<string, string[]> = Object.create(null);
tsExtensions['.js'] = ['.ts', '.tsx', '.js', '.jsx'];
tsExtensions['.jsx'] = ['.tsx', '.ts', '.jsx', '.js'];
Expand All @@ -10,28 +24,47 @@ tsExtensions['.mjs'] = ['.mts'];

export const mapTsExtensions = (
filePath: string,
handleMissingExtension?: boolean,
) => {
const splitPath = filePath.split('?');
let [pathname] = splitPath;
const pathQuery = splitPath[1] ? `?${splitPath[1]}` : '';
const [pathname] = splitPath;
const extension = path.extname(pathname);

let tryExtensions = tsExtensions[extension];
const tryPaths: string[] = [];

const tryExtensions = tsExtensions[extension];
if (tryExtensions) {
pathname = pathname.slice(0, -extension.length);
} else {
if (!handleMissingExtension) {
return;
}
const extensionlessPath = pathname.slice(0, -extension.length);

tryExtensions = noExtension;
tryPaths.push(
...tryExtensions.map(
extension_ => (
extensionlessPath
+ extension_
+ pathQuery
),
),
);
}

return tryExtensions.map(
tsExtension => (
pathname
+ tsExtension
+ (splitPath[1] ? `?${splitPath[1]}` : '')
const guessExtensions = (
(
!(filePath.startsWith(fileUrlPrefix) || isFilePath(pathname))
|| pathname.includes(nodeModulesPath)
|| pathname.includes('/node_modules/') // For file:// URLs on Windows
)
? dependencyExtensions
: localExtensions
);
tryPaths.push(
...guessExtensions.map(
extension_ => (
pathname
+ extension_
+ pathQuery
),
),
);

return tryPaths;
};
Loading

0 comments on commit 3df00f4

Please sign in to comment.