From e62a54af7acf265f856a714585e4c2c342ceaca1 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 31 Jul 2024 13:17:09 -0700 Subject: [PATCH] fix(test runner): do not revert the transform (#31930) This allows a dynamic import of a TS file to be processed by Babel. For some reason, Playwright used to revert the CJS transforms. However, ESM loader and transforms are always active, so CJS should be too. --- .../src/transform/compilationCache.ts | 8 +--- .../playwright/src/transform/transform.ts | 48 ++++++++----------- tests/playwright-test/loader.spec.ts | 19 ++++++-- 3 files changed, 38 insertions(+), 37 deletions(-) diff --git a/packages/playwright/src/transform/compilationCache.ts b/packages/playwright/src/transform/compilationCache.ts index 060fc717ef..39c1189ea2 100644 --- a/packages/playwright/src/transform/compilationCache.ts +++ b/packages/playwright/src/transform/compilationCache.ts @@ -64,13 +64,7 @@ const fileDependencies = new Map>(); // Dependencies resolved by the external bundler. const externalDependencies = new Map>(); -let sourceMapSupportInstalled = false; - -export function installSourceMapSupportIfNeeded() { - if (sourceMapSupportInstalled) - return; - sourceMapSupportInstalled = true; - +export function installSourceMapSupport() { Error.stackTraceLimit = 200; sourceMapSupport.install({ diff --git a/packages/playwright/src/transform/transform.ts b/packages/playwright/src/transform/transform.ts index 2f6431c4f1..7ba1190bfd 100644 --- a/packages/playwright/src/transform/transform.ts +++ b/packages/playwright/src/transform/transform.ts @@ -25,7 +25,7 @@ import Module from 'module'; import type { BabelPlugin, BabelTransformFunction } from './babelBundle'; import { createFileMatcher, fileIsModule, resolveImportSpecifierExtension } from '../util'; import type { Matcher } from '../util'; -import { getFromCompilationCache, currentFileDepsCollector, belongsToNodeModules, installSourceMapSupportIfNeeded } from './compilationCache'; +import { getFromCompilationCache, currentFileDepsCollector, belongsToNodeModules, installSourceMapSupport } from './compilationCache'; const version = require('../../package.json').version; @@ -201,33 +201,33 @@ function calculateHash(content: string, filePath: string, isModule: boolean, plu } export async function requireOrImport(file: string) { - const revertBabelRequire = installTransform(); + installTransformIfNeeded(); const isModule = fileIsModule(file); - try { - const esmImport = () => eval(`import(${JSON.stringify(url.pathToFileURL(file))})`); - if (isModule) - return await esmImport(); - const result = require(file); - const depsCollector = currentFileDepsCollector(); - if (depsCollector) { - const module = require.cache[file]; - if (module) - collectCJSDependencies(module, depsCollector); - } - return result; - } finally { - revertBabelRequire(); + const esmImport = () => eval(`import(${JSON.stringify(url.pathToFileURL(file))})`); + if (isModule) + return await esmImport(); + const result = require(file); + const depsCollector = currentFileDepsCollector(); + if (depsCollector) { + const module = require.cache[file]; + if (module) + collectCJSDependencies(module, depsCollector); } + return result; } -function installTransform(): () => void { - installSourceMapSupportIfNeeded(); +let transformInstalled = false; - let reverted = false; +function installTransformIfNeeded() { + if (transformInstalled) + return; + transformInstalled = true; + + installSourceMapSupport(); const originalResolveFilename = (Module as any)._resolveFilename; function resolveFilename(this: any, specifier: string, parent: Module, ...rest: any[]) { - if (!reverted && parent) { + if (parent) { const resolved = resolveHook(parent.filename, specifier); if (resolved !== undefined) specifier = resolved; @@ -236,17 +236,11 @@ function installTransform(): () => void { } (Module as any)._resolveFilename = resolveFilename; - const revertPirates = pirates.addHook((code: string, filename: string) => { + pirates.addHook((code: string, filename: string) => { if (!shouldTransform(filename)) return code; return transformHook(code, filename).code; }, { exts: ['.ts', '.tsx', '.js', '.jsx', '.mjs', '.mts', '.cjs', '.cts'] }); - - return () => { - reverted = true; - (Module as any)._resolveFilename = originalResolveFilename; - revertPirates(); - }; } const collectCJSDependencies = (module: Module, dependencies: Set) => { diff --git a/tests/playwright-test/loader.spec.ts b/tests/playwright-test/loader.spec.ts index 491fa3d2bf..9b39733fc3 100644 --- a/tests/playwright-test/loader.spec.ts +++ b/tests/playwright-test/loader.spec.ts @@ -961,10 +961,11 @@ test('should complain when one test file imports another', async ({ runInlineTes expect(result.output).toContain(`test file "a.test.ts" should not import test file "b.test.ts"`); }); -test('should support dynamic imports of js, ts from js, ts and cjs', async ({ runInlineTest }) => { +test('should support dynamic imports and requires of js, ts from js, ts and cjs', async ({ runInlineTest }) => { const result = await runInlineTest({ 'helper.ts': ` - module.exports.foo = 'foo'; + const foo: string = 'foo'; + module.exports.foo = foo; `, 'helper2.ts': ` module.exports.bar = 'bar'; @@ -972,6 +973,10 @@ test('should support dynamic imports of js, ts from js, ts and cjs', async ({ ru 'helper3.js': ` module.exports.baz = 'baz'; `, + 'helper4.ts': ` + const foo: string = 'foo'; + module.exports.foo = foo; + `, 'passthrough.cjs': ` module.exports.load = () => import('./helper2'); `, @@ -1006,8 +1011,16 @@ test('should support dynamic imports of js, ts from js, ts and cjs', async ({ ru expect(foo).toBe('foo'); }); `, + 'd.test.js': ` + import { test, expect } from '@playwright/test'; + + test('pass', async () => { + const { foo } = require('./helper4'); + expect(foo).toBe('foo'); + }); + `, }, { workers: 1 }); - expect(result.passed).toBe(3); + expect(result.passed).toBe(4); expect(result.exitCode).toBe(0); });