Skip to content

Commit

Permalink
cherry-pick(#32491): fix(test runner): allow directory imports with p…
Browse files Browse the repository at this point in the history
…ath mapping (#32571)

We now hopefully align with `moduleResolution: bundler` tsconfig option,
allowing directory imports in every scenario, and allowing proper module
imports when not going through the type mapping.

This regressed in #32078. Fixes #32480, fixes #31811.
  • Loading branch information
dgozman committed Sep 12, 2024
1 parent 8762407 commit 13e6e48
Show file tree
Hide file tree
Showing 3 changed files with 484 additions and 46 deletions.
13 changes: 9 additions & 4 deletions packages/playwright/src/transform/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import type { LoadedTsConfig } from '../third_party/tsconfig-loader';
import { tsConfigLoader } from '../third_party/tsconfig-loader';
import Module from 'module';
import type { BabelPlugin, BabelTransformFunction } from './babelBundle';
import { createFileMatcher, fileIsModule, resolveImportSpecifierExtension } from '../util';
import { createFileMatcher, fileIsModule, resolveImportSpecifierAfterMapping } from '../util';
import type { Matcher } from '../util';
import { getFromCompilationCache, currentFileDepsCollector, belongsToNodeModules, installSourceMapSupport } from './compilationCache';

Expand Down Expand Up @@ -99,8 +99,13 @@ export function resolveHook(filename: string, specifier: string): string | undef
return;

if (isRelativeSpecifier(specifier))
return resolveImportSpecifierExtension(path.resolve(path.dirname(filename), specifier));
return resolveImportSpecifierAfterMapping(path.resolve(path.dirname(filename), specifier), false);

/**
* TypeScript discourages path-mapping into node_modules:
* https://www.typescriptlang.org/docs/handbook/modules/reference.html#paths-should-not-point-to-monorepo-packages-or-node_modules-packages
* However, if path-mapping doesn't yield a result, TypeScript falls back to the default resolution through node_modules.
*/
const isTypeScript = filename.endsWith('.ts') || filename.endsWith('.tsx');
const tsconfigs = loadAndValidateTsconfigsForFile(filename);
for (const tsconfig of tsconfigs) {
Expand Down Expand Up @@ -142,7 +147,7 @@ export function resolveHook(filename: string, specifier: string): string | undef
if (value.includes('*'))
candidate = candidate.replace('*', matchedPartOfSpecifier);
candidate = path.resolve(tsconfig.pathsBase!, candidate);
const existing = resolveImportSpecifierExtension(candidate);
const existing = resolveImportSpecifierAfterMapping(candidate, true);
if (existing) {
longestPrefixLength = keyPrefix.length;
pathMatchedByLongestPrefix = existing;
Expand All @@ -156,7 +161,7 @@ export function resolveHook(filename: string, specifier: string): string | undef
if (path.isAbsolute(specifier)) {
// Handle absolute file paths like `import '/path/to/file'`
// Do not handle module imports like `import 'fs'`
return resolveImportSpecifierExtension(specifier);
return resolveImportSpecifierAfterMapping(specifier, false);
}
}

Expand Down
57 changes: 52 additions & 5 deletions packages/playwright/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,16 +295,31 @@ function folderIsModule(folder: string): boolean {
return require(packageJsonPath).type === 'module';
}

// This follows the --moduleResolution=bundler strategy from tsc.
// https://devblogs.microsoft.com/typescript/announcing-typescript-5-0-beta/#moduleresolution-bundler
const packageJsonMainFieldCache = new Map<string, string | undefined>();

function getMainFieldFromPackageJson(packageJsonPath: string) {
if (!packageJsonMainFieldCache.has(packageJsonPath)) {
let mainField: string | undefined;
try {
mainField = JSON.parse(fs.readFileSync(packageJsonPath, 'utf8')).main;
} catch {
}
packageJsonMainFieldCache.set(packageJsonPath, mainField);
}
return packageJsonMainFieldCache.get(packageJsonPath);
}

// This method performs "file extension subsitution" to find the ts, js or similar source file
// based on the import specifier, which might or might not have an extension. See TypeScript docs:
// https://www.typescriptlang.org/docs/handbook/modules/reference.html#file-extension-substitution.
const kExtLookups = new Map([
['.js', ['.jsx', '.ts', '.tsx']],
['.jsx', ['.tsx']],
['.cjs', ['.cts']],
['.mjs', ['.mts']],
['', ['.js', '.ts', '.jsx', '.tsx', '.cjs', '.mjs', '.cts', '.mts']],
]);
export function resolveImportSpecifierExtension(resolved: string): string | undefined {
function resolveImportSpecifierExtension(resolved: string): string | undefined {
if (fileExists(resolved))
return resolved;

Expand All @@ -318,13 +333,45 @@ export function resolveImportSpecifierExtension(resolved: string): string | unde
}
break; // Do not try '' when a more specific extension like '.jsx' matched.
}
}

// This method resolves directory imports and performs "file extension subsitution".
// It is intended to be called after the path mapping resolution.
//
// Directory imports follow the --moduleResolution=bundler strategy from tsc.
// https://www.typescriptlang.org/docs/handbook/modules/reference.html#directory-modules-index-file-resolution
// https://www.typescriptlang.org/docs/handbook/modules/reference.html#bundler
//
// See also Node.js "folder as module" behavior:
// https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#folders-as-modules.
export function resolveImportSpecifierAfterMapping(resolved: string, afterPathMapping: boolean): string | undefined {
const resolvedFile = resolveImportSpecifierExtension(resolved);
if (resolvedFile)
return resolvedFile;

if (dirExists(resolved)) {
const packageJsonPath = path.join(resolved, 'package.json');

if (afterPathMapping) {
// Most notably, the module resolution algorithm is not performed after the path mapping.
// This means no node_modules lookup or package.json#exports.
//
// Only the "folder as module" Node.js behavior is respected:
// - consult `package.json#main`;
// - look for `index.js` or similar.
const mainField = getMainFieldFromPackageJson(packageJsonPath);
const mainFieldResolved = mainField ? resolveImportSpecifierExtension(path.resolve(resolved, mainField)) : undefined;
return mainFieldResolved || resolveImportSpecifierExtension(path.join(resolved, 'index'));
}

// If we import a package, let Node.js figure out the correct import based on package.json.
if (fileExists(path.join(resolved, 'package.json')))
// This also covers the "main" field for "folder as module".
if (fileExists(packageJsonPath))
return resolved;

// Otherwise, try to find a corresponding index file.
// Implement the "folder as module" Node.js behavior.
// Note that we do not delegate to Node.js, because we support this for ESM as well,
// following the TypeScript "bundler" mode.
const dirImport = path.join(resolved, 'index');
return resolveImportSpecifierExtension(dirImport);
}
Expand Down
Loading

0 comments on commit 13e6e48

Please sign in to comment.