From 8fe370acf8e4e802f00e640c7ba0eb62069bf5e4 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Thu, 8 Aug 2024 10:21:53 +0200 Subject: [PATCH] apply feedback --- packages/playwright/src/transform/esmLoader.ts | 2 +- packages/playwright/src/transform/transform.ts | 8 ++++---- packages/playwright/src/util.ts | 9 +++------ 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/playwright/src/transform/esmLoader.ts b/packages/playwright/src/transform/esmLoader.ts index 0ef0f29eaf..c84d15146b 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, 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 index 906e1915a0..6514762ace 100644 --- a/packages/playwright/src/transform/transform.ts +++ b/packages/playwright/src/transform/transform.ts @@ -92,14 +92,14 @@ function loadAndValidateTsconfigsForFile(file: string): ParsedTsConfigData[] { const pathSeparator = process.platform === 'win32' ? ';' : ':'; const builtins = new Set(Module.builtinModules); -export function resolveHook(filename: string, specifier: string, dontResolveDirectories = false): 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); + return resolveImportSpecifierExtension(path.resolve(path.dirname(filename), specifier), true); const isTypeScript = filename.endsWith('.ts') || filename.endsWith('.tsx'); const tsconfigs = loadAndValidateTsconfigsForFile(filename); @@ -142,7 +142,7 @@ export function resolveHook(filename: string, specifier: string, dontResolveDire if (value.includes('*')) candidate = candidate.replace('*', matchedPartOfSpecifier); candidate = path.resolve(tsconfig.pathsBase!, candidate); - const existing = resolveImportSpecifierExtension(candidate, dontResolveDirectories); + const existing = resolveImportSpecifierExtension(candidate, false); if (existing) { longestPrefixLength = keyPrefix.length; pathMatchedByLongestPrefix = existing; @@ -156,7 +156,7 @@ export function resolveHook(filename: string, specifier: string, dontResolveDire 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); + return resolveImportSpecifierExtension(specifier, true); } } diff --git a/packages/playwright/src/util.ts b/packages/playwright/src/util.ts index a9dc18934a..ee41e15701 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, dontResolveDirectories: boolean): string | undefined { +export function resolveImportSpecifierExtension(resolved: string, resolveDirectories: boolean): string | undefined { if (fileExists(resolved)) return resolved; @@ -319,17 +319,14 @@ export function resolveImportSpecifierExtension(resolved: string, dontResolveDir break; // Do not try '' when a more specific extension like '.jsx' matched. } - if (dirExists(resolved)) { - if (dontResolveDirectories) - return; - + if (resolveDirectories && 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, dontResolveDirectories); + return resolveImportSpecifierExtension(dirImport, resolveDirectories); } }