microsoft · dgozman · Sep 11, 2024 · Sep 6, 2024
diff --git a/packages/playwright-ct-core/src/vitePlugin.ts b/packages/playwright-ct-core/src/vitePlugin.ts
@@ -254,7 +254,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, true);
+        const importPath = resolveHook(importInfo.filename, importInfo.importSource);
         if (!importPath)
           continue;
         const deps = new Set<string>();

diff --git 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, true)).filter(Boolean) as string[]);
+      componentsByImportingFile.set(file, importList.map(i => resolveHook(i.filename, i.importSource)).filter(Boolean) as string[]);
   }
 }
 
 export function hasJSComponents(components: ImportInfo[]): boolean {
   for (const component of components) {
-    const importPath = resolveHook(component.filename, component.importSource, true);
+    const importPath = resolveHook(component.filename, component.importSource);
     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, true) || value.importSource;
+    const importPath = resolveHook(value.filename, value.importSource) || 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
@@ -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, true);
+    const resolved = resolveHook(filename, specifier);
     if (resolved !== undefined)
       specifier = url.pathToFileURL(resolved).toString();
   }

diff --git a/packages/playwright/src/transform/transform.ts b/packages/playwright/src/transform/transform.ts
@@ -92,21 +92,15 @@ function loadAndValidateTsconfigsForFile(file: string): ParsedTsConfigData[] {
 const pathSeparator = process.platform === 'win32' ? ';' : ':';
 const builtins = new Set(Module.builtinModules);
 
-export function resolveHook(filename: string, specifier: string, isESM: boolean): string | undefined {
+export function resolveHook(filename: string, specifier: string): 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), 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).
-   */
+    return resolveImportSpecifierExtension(path.resolve(path.dirname(filename), specifier));
+
   const isTypeScript = filename.endsWith('.ts') || filename.endsWith('.tsx');
   const tsconfigs = loadAndValidateTsconfigsForFile(filename);
   for (const tsconfig of tsconfigs) {
@@ -148,7 +142,7 @@ export function resolveHook(filename: string, specifier: string, isESM: boolean)
         if (value.includes('*'))
           candidate = candidate.replace('*', matchedPartOfSpecifier);
         candidate = path.resolve(tsconfig.pathsBase!, candidate);
-        const existing = resolveImportSpecifierExtension(candidate, true, isESM);
+        const existing = resolveImportSpecifierExtension(candidate);
         if (existing) {
           longestPrefixLength = keyPrefix.length;
           pathMatchedByLongestPrefix = existing;
@@ -162,7 +156,7 @@ export function resolveHook(filename: string, specifier: string, isESM: boolean)
   if (path.isAbsolute(specifier)) {
     // Handle absolute file paths like `import '/path/to/file'`
     // Do not handle module imports like `import 'fs'`
-    return resolveImportSpecifierExtension(specifier, false, isESM);
+    return resolveImportSpecifierExtension(specifier);
   }
 }
 
@@ -244,7 +238,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, false);
+      const resolved = resolveHook(parent.filename, specifier);
       if (resolved !== undefined)
         specifier = resolved;
     }

diff --git 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, isPathMapping: boolean, isESM: boolean): string | undefined {
+export function resolveImportSpecifierExtension(resolved: string): string | undefined {
   if (fileExists(resolved))
     return resolved;
 
@@ -319,25 +319,14 @@ export function resolveImportSpecifierExtension(resolved: string, isPathMapping:
     break;  // Do not try '' when a more specific extension like '.jsx' matched.
   }
 
-  // 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 (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, isPathMapping, isESM);
+    return resolveImportSpecifierExtension(dirImport);
   }
 }
 

diff --git a/tests/playwright-test/resolver.spec.ts b/tests/playwright-test/resolver.spec.ts
@@ -606,79 +606,6 @@ 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' });
 
@@ -766,65 +693,3 @@ 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'`);
-});