fix(test runner): align with typescript behaviour for resolving index.js and package.json through path mapping
Co-Authored-By: Dmitry Gozman <dgozman@gmail.com>
This commit is contained in:
parent
ea747afcdd
commit
84690a5e94
|
|
@ -249,7 +249,7 @@ function vitePlugin(registerSource: string, templateDir: string, buildInfo: Buil
|
||||||
|
|
||||||
async writeBundle(this: PluginContext) {
|
async writeBundle(this: PluginContext) {
|
||||||
for (const importInfo of importInfos.values()) {
|
for (const importInfo of importInfos.values()) {
|
||||||
const importPath = resolveHook(importInfo.filename, importInfo.importSource);
|
const importPath = resolveHook(importInfo.filename, importInfo.importSource, true);
|
||||||
if (!importPath)
|
if (!importPath)
|
||||||
continue;
|
continue;
|
||||||
const deps = new Set<string>();
|
const deps = new Set<string>();
|
||||||
|
|
|
||||||
|
|
@ -147,13 +147,13 @@ export async function populateComponentsFromTests(componentRegistry: ComponentRe
|
||||||
for (const importInfo of importList)
|
for (const importInfo of importList)
|
||||||
componentRegistry.set(importInfo.id, importInfo);
|
componentRegistry.set(importInfo.id, importInfo);
|
||||||
if (componentsByImportingFile)
|
if (componentsByImportingFile)
|
||||||
componentsByImportingFile.set(file, importList.map(i => resolveHook(i.filename, i.importSource)).filter(Boolean) as string[]);
|
componentsByImportingFile.set(file, importList.map(i => resolveHook(i.filename, i.importSource, true)).filter(Boolean) as string[]);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
export function hasJSComponents(components: ImportInfo[]): boolean {
|
export function hasJSComponents(components: ImportInfo[]): boolean {
|
||||||
for (const component of components) {
|
for (const component of components) {
|
||||||
const importPath = resolveHook(component.filename, component.importSource);
|
const importPath = resolveHook(component.filename, component.importSource, true);
|
||||||
const extname = importPath ? path.extname(importPath) : '';
|
const extname = importPath ? path.extname(importPath) : '';
|
||||||
if (extname === '.js' || (importPath && !extname && fs.existsSync(importPath + '.js')))
|
if (extname === '.js' || (importPath && !extname && fs.existsSync(importPath + '.js')))
|
||||||
return true;
|
return true;
|
||||||
|
|
@ -183,7 +183,7 @@ export function transformIndexFile(id: string, content: string, templateDir: str
|
||||||
lines.push(registerSource);
|
lines.push(registerSource);
|
||||||
|
|
||||||
for (const value of importInfos.values()) {
|
for (const value of importInfos.values()) {
|
||||||
const importPath = resolveHook(value.filename, value.importSource) || value.importSource;
|
const importPath = resolveHook(value.filename, value.importSource, true) || value.importSource;
|
||||||
lines.push(`const ${value.id} = () => import('${importPath?.replaceAll(path.sep, '/')}').then((mod) => mod.${value.remoteName || 'default'});`);
|
lines.push(`const ${value.id} = () => import('${importPath?.replaceAll(path.sep, '/')}').then((mod) => mod.${value.remoteName || 'default'});`);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -26,7 +26,7 @@ import { fileIsModule } from '../util';
|
||||||
async function resolve(specifier: string, context: { parentURL?: string }, defaultResolve: Function) {
|
async function resolve(specifier: string, context: { parentURL?: string }, defaultResolve: Function) {
|
||||||
if (context.parentURL && context.parentURL.startsWith('file://')) {
|
if (context.parentURL && context.parentURL.startsWith('file://')) {
|
||||||
const filename = url.fileURLToPath(context.parentURL);
|
const filename = url.fileURLToPath(context.parentURL);
|
||||||
const resolved = resolveHook(filename, specifier);
|
const resolved = resolveHook(filename, specifier, true);
|
||||||
if (resolved !== undefined)
|
if (resolved !== undefined)
|
||||||
specifier = url.pathToFileURL(resolved).toString();
|
specifier = url.pathToFileURL(resolved).toString();
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -92,15 +92,21 @@ function loadAndValidateTsconfigsForFile(file: string): ParsedTsConfigData[] {
|
||||||
const pathSeparator = process.platform === 'win32' ? ';' : ':';
|
const pathSeparator = process.platform === 'win32' ? ';' : ':';
|
||||||
const builtins = new Set(Module.builtinModules);
|
const builtins = new Set(Module.builtinModules);
|
||||||
|
|
||||||
export function resolveHook(filename: string, specifier: string): string | undefined {
|
export function resolveHook(filename: string, specifier: string, isESM: boolean): string | undefined {
|
||||||
if (specifier.startsWith('node:') || builtins.has(specifier))
|
if (specifier.startsWith('node:') || builtins.has(specifier))
|
||||||
return;
|
return;
|
||||||
if (!shouldTransform(filename))
|
if (!shouldTransform(filename))
|
||||||
return;
|
return;
|
||||||
|
|
||||||
if (isRelativeSpecifier(specifier))
|
if (isRelativeSpecifier(specifier))
|
||||||
return resolveImportSpecifierExtension(path.resolve(path.dirname(filename), specifier));
|
return resolveImportSpecifierExtension(path.resolve(path.dirname(filename), specifier), false, isESM);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 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).
|
||||||
|
* It seems like TypeScript tries path-mapping first, but does not look at the `package.json` or `index.js` files in ESM.
|
||||||
|
* If path-mapping doesn't yield a result, TypeScript falls back to the default resolution (typically node_modules).
|
||||||
|
*/
|
||||||
const isTypeScript = filename.endsWith('.ts') || filename.endsWith('.tsx');
|
const isTypeScript = filename.endsWith('.ts') || filename.endsWith('.tsx');
|
||||||
const tsconfigs = loadAndValidateTsconfigsForFile(filename);
|
const tsconfigs = loadAndValidateTsconfigsForFile(filename);
|
||||||
for (const tsconfig of tsconfigs) {
|
for (const tsconfig of tsconfigs) {
|
||||||
|
|
@ -142,7 +148,7 @@ export function resolveHook(filename: string, specifier: string): string | undef
|
||||||
if (value.includes('*'))
|
if (value.includes('*'))
|
||||||
candidate = candidate.replace('*', matchedPartOfSpecifier);
|
candidate = candidate.replace('*', matchedPartOfSpecifier);
|
||||||
candidate = path.resolve(tsconfig.pathsBase!, candidate);
|
candidate = path.resolve(tsconfig.pathsBase!, candidate);
|
||||||
const existing = resolveImportSpecifierExtension(candidate);
|
const existing = resolveImportSpecifierExtension(candidate, true, isESM);
|
||||||
if (existing) {
|
if (existing) {
|
||||||
longestPrefixLength = keyPrefix.length;
|
longestPrefixLength = keyPrefix.length;
|
||||||
pathMatchedByLongestPrefix = existing;
|
pathMatchedByLongestPrefix = existing;
|
||||||
|
|
@ -156,7 +162,7 @@ export function resolveHook(filename: string, specifier: string): string | undef
|
||||||
if (path.isAbsolute(specifier)) {
|
if (path.isAbsolute(specifier)) {
|
||||||
// Handle absolute file paths like `import '/path/to/file'`
|
// Handle absolute file paths like `import '/path/to/file'`
|
||||||
// Do not handle module imports like `import 'fs'`
|
// Do not handle module imports like `import 'fs'`
|
||||||
return resolveImportSpecifierExtension(specifier);
|
return resolveImportSpecifierExtension(specifier, false, isESM);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -238,7 +244,7 @@ function installTransformIfNeeded() {
|
||||||
const originalResolveFilename = (Module as any)._resolveFilename;
|
const originalResolveFilename = (Module as any)._resolveFilename;
|
||||||
function resolveFilename(this: any, specifier: string, parent: Module, ...rest: any[]) {
|
function resolveFilename(this: any, specifier: string, parent: Module, ...rest: any[]) {
|
||||||
if (parent) {
|
if (parent) {
|
||||||
const resolved = resolveHook(parent.filename, specifier);
|
const resolved = resolveHook(parent.filename, specifier, false);
|
||||||
if (resolved !== undefined)
|
if (resolved !== undefined)
|
||||||
specifier = resolved;
|
specifier = resolved;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -304,7 +304,7 @@ const kExtLookups = new Map([
|
||||||
['.mjs', ['.mts']],
|
['.mjs', ['.mts']],
|
||||||
['', ['.js', '.ts', '.jsx', '.tsx', '.cjs', '.mjs', '.cts', '.mts']],
|
['', ['.js', '.ts', '.jsx', '.tsx', '.cjs', '.mjs', '.cts', '.mts']],
|
||||||
]);
|
]);
|
||||||
export function resolveImportSpecifierExtension(resolved: string): string | undefined {
|
export function resolveImportSpecifierExtension(resolved: string, isPathMapping: boolean, isESM: boolean): string | undefined {
|
||||||
if (fileExists(resolved))
|
if (fileExists(resolved))
|
||||||
return resolved;
|
return resolved;
|
||||||
|
|
||||||
|
|
@ -321,12 +321,16 @@ export function resolveImportSpecifierExtension(resolved: string): string | unde
|
||||||
|
|
||||||
if (dirExists(resolved)) {
|
if (dirExists(resolved)) {
|
||||||
// If we import a package, let Node.js figure out the correct import based on package.json.
|
// If we import a package, let Node.js figure out the correct import based on package.json.
|
||||||
if (fileExists(path.join(resolved, 'package.json')))
|
// TypeScript does not interpret package.json for path mappings: https://www.typescriptlang.org/docs/handbook/modules/reference.html#paths-should-not-point-to-monorepo-packages-or-node_modules-packages
|
||||||
|
if (!isPathMapping && fileExists(path.join(resolved, 'package.json')))
|
||||||
return resolved;
|
return resolved;
|
||||||
|
|
||||||
// Otherwise, try to find a corresponding index file.
|
// Following TypeScript's path mapping logic, index files are still resolved in CommonJS.
|
||||||
const dirImport = path.join(resolved, 'index');
|
const shouldNotResolveIndex = isPathMapping && isESM;
|
||||||
return resolveImportSpecifierExtension(dirImport);
|
if (!shouldNotResolveIndex) {
|
||||||
|
const dirImport = path.join(resolved, 'index');
|
||||||
|
return resolveImportSpecifierExtension(dirImport, isPathMapping, isESM);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -693,3 +693,65 @@ test('should respect --tsconfig option', async ({ runInlineTest }) => {
|
||||||
expect(result.exitCode).toBe(0);
|
expect(result.exitCode).toBe(0);
|
||||||
expect(result.output).not.toContain(`Could not`);
|
expect(result.output).not.toContain(`Could not`);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
||||||
|
test('should resolve index.js in CJS after path mapping', async ({ runInlineTest }) => {
|
||||||
|
test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/31811' });
|
||||||
|
|
||||||
|
const result = await runInlineTest({
|
||||||
|
'@acme/lib/index.js': `
|
||||||
|
exports.greet = () => console.log('hello playwright');
|
||||||
|
`,
|
||||||
|
'@acme/lib/index.d.ts': `
|
||||||
|
export const greet: () => void;
|
||||||
|
`,
|
||||||
|
'tests/hello.test.ts': `
|
||||||
|
import { greet } from '@acme/lib';
|
||||||
|
import { test } from '@playwright/test';
|
||||||
|
test('hello', async ({}) => {
|
||||||
|
greet();
|
||||||
|
});
|
||||||
|
`,
|
||||||
|
'tests/tsconfig.json': JSON.stringify({
|
||||||
|
compilerOptions: {
|
||||||
|
'paths': {
|
||||||
|
'@acme/*': ['../@acme/*'],
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result.exitCode).toBe(0);
|
||||||
|
expect(result.passed).toBe(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('should not resolve index.js in ESM after path mapping', async ({ runInlineTest }) => {
|
||||||
|
test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/31811' });
|
||||||
|
|
||||||
|
const result = await runInlineTest({
|
||||||
|
'@acme/lib/index.js': `
|
||||||
|
export const greet = () => console.log('hello playwright');
|
||||||
|
`,
|
||||||
|
'@acme/lib/index.d.ts': `
|
||||||
|
export const greet: () => void;
|
||||||
|
`,
|
||||||
|
'tests/hello.test.ts': `
|
||||||
|
import { greet } from '@acme/lib';
|
||||||
|
import { test } from '@playwright/test';
|
||||||
|
test('hello', async ({}) => {
|
||||||
|
greet();
|
||||||
|
});
|
||||||
|
`,
|
||||||
|
'tests/tsconfig.json': JSON.stringify({
|
||||||
|
compilerOptions: {
|
||||||
|
'paths': {
|
||||||
|
'@acme/*': ['../@acme/*'],
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}),
|
||||||
|
'package.json': JSON.stringify({ type: 'module' }),
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result.exitCode).toBe(1);
|
||||||
|
expect(result.output).toContain(`Cannot find package '@acme/lib'`);
|
||||||
|
});
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue