From e6d8cf9693668e4eb82360551953bda35096aab5 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Thu, 11 May 2023 21:09:15 -0700 Subject: [PATCH] chore: include plugin list into the cache digest (#22946) Fixes https://github.com/microsoft/playwright/issues/22931 --- .../bundles/babel/src/babelBundleImpl.ts | 42 +++++++++++-------- .../playwright-test/src/common/babelBundle.ts | 4 +- .../src/common/compilationCache.ts | 17 ++------ .../src/common/configLoader.ts | 3 +- .../playwright-test/src/common/transform.ts | 37 ++++++++++++---- .../playwright-test/src/runner/loaderHost.ts | 2 +- .../playwright.ct-build.spec.ts | 25 +++++++++++ 7 files changed, 86 insertions(+), 44 deletions(-) diff --git a/packages/playwright-test/bundles/babel/src/babelBundleImpl.ts b/packages/playwright-test/bundles/babel/src/babelBundleImpl.ts index b2e489a7d6..4e2d8b6bc4 100644 --- a/packages/playwright-test/bundles/babel/src/babelBundleImpl.ts +++ b/packages/playwright-test/bundles/babel/src/babelBundleImpl.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import type { BabelFileResult, NodePath, PluginObj } from '@babel/core'; +import type { BabelFileResult, NodePath, PluginObj, TransformOptions } from '@babel/core'; import type { TSExportAssignment } from '@babel/types'; import type { TemplateBuilder } from '@babel/template'; import * as babel from '@babel/core'; @@ -26,14 +26,7 @@ export { parse } from '@babel/parser'; import traverseFunction from '@babel/traverse'; export const traverse = traverseFunction; - -let additionalPlugins: [string, any?][] = []; - -export function setBabelPlugins(plugins: [string, any?][]) { - additionalPlugins = plugins; -} - -export function babelTransform(filename: string, isTypeScript: boolean, isModule: boolean, scriptPreprocessor: string | undefined): BabelFileResult { +function babelTransformOptions(isTypeScript: boolean, isModule: boolean, pluginsPrologue: [string, any?][], pluginsEpilogue: [string, any?][]): TransformOptions { const plugins = []; if (isTypeScript) { @@ -82,12 +75,7 @@ export function babelTransform(filename: string, isTypeScript: boolean, isModule plugins.push([require('@babel/plugin-syntax-import-assertions')]); } - plugins.unshift(...additionalPlugins.map(([name, options]) => [require(name), options])); - - if (scriptPreprocessor) - plugins.push([scriptPreprocessor]); - - return babel.transformFileSync(filename, { + return { babelrc: false, configFile: false, assumptions: { @@ -98,8 +86,28 @@ export function babelTransform(filename: string, isTypeScript: boolean, isModule presets: [ [require('@babel/preset-typescript'), { onlyRemoveTypeImports: false }], ], - plugins, + plugins: [ + ...pluginsPrologue.map(([name, options]) => [require(name), options]), + ...plugins, + ...pluginsEpilogue.map(([name, options]) => [require(name), options]), + ], compact: false, sourceMaps: 'both', - } as babel.TransformOptions)!; + }; +} + +let isTransforming = false; + +export function babelTransform(filename: string, isTypeScript: boolean, isModule: boolean, pluginsPrologue: [string, any?][], pluginsEpilogue: [string, any?][]): BabelFileResult { + if (isTransforming) + return {}; + + // Prevent reentry while requiring plugins lazily. + isTransforming = true; + try { + const options = babelTransformOptions(isTypeScript, isModule, pluginsPrologue, pluginsEpilogue); + return babel.transformFileSync(filename, options)!; + } finally { + isTransforming = false; + } } diff --git a/packages/playwright-test/src/common/babelBundle.ts b/packages/playwright-test/src/common/babelBundle.ts index a3505b4414..278dd500df 100644 --- a/packages/playwright-test/src/common/babelBundle.ts +++ b/packages/playwright-test/src/common/babelBundle.ts @@ -20,8 +20,8 @@ export const declare: typeof import('../../bundles/babel/node_modules/@types/bab export const types: typeof import('../../bundles/babel/node_modules/@types/babel__core').types = require('./babelBundleImpl').types; export const parse: typeof import('../../bundles/babel/node_modules/@babel/parser/typings/babel-parser').parse = require('./babelBundleImpl').parse; export const traverse: typeof import('../../bundles/babel/node_modules/@types/babel__traverse').default = require('./babelBundleImpl').traverse; -export type BabelTransformFunction = (filename: string, isTypeScript: boolean, isModule: boolean, scriptPreprocessor: string | undefined) => BabelFileResult; -export const setBabelPlugins: (plugins: [string, any?][]) => void = require('./babelBundleImpl').setBabelPlugins; +export type BabelPlugin = [string, any?]; +export type BabelTransformFunction = (filename: string, isTypeScript: boolean, isModule: boolean, pluginsPrefix: BabelPlugin[], pluginsSuffix: BabelPlugin[]) => BabelFileResult; export const babelTransform: BabelTransformFunction = require('./babelBundleImpl').babelTransform; export type { NodePath, types as T } from '../../bundles/babel/node_modules/@types/babel__core'; export type { BabelAPI } from '../../bundles/babel/node_modules/@types/babel__helper-plugin-utils'; diff --git a/packages/playwright-test/src/common/compilationCache.ts b/packages/playwright-test/src/common/compilationCache.ts index 0e6e3cf6cc..7c2fff7f3b 100644 --- a/packages/playwright-test/src/common/compilationCache.ts +++ b/packages/playwright-test/src/common/compilationCache.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import crypto from 'crypto'; import fs from 'fs'; import os from 'os'; import path from 'path'; @@ -26,8 +25,6 @@ export type MemoryCache = { moduleUrl?: string; }; -const version = 13; - const cacheDir = process.env.PWTEST_CACHE_DIR || (() => { if (process.platform === 'win32') return path.join(os.tmpdir(), `playwright-transform-cache`); @@ -68,7 +65,7 @@ function _innerAddToCompilationCache(filename: string, options: { codePath: stri memoryCache.set(filename, options); } -export function getFromCompilationCache(filename: string, code: string, moduleUrl?: string): { cachedCode?: string, addToCache?: (code: string, map?: any) => void } { +export function getFromCompilationCache(filename: string, hash: string, moduleUrl?: string): { cachedCode?: string, addToCache?: (code: string, map?: any) => void } { // First check the memory cache by filename, this cache will always work in the worker, // because we just compiled this file in the loader. const cache = memoryCache.get(filename); @@ -76,8 +73,7 @@ export function getFromCompilationCache(filename: string, code: string, moduleUr return { cachedCode: fs.readFileSync(cache.codePath, 'utf-8') }; // Then do the disk cache, this cache works between the Playwright Test runs. - const isModule = !!moduleUrl; - const cachePath = calculateCachePath(code, filename, isModule); + const cachePath = calculateCachePath(filename, hash); const codePath = cachePath + '.js'; const sourceMapPath = cachePath + '.map'; if (fs.existsSync(codePath)) { @@ -121,14 +117,7 @@ export function addToCompilationCache(payload: any) { externalDependencies.set(entry[0], new Set(entry[1])); } -function calculateCachePath(content: string, filePath: string, isModule: boolean): string { - const hash = crypto.createHash('sha1') - .update(process.env.PW_TEST_SOURCE_TRANSFORM || '') - .update(isModule ? 'esm' : 'no_esm') - .update(content) - .update(filePath) - .update(String(version)) - .digest('hex'); +function calculateCachePath(filePath: string, hash: string): string { const fileName = path.basename(filePath, path.extname(filePath)).replace(/\W/g, '') + '_' + hash; return path.join(cacheDir, hash[0] + hash[1], fileName); } diff --git a/packages/playwright-test/src/common/configLoader.ts b/packages/playwright-test/src/common/configLoader.ts index e2f74a0ced..c0252e1ee3 100644 --- a/packages/playwright-test/src/common/configLoader.ts +++ b/packages/playwright-test/src/common/configLoader.ts @@ -18,12 +18,11 @@ import * as fs from 'fs'; import * as path from 'path'; import { isRegExp } from 'playwright-core/lib/utils'; import type { ConfigCLIOverrides, SerializedConfig } from './ipc'; -import { requireOrImport } from './transform'; +import { requireOrImport, setBabelPlugins } from './transform'; import type { Config, Project } from '../../types/test'; import { errorWithFile } from '../util'; import { setCurrentConfig } from './globals'; import { FullConfigInternal } from './config'; -import { setBabelPlugins } from './babelBundle'; const kDefineConfigWasUsed = Symbol('defineConfigWasUsed'); export const defineConfig = (config: any) => { diff --git a/packages/playwright-test/src/common/transform.ts b/packages/playwright-test/src/common/transform.ts index 84691dea78..f311e4dbb6 100644 --- a/packages/playwright-test/src/common/transform.ts +++ b/packages/playwright-test/src/common/transform.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import crypto from 'crypto'; import path from 'path'; import { sourceMapSupport, pirates } from '../utilsBundle'; import url from 'url'; @@ -21,10 +22,12 @@ import type { Location } from '../../types/testReporter'; import type { TsConfigLoaderResult } from '../third_party/tsconfig-loader'; import { tsConfigLoader } from '../third_party/tsconfig-loader'; import Module from 'module'; -import type { BabelTransformFunction } from './babelBundle'; +import type { BabelPlugin, BabelTransformFunction } from './babelBundle'; import { fileIsModule, resolveImportSpecifierExtension } from '../util'; import { getFromCompilationCache, currentFileDepsCollector, belongsToNodeModules } from './compilationCache'; +const version = require('../../package.json').version; + type ParsedTsConfigData = { absoluteBaseUrl: string; paths: { key: string, values: string[] }[]; @@ -32,6 +35,12 @@ type ParsedTsConfigData = { }; const cachedTSConfigs = new Map(); +let babelPlugins: BabelPlugin[] = []; + +export function setBabelPlugins(plugins: BabelPlugin[]) { + babelPlugins = plugins; +} + function validateTsConfig(tsconfig: TsConfigLoaderResult): ParsedTsConfigData | undefined { if (!tsconfig.tsConfigPath || !tsconfig.baseUrl) return; @@ -57,8 +66,6 @@ function loadAndValidateTsconfigForFile(file: string): ParsedTsConfigData | unde } const pathSeparator = process.platform === 'win32' ? ';' : ':'; -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(filename: string, specifier: string): string | undefined { @@ -128,15 +135,17 @@ export function resolveHook(filename: string, specifier: string): string | undef } export function transformHook(code: string, filename: string, moduleUrl?: string): string { - const { cachedCode, addToCache } = getFromCompilationCache(filename, code, moduleUrl); - if (cachedCode) - return cachedCode; - const isTypeScript = filename.endsWith('.ts') || filename.endsWith('.tsx'); const hasPreprocessor = process.env.PW_TEST_SOURCE_TRANSFORM && process.env.PW_TEST_SOURCE_TRANSFORM_SCOPE && process.env.PW_TEST_SOURCE_TRANSFORM_SCOPE.split(pathSeparator).some(f => filename.startsWith(f)); + const pluginsPrologue = babelPlugins; + const pluginsEpilogue = hasPreprocessor ? [[process.env.PW_TEST_SOURCE_TRANSFORM!]] as BabelPlugin[] : []; + const hash = calculateHash(code, filename, !!moduleUrl, pluginsPrologue, pluginsEpilogue); + const { cachedCode, addToCache } = getFromCompilationCache(filename, hash, moduleUrl); + if (cachedCode) + return cachedCode; // We don't use any browserslist data, but babel checks it anyway. // Silence the annoying warning. @@ -144,7 +153,7 @@ export function transformHook(code: string, filename: string, moduleUrl?: string try { const { babelTransform }: { babelTransform: BabelTransformFunction } = require('./babelBundle'); - const { code, map } = babelTransform(filename, isTypeScript, !!moduleUrl, hasPreprocessor ? scriptPreprocessor : undefined); + const { code, map } = babelTransform(filename, isTypeScript, !!moduleUrl, pluginsPrologue, pluginsEpilogue); if (code) addToCache!(code, map); return code || ''; @@ -155,6 +164,18 @@ export function transformHook(code: string, filename: string, moduleUrl?: string } } +function calculateHash(content: string, filePath: string, isModule: boolean, pluginsPrologue: BabelPlugin[], pluginsEpilogue: BabelPlugin[]): string { + const hash = crypto.createHash('sha1') + .update(isModule ? 'esm' : 'no_esm') + .update(content) + .update(filePath) + .update(version) + .update(pluginsPrologue.map(p => p[0]).join(',')) + .update(pluginsEpilogue.map(p => p[0]).join(',')) + .digest('hex'); + return hash; +} + export async function requireOrImport(file: string) { const revertBabelRequire = installTransform(); const isModule = fileIsModule(file); diff --git a/packages/playwright-test/src/runner/loaderHost.ts b/packages/playwright-test/src/runner/loaderHost.ts index 68ca6cbbb5..4bbade6ecb 100644 --- a/packages/playwright-test/src/runner/loaderHost.ts +++ b/packages/playwright-test/src/runner/loaderHost.ts @@ -22,7 +22,7 @@ import { loadTestFile } from '../common/testLoader'; import type { FullConfigInternal } from '../common/config'; import { PoolBuilder } from '../common/poolBuilder'; import { addToCompilationCache } from '../common/compilationCache'; -import { setBabelPlugins } from '../common/babelBundle'; +import { setBabelPlugins } from '../common/transform'; export class InProcessLoaderHost { private _config: FullConfigInternal; diff --git a/tests/playwright-test/playwright.ct-build.spec.ts b/tests/playwright-test/playwright.ct-build.spec.ts index 24f85906cd..2b341a1ad9 100644 --- a/tests/playwright-test/playwright.ct-build.spec.ts +++ b/tests/playwright-test/playwright.ct-build.spec.ts @@ -411,3 +411,28 @@ test('should work with https enabled', async ({ runInlineTest }) => { expect(result.exitCode).toBe(0); expect(result.passed).toBe(1); }); + +test('list compilation cache should not clash with the run one', async ({ runInlineTest }) => { + const listResult = await runInlineTest({ + 'playwright.config.ts': playwrightConfig, + 'playwright/index.html': ``, + 'playwright/index.ts': ``, + 'src/button.tsx': ` + export const Button = () => ; + `, + 'src/button.spec.tsx': ` + import { test, expect } from '@playwright/experimental-ct-react'; + import { Button } from './button'; + test('pass', async ({ mount }) => { + const component = await mount(); + await expect(component).toHaveText('Button'); + }); + `, + }, { workers: 1 }, {}, { additionalArgs: ['--list'] }); + expect(listResult.exitCode).toBe(0); + expect(listResult.passed).toBe(0); + + const runResult = await runInlineTest({}, { workers: 1 }); + expect(runResult.exitCode).toBe(0); + expect(runResult.passed).toBe(1); +});