From fb9555fb5da1fd5b9fcc15134f25a3b2bf29e210 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Thu, 20 Oct 2022 15:21:22 -0400 Subject: [PATCH] fix(ts): resolve .js to .ts in non-ESM mode too (#18219) Fixes https://github.com/microsoft/playwright/issues/18077 --- .../playwright-test/src/experimentalLoader.ts | 2 +- packages/playwright-test/src/transform.ts | 33 ++++++++----------- tests/playwright-test/loader.spec.ts | 19 +++++++++++ 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/packages/playwright-test/src/experimentalLoader.ts b/packages/playwright-test/src/experimentalLoader.ts index 35a5adfca7..67c18f0631 100644 --- a/packages/playwright-test/src/experimentalLoader.ts +++ b/packages/playwright-test/src/experimentalLoader.ts @@ -23,7 +23,7 @@ import { transformHook, resolveHook, belongsToNodeModules } from './transform'; 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(true, filename, specifier); + const resolved = resolveHook(filename, specifier); if (resolved !== undefined) specifier = url.pathToFileURL(resolved).toString(); } diff --git a/packages/playwright-test/src/transform.ts b/packages/playwright-test/src/transform.ts index 5add8c15c1..3e19ad29d1 100644 --- a/packages/playwright-test/src/transform.ts +++ b/packages/playwright-test/src/transform.ts @@ -97,15 +97,14 @@ const scriptPreprocessor = process.env.PW_TEST_SOURCE_TRANSFORM ? require(process.env.PW_TEST_SOURCE_TRANSFORM) : undefined; const builtins = new Set(Module.builtinModules); -export function resolveHook(isModule: boolean, filename: string, specifier: string): string | undefined { +export function resolveHook(filename: string, specifier: string): string | undefined { if (builtins.has(specifier)) return; - // In real life, playwright-test is under node_modules, but in the tests it isn't. - if (filename.startsWith(kPlaywrightInternalPrefix)) + if (belongsToNodeModules(filename)) return; if (isRelativeSpecifier(specifier)) - return isModule ? js2ts(path.resolve(path.dirname(filename), specifier)) : undefined; + return js2ts(path.resolve(path.dirname(filename), specifier)); const isTypeScript = filename.endsWith('.ts') || filename.endsWith('.tsx'); const tsconfig = loadAndValidateTsconfigForFile(filename); @@ -138,27 +137,24 @@ export function resolveHook(isModule: boolean, filename: string, specifier: stri matchedPartOfSpecifier = specifier; } + if (keyPrefix.length <= longestPrefixLength) + continue; + for (const value of values) { let candidate: string = value; if (value.includes('*')) candidate = candidate.replace('*', matchedPartOfSpecifier); candidate = path.resolve(tsconfig.absoluteBaseUrl, candidate.replace(/\//g, path.sep)); - if (isModule) { - const transformed = js2ts(candidate); - if (transformed || fs.existsSync(candidate)) { - if (keyPrefix.length > longestPrefixLength) { - longestPrefixLength = keyPrefix.length; - pathMatchedByLongestPrefix = transformed || candidate; - } - } + const ts = js2ts(candidate); + if (ts) { + longestPrefixLength = keyPrefix.length; + pathMatchedByLongestPrefix = ts; } else { for (const ext of ['', '.js', '.ts', '.mjs', '.cjs', '.jsx', '.tsx', '.cjs', '.mts', '.cts']) { if (fs.existsSync(candidate + ext)) { - if (keyPrefix.length > longestPrefixLength) { - longestPrefixLength = keyPrefix.length; - pathMatchedByLongestPrefix = candidate; - } + longestPrefixLength = keyPrefix.length; + pathMatchedByLongestPrefix = candidate; } } } @@ -168,8 +164,7 @@ export function resolveHook(isModule: boolean, filename: string, specifier: stri return pathMatchedByLongestPrefix; } - if (isModule) - return js2ts(path.resolve(path.dirname(filename), specifier)); + return js2ts(path.resolve(path.dirname(filename), specifier)); } export function js2ts(resolved: string): string | undefined { @@ -223,7 +218,7 @@ export function installTransform(): () => void { const originalResolveFilename = (Module as any)._resolveFilename; function resolveFilename(this: any, specifier: string, parent: Module, ...rest: any[]) { if (!reverted && parent) { - const resolved = resolveHook(false, parent.filename, specifier); + const resolved = resolveHook(parent.filename, specifier); if (resolved !== undefined) specifier = resolved; } diff --git a/tests/playwright-test/loader.spec.ts b/tests/playwright-test/loader.spec.ts index 351550d40a..b4766f40ff 100644 --- a/tests/playwright-test/loader.spec.ts +++ b/tests/playwright-test/loader.spec.ts @@ -488,3 +488,22 @@ test('should remove type imports from ts', async ({ runInlineTest }) => { expect(result.passed).toBe(1); expect(result.exitCode).toBe(0); }); + +test('should resolve .js import to .ts file in non-ESM mode', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.test.ts': ` + const { test } = pwt; + import { gimmeAOne } from './playwright-utils.js'; + test('pass', ({}) => { + expect(gimmeAOne()).toBe(1); + }); + `, + 'playwright-utils.ts': ` + export function gimmeAOne() { + return 1; + } + `, + }); + expect(result.passed).toBe(1); + expect(result.exitCode).toBe(0); +});