Skip to content

Commit 8762407

Browse files
authored
cherry-pick(#32492): Revert "fix(test runner): align with typescript … (#32560)
…behaviour for resolving `index.js` and `package.json` through path mapping (#32078)" This reverts commit effb1ae. This broke path mapping into directories in ESM mode. References #32480.
1 parent f9145b1 commit 8762407

File tree

6 files changed

+15
-167
lines changed

6 files changed

+15
-167
lines changed

packages/playwright-ct-core/src/vitePlugin.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ function vitePlugin(registerSource: string, templateDir: string, buildInfo: Buil
254254

255255
async writeBundle(this: PluginContext) {
256256
for (const importInfo of importInfos.values()) {
257-
const importPath = resolveHook(importInfo.filename, importInfo.importSource, true);
257+
const importPath = resolveHook(importInfo.filename, importInfo.importSource);
258258
if (!importPath)
259259
continue;
260260
const deps = new Set<string>();

packages/playwright-ct-core/src/viteUtils.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,13 +147,13 @@ export async function populateComponentsFromTests(componentRegistry: ComponentRe
147147
for (const importInfo of importList)
148148
componentRegistry.set(importInfo.id, importInfo);
149149
if (componentsByImportingFile)
150-
componentsByImportingFile.set(file, importList.map(i => resolveHook(i.filename, i.importSource, true)).filter(Boolean) as string[]);
150+
componentsByImportingFile.set(file, importList.map(i => resolveHook(i.filename, i.importSource)).filter(Boolean) as string[]);
151151
}
152152
}
153153

154154
export function hasJSComponents(components: ImportInfo[]): boolean {
155155
for (const component of components) {
156-
const importPath = resolveHook(component.filename, component.importSource, true);
156+
const importPath = resolveHook(component.filename, component.importSource);
157157
const extname = importPath ? path.extname(importPath) : '';
158158
if (extname === '.js' || (importPath && !extname && fs.existsSync(importPath + '.js')))
159159
return true;
@@ -183,7 +183,7 @@ export function transformIndexFile(id: string, content: string, templateDir: str
183183
lines.push(registerSource);
184184

185185
for (const value of importInfos.values()) {
186-
const importPath = resolveHook(value.filename, value.importSource, true) || value.importSource;
186+
const importPath = resolveHook(value.filename, value.importSource) || value.importSource;
187187
lines.push(`const ${value.id} = () => import('${importPath?.replaceAll(path.sep, '/')}').then((mod) => mod.${value.remoteName || 'default'});`);
188188
}
189189

packages/playwright/src/transform/esmLoader.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import { fileIsModule } from '../util';
2626
async function resolve(specifier: string, context: { parentURL?: string }, defaultResolve: Function) {
2727
if (context.parentURL && context.parentURL.startsWith('file://')) {
2828
const filename = url.fileURLToPath(context.parentURL);
29-
const resolved = resolveHook(filename, specifier, true);
29+
const resolved = resolveHook(filename, specifier);
3030
if (resolved !== undefined)
3131
specifier = url.pathToFileURL(resolved).toString();
3232
}

packages/playwright/src/transform/transform.ts

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -92,21 +92,15 @@ function loadAndValidateTsconfigsForFile(file: string): ParsedTsConfigData[] {
9292
const pathSeparator = process.platform === 'win32' ? ';' : ':';
9393
const builtins = new Set(Module.builtinModules);
9494

95-
export function resolveHook(filename: string, specifier: string, isESM: boolean): string | undefined {
95+
export function resolveHook(filename: string, specifier: string): string | undefined {
9696
if (specifier.startsWith('node:') || builtins.has(specifier))
9797
return;
9898
if (!shouldTransform(filename))
9999
return;
100100

101101
if (isRelativeSpecifier(specifier))
102-
return resolveImportSpecifierExtension(path.resolve(path.dirname(filename), specifier), false, isESM);
103-
104-
/**
105-
* TypeScript discourages path-mapping into node_modules
106-
* (https://www.typescriptlang.org/docs/handbook/modules/reference.html#paths-should-not-point-to-monorepo-packages-or-node_modules-packages).
107-
* It seems like TypeScript tries path-mapping first, but does not look at the `package.json` or `index.js` files in ESM.
108-
* If path-mapping doesn't yield a result, TypeScript falls back to the default resolution (typically node_modules).
109-
*/
102+
return resolveImportSpecifierExtension(path.resolve(path.dirname(filename), specifier));
103+
110104
const isTypeScript = filename.endsWith('.ts') || filename.endsWith('.tsx');
111105
const tsconfigs = loadAndValidateTsconfigsForFile(filename);
112106
for (const tsconfig of tsconfigs) {
@@ -148,7 +142,7 @@ export function resolveHook(filename: string, specifier: string, isESM: boolean)
148142
if (value.includes('*'))
149143
candidate = candidate.replace('*', matchedPartOfSpecifier);
150144
candidate = path.resolve(tsconfig.pathsBase!, candidate);
151-
const existing = resolveImportSpecifierExtension(candidate, true, isESM);
145+
const existing = resolveImportSpecifierExtension(candidate);
152146
if (existing) {
153147
longestPrefixLength = keyPrefix.length;
154148
pathMatchedByLongestPrefix = existing;
@@ -162,7 +156,7 @@ export function resolveHook(filename: string, specifier: string, isESM: boolean)
162156
if (path.isAbsolute(specifier)) {
163157
// Handle absolute file paths like `import '/path/to/file'`
164158
// Do not handle module imports like `import 'fs'`
165-
return resolveImportSpecifierExtension(specifier, false, isESM);
159+
return resolveImportSpecifierExtension(specifier);
166160
}
167161
}
168162

@@ -244,7 +238,7 @@ function installTransformIfNeeded() {
244238
const originalResolveFilename = (Module as any)._resolveFilename;
245239
function resolveFilename(this: any, specifier: string, parent: Module, ...rest: any[]) {
246240
if (parent) {
247-
const resolved = resolveHook(parent.filename, specifier, false);
241+
const resolved = resolveHook(parent.filename, specifier);
248242
if (resolved !== undefined)
249243
specifier = resolved;
250244
}

packages/playwright/src/util.ts

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ const kExtLookups = new Map([
304304
['.mjs', ['.mts']],
305305
['', ['.js', '.ts', '.jsx', '.tsx', '.cjs', '.mjs', '.cts', '.mts']],
306306
]);
307-
export function resolveImportSpecifierExtension(resolved: string, isPathMapping: boolean, isESM: boolean): string | undefined {
307+
export function resolveImportSpecifierExtension(resolved: string): string | undefined {
308308
if (fileExists(resolved))
309309
return resolved;
310310

@@ -319,25 +319,14 @@ export function resolveImportSpecifierExtension(resolved: string, isPathMapping:
319319
break; // Do not try '' when a more specific extension like '.jsx' matched.
320320
}
321321

322-
// After TypeScript path mapping, here's how directories with a `package.json` are resolved:
323-
// - `package.json#exports` is not respected
324-
// - `package.json#main` is respected only in CJS mode
325-
// - `index.js` default is respected only in CJS mode
326-
//
327-
// More info:
328-
// - https://www.typescriptlang.org/docs/handbook/modules/reference.html#paths-should-not-point-to-monorepo-packages-or-node_modules-packages
329-
// - https://www.typescriptlang.org/docs/handbook/modules/reference.html#directory-modules-index-file-resolution
330-
// - https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#folders-as-modules
331-
332-
const shouldNotResolveDirectory = isPathMapping && isESM;
333-
334-
if (!shouldNotResolveDirectory && dirExists(resolved)) {
322+
if (dirExists(resolved)) {
335323
// If we import a package, let Node.js figure out the correct import based on package.json.
336324
if (fileExists(path.join(resolved, 'package.json')))
337325
return resolved;
338326

327+
// Otherwise, try to find a corresponding index file.
339328
const dirImport = path.join(resolved, 'index');
340-
return resolveImportSpecifierExtension(dirImport, isPathMapping, isESM);
329+
return resolveImportSpecifierExtension(dirImport);
341330
}
342331
}
343332

tests/playwright-test/resolver.spec.ts

Lines changed: 0 additions & 135 deletions
Original file line numberDiff line numberDiff line change
@@ -606,79 +606,6 @@ test('should import packages with non-index main script through path resolver',
606606
expect(result.output).toContain(`foo=42`);
607607
});
608608

609-
test('should not honor `package.json#main` field in ESM mode', async ({ runInlineTest }) => {
610-
const result = await runInlineTest({
611-
'app/pkg/main.ts': `
612-
export const foo = 42;
613-
`,
614-
'app/pkg/package.json': `
615-
{ "main": "main.ts" }
616-
`,
617-
'package.json': `
618-
{ "name": "example-project", "type": "module" }
619-
`,
620-
'playwright.config.ts': `
621-
export default {};
622-
`,
623-
'tsconfig.json': `{
624-
"compilerOptions": {
625-
"baseUrl": ".",
626-
"paths": {
627-
"app/*": ["app/*"],
628-
},
629-
},
630-
}`,
631-
'example.spec.ts': `
632-
import { foo } from 'app/pkg';
633-
import { test, expect } from '@playwright/test';
634-
test('test', ({}) => {
635-
console.log('foo=' + foo);
636-
});
637-
`,
638-
});
639-
640-
expect(result.exitCode).toBe(1);
641-
expect(result.output).toContain(`Cannot find package 'app'`);
642-
});
643-
644-
645-
test('does not honor `exports` field after type mapping', async ({ runInlineTest }) => {
646-
const result = await runInlineTest({
647-
'app/pkg/main.ts': `
648-
export const filename = 'main.ts';
649-
`,
650-
'app/pkg/index.js': `
651-
export const filename = 'index.js';
652-
`,
653-
'app/pkg/package.json': JSON.stringify({
654-
exports: { '.': { require: './main.ts' } }
655-
}),
656-
'package.json': JSON.stringify({
657-
name: 'example-project'
658-
}),
659-
'playwright.config.ts': `
660-
export default {};
661-
`,
662-
'tsconfig.json': JSON.stringify({
663-
compilerOptions: {
664-
baseUrl: '.',
665-
paths: {
666-
'app/*': ['app/*'],
667-
},
668-
}
669-
}),
670-
'example.spec.ts': `
671-
import { filename } from 'app/pkg';
672-
import { test, expect } from '@playwright/test';
673-
test('test', ({}) => {
674-
console.log('filename=' + filename);
675-
});
676-
`,
677-
});
678-
679-
expect(result.output).toContain('filename=index.js');
680-
});
681-
682609
test('should respect tsconfig project references', async ({ runInlineTest }) => {
683610
test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/29256' });
684611

@@ -766,65 +693,3 @@ test('should respect --tsconfig option', async ({ runInlineTest }) => {
766693
expect(result.exitCode).toBe(0);
767694
expect(result.output).not.toContain(`Could not`);
768695
});
769-
770-
771-
test('should resolve index.js in CJS after path mapping', async ({ runInlineTest }) => {
772-
test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/31811' });
773-
774-
const result = await runInlineTest({
775-
'@acme/lib/index.js': `
776-
exports.greet = () => console.log('hello playwright');
777-
`,
778-
'@acme/lib/index.d.ts': `
779-
export const greet: () => void;
780-
`,
781-
'tests/hello.test.ts': `
782-
import { greet } from '@acme/lib';
783-
import { test } from '@playwright/test';
784-
test('hello', async ({}) => {
785-
greet();
786-
});
787-
`,
788-
'tests/tsconfig.json': JSON.stringify({
789-
compilerOptions: {
790-
'paths': {
791-
'@acme/*': ['../@acme/*'],
792-
}
793-
}
794-
})
795-
});
796-
797-
expect(result.exitCode).toBe(0);
798-
expect(result.passed).toBe(1);
799-
});
800-
801-
test('should not resolve index.js in ESM after path mapping', async ({ runInlineTest }) => {
802-
test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/31811' });
803-
804-
const result = await runInlineTest({
805-
'@acme/lib/index.js': `
806-
export const greet = () => console.log('hello playwright');
807-
`,
808-
'@acme/lib/index.d.ts': `
809-
export const greet: () => void;
810-
`,
811-
'tests/hello.test.ts': `
812-
import { greet } from '@acme/lib';
813-
import { test } from '@playwright/test';
814-
test('hello', async ({}) => {
815-
greet();
816-
});
817-
`,
818-
'tests/tsconfig.json': JSON.stringify({
819-
compilerOptions: {
820-
'paths': {
821-
'@acme/*': ['../@acme/*'],
822-
}
823-
}
824-
}),
825-
'package.json': JSON.stringify({ type: 'module' }),
826-
});
827-
828-
expect(result.exitCode).toBe(1);
829-
expect(result.output).toContain(`Cannot find package '@acme/lib'`);
830-
});

0 commit comments

Comments
 (0)