diff --git a/packages/playwright-ct-core/src/vitePlugin.ts b/packages/playwright-ct-core/src/vitePlugin.ts index e4a0a43266..80f4d21774 100644 --- a/packages/playwright-ct-core/src/vitePlugin.ts +++ b/packages/playwright-ct-core/src/vitePlugin.ts @@ -249,7 +249,7 @@ function vitePlugin(registerSource: string, templateDir: string, buildInfo: Buil async writeBundle(this: PluginContext) { for (const importInfo of importInfos.values()) { - const importPath = resolveHook(importInfo.filename, importInfo.importSource); + const importPath = resolveHook(importInfo.filename, importInfo.importSource, true); if (!importPath) continue; const deps = new Set(); diff --git a/packages/playwright-ct-core/src/viteUtils.ts b/packages/playwright-ct-core/src/viteUtils.ts index a9f7020999..d1160c16f0 100644 --- a/packages/playwright-ct-core/src/viteUtils.ts +++ b/packages/playwright-ct-core/src/viteUtils.ts @@ -147,13 +147,13 @@ export async function populateComponentsFromTests(componentRegistry: ComponentRe for (const importInfo of importList) componentRegistry.set(importInfo.id, importInfo); 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 { 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) : ''; if (extname === '.js' || (importPath && !extname && fs.existsSync(importPath + '.js'))) return true; @@ -183,7 +183,7 @@ export function transformIndexFile(id: string, content: string, templateDir: str lines.push(registerSource); 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'});`); } diff --git a/packages/playwright/src/transform/esmLoader.ts b/packages/playwright/src/transform/esmLoader.ts index c84d15146b..0ef0f29eaf 100644 --- a/packages/playwright/src/transform/esmLoader.ts +++ b/packages/playwright/src/transform/esmLoader.ts @@ -26,7 +26,7 @@ import { fileIsModule } from '../util'; async function resolve(specifier: string, context: { parentURL?: string }, defaultResolve: Function) { if (context.parentURL && context.parentURL.startsWith('file://')) { const filename = url.fileURLToPath(context.parentURL); - const resolved = resolveHook(filename, specifier); + const resolved = resolveHook(filename, specifier, true); if (resolved !== undefined) specifier = url.pathToFileURL(resolved).toString(); } diff --git a/packages/playwright/src/transform/transform.ts b/packages/playwright/src/transform/transform.ts index 3ad490d19d..925cf7a83e 100644 --- a/packages/playwright/src/transform/transform.ts +++ b/packages/playwright/src/transform/transform.ts @@ -92,15 +92,21 @@ function loadAndValidateTsconfigsForFile(file: string): ParsedTsConfigData[] { const pathSeparator = process.platform === 'win32' ? ';' : ':'; 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)) return; if (!shouldTransform(filename)) return; 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 tsconfigs = loadAndValidateTsconfigsForFile(filename); for (const tsconfig of tsconfigs) { @@ -142,7 +148,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 = resolveImportSpecifierExtension(candidate, true, isESM); if (existing) { longestPrefixLength = keyPrefix.length; pathMatchedByLongestPrefix = existing; @@ -156,7 +162,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 resolveImportSpecifierExtension(specifier, false, isESM); } } @@ -238,7 +244,7 @@ function installTransformIfNeeded() { const originalResolveFilename = (Module as any)._resolveFilename; function resolveFilename(this: any, specifier: string, parent: Module, ...rest: any[]) { if (parent) { - const resolved = resolveHook(parent.filename, specifier); + const resolved = resolveHook(parent.filename, specifier, false); if (resolved !== undefined) specifier = resolved; } diff --git a/packages/playwright/src/util.ts b/packages/playwright/src/util.ts index a4ddce7a3b..367bb721d9 100644 --- a/packages/playwright/src/util.ts +++ b/packages/playwright/src/util.ts @@ -304,7 +304,7 @@ const kExtLookups = new Map([ ['.mjs', ['.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)) return resolved; @@ -319,14 +319,25 @@ export function resolveImportSpecifierExtension(resolved: string): string | unde break; // Do not try '' when a more specific extension like '.jsx' matched. } - if (dirExists(resolved)) { + // After TypeScript path mapping, here's how directories with a `package.json` are resolved: + // - `package.json#exports` is not respected + // - `package.json#main` is respected only in CJS mode + // - `index.js` default is respected only in CJS mode + // + // More info: + // - https://www.typescriptlang.org/docs/handbook/modules/reference.html#paths-should-not-point-to-monorepo-packages-or-node_modules-packages + // - https://www.typescriptlang.org/docs/handbook/modules/reference.html#directory-modules-index-file-resolution + // - https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#folders-as-modules + + const shouldNotResolveDirectory = isPathMapping && isESM; + + if (!shouldNotResolveDirectory && dirExists(resolved)) { // If we import a package, let Node.js figure out the correct import based on package.json. if (fileExists(path.join(resolved, 'package.json'))) return resolved; - // Otherwise, try to find a corresponding index file. const dirImport = path.join(resolved, 'index'); - return resolveImportSpecifierExtension(dirImport); + return resolveImportSpecifierExtension(dirImport, isPathMapping, isESM); } } diff --git a/tests/playwright-test/resolver.spec.ts b/tests/playwright-test/resolver.spec.ts index 5a0e91b099..b87e22babb 100644 --- a/tests/playwright-test/resolver.spec.ts +++ b/tests/playwright-test/resolver.spec.ts @@ -606,6 +606,79 @@ test('should import packages with non-index main script through path resolver', expect(result.output).toContain(`foo=42`); }); +test('should not honor `package.json#main` field in ESM mode', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'app/pkg/main.ts': ` + export const foo = 42; + `, + 'app/pkg/package.json': ` + { "main": "main.ts" } + `, + 'package.json': ` + { "name": "example-project", "type": "module" } + `, + 'playwright.config.ts': ` + export default {}; + `, + 'tsconfig.json': `{ + "compilerOptions": { + "baseUrl": ".", + "paths": { + "app/*": ["app/*"], + }, + }, + }`, + 'example.spec.ts': ` + import { foo } from 'app/pkg'; + import { test, expect } from '@playwright/test'; + test('test', ({}) => { + console.log('foo=' + foo); + }); + `, + }); + + expect(result.exitCode).toBe(1); + expect(result.output).toContain(`Cannot find package 'app'`); +}); + + +test('does not honor `exports` field after type mapping', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'app/pkg/main.ts': ` + export const filename = 'main.ts'; + `, + 'app/pkg/index.js': ` + export const filename = 'index.js'; + `, + 'app/pkg/package.json': JSON.stringify({ + exports: { '.': { require: './main.ts' } } + }), + 'package.json': JSON.stringify({ + name: 'example-project' + }), + 'playwright.config.ts': ` + export default {}; + `, + 'tsconfig.json': JSON.stringify({ + compilerOptions: { + baseUrl: '.', + paths: { + 'app/*': ['app/*'], + }, + } + }), + 'example.spec.ts': ` + import { filename } from 'app/pkg'; + import { test, expect } from '@playwright/test'; + test('test', ({}) => { + console.log('filename=' + filename); + }); + `, + }); + + expect(result.output).toContain('filename=index.js'); +}); + test('should respect tsconfig project references', async ({ runInlineTest }) => { test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/29256' }); @@ -693,3 +766,65 @@ test('should respect --tsconfig option', async ({ runInlineTest }) => { expect(result.exitCode).toBe(0); 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'`); +});