From e912b6897d496e5dedc9ce15e2c8e89ead01298f Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Wed, 1 Jun 2022 16:50:23 -0700 Subject: [PATCH] fix(esm): respect source maps in esm-transpiled code (#14561) --- .github/workflows/tests_primary.yml | 30 +++++--------- .../playwright-test/src/experimentalLoader.ts | 2 +- packages/playwright-test/src/transform.ts | 5 ++- tests/playwright-test/esm.spec.ts | 39 ++++++++++++++----- .../playwright-test-fixtures.ts | 6 +++ 5 files changed, 49 insertions(+), 33 deletions(-) diff --git a/.github/workflows/tests_primary.yml b/.github/workflows/tests_primary.yml index 188251d973..05ea1a40e9 100644 --- a/.github/workflows/tests_primary.yml +++ b/.github/workflows/tests_primary.yml @@ -103,6 +103,15 @@ jobs: matrix: os: [ubuntu-latest, windows-latest, macos-latest] node-version: [14] + include: + - os: ubuntu-latest + node-version: 12 + - os: ubuntu-latest + node-version: 16 + - os: ubuntu-latest + node-version: 18 + - os: windows-latest + node-version: 16 runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v2 @@ -123,27 +132,6 @@ jobs: if: always() shell: bash - test_test_runner_esm: - name: Test Runner ESM - strategy: - fail-fast: false - matrix: - os: [ubuntu-latest, windows-latest, macos-latest] - runs-on: ${{ matrix.os }} - steps: - - uses: actions/checkout@v2 - - uses: actions/setup-node@v2 - with: - # ESM tests rely on the experimental loader in Node.js 16+ - node-version: 16 - - run: npm i -g npm@8 - - run: npm ci - env: - DEBUG: pw:install - - run: npm run build - - run: npx playwright install --with-deps - - run: npm run ttest -- esm.spec.ts - test_web_components: name: Web Components runs-on: ubuntu-latest diff --git a/packages/playwright-test/src/experimentalLoader.ts b/packages/playwright-test/src/experimentalLoader.ts index c2d47e8606..3a033e05c8 100644 --- a/packages/playwright-test/src/experimentalLoader.ts +++ b/packages/playwright-test/src/experimentalLoader.ts @@ -32,7 +32,7 @@ async function load(moduleUrl: string, context: any, defaultLoad: any) { if (moduleUrl.startsWith('file://') && (moduleUrl.endsWith('.ts') || moduleUrl.endsWith('.tsx'))) { const filename = url.fileURLToPath(moduleUrl); const code = fs.readFileSync(filename, 'utf-8'); - const source = transformHook(code, filename, true); + const source = transformHook(code, filename, moduleUrl); return { format: 'module', source }; } return defaultLoad(moduleUrl, context, defaultLoad); diff --git a/packages/playwright-test/src/transform.ts b/packages/playwright-test/src/transform.ts index ad19c2d6c9..4592462f78 100644 --- a/packages/playwright-test/src/transform.ts +++ b/packages/playwright-test/src/transform.ts @@ -158,8 +158,9 @@ export function resolveHook(filename: string, specifier: string): string | undef } } -export function transformHook(code: string, filename: string, isModule = false): string { +export function transformHook(code: string, filename: string, moduleUrl?: string): string { // If we are not TypeScript and there is no applicable preprocessor - bail out. + const isModule = !!moduleUrl; const isTypeScript = filename.endsWith('.ts') || filename.endsWith('.tsx'); const isJSX = filename.endsWith('.jsx'); const hasPreprocessor = @@ -173,7 +174,7 @@ export function transformHook(code: string, filename: string, isModule = false): const cachePath = calculateCachePath(code, filename, isModule); const codePath = cachePath + '.js'; const sourceMapPath = cachePath + '.map'; - sourceMaps.set(filename, sourceMapPath); + sourceMaps.set(moduleUrl || filename, sourceMapPath); if (!process.env.PW_IGNORE_COMPILE_CACHE && fs.existsSync(codePath)) return fs.readFileSync(codePath, 'utf8'); // We don't use any browserslist data, but babel checks it anyway. diff --git a/tests/playwright-test/esm.spec.ts b/tests/playwright-test/esm.spec.ts index 7283a3e713..1f077420d6 100644 --- a/tests/playwright-test/esm.spec.ts +++ b/tests/playwright-test/esm.spec.ts @@ -14,9 +14,7 @@ * limitations under the License. */ -import { test, expect } from './playwright-test-fixtures'; - -// Note: tests from this file are additionally run on Node16 bots. +import { test, expect, stripAnsi } from './playwright-test-fixtures'; test('should load nested as esm when package.json has type module', async ({ runInlineTest }) => { const result = await runInlineTest({ @@ -38,9 +36,9 @@ test('should load nested as esm when package.json has type module', async ({ run expect(result.passed).toBe(1); }); -test('should import esm from ts when package.json has type module in experimental mode', async ({ runInlineTest }) => { +test('should import esm from ts when package.json has type module in experimental mode', async ({ runInlineTest, nodeVersion }) => { // We only support experimental esm mode on Node 16+ - test.skip(parseInt(process.version.slice(1), 10) < 16); + test.skip(nodeVersion.major < 16); const result = await runInlineTest({ 'playwright.config.ts': ` import * as fs from 'fs'; @@ -73,9 +71,9 @@ test('should import esm from ts when package.json has type module in experimenta expect(result.exitCode).toBe(0); }); -test('should propagate subprocess exit code in experimental mode', async ({ runInlineTest }) => { +test('should propagate subprocess exit code in experimental mode', async ({ runInlineTest, nodeVersion }) => { // We only support experimental esm mode on Node 16+ - test.skip(parseInt(process.version.slice(1), 10) < 16); + test.skip(nodeVersion.major < 16); const result = await runInlineTest({ 'package.json': JSON.stringify({ type: 'module' }), 'a.test.ts': ` @@ -89,9 +87,9 @@ test('should propagate subprocess exit code in experimental mode', async ({ runI expect(result.exitCode).toBe(1); }); -test('should respect path resolver in experimental mode', async ({ runInlineTest }) => { +test('should respect path resolver in experimental mode', async ({ runInlineTest, nodeVersion }) => { // We only support experimental esm mode on Node 16+ - test.skip(parseInt(process.version.slice(1), 10) < 16); + test.skip(nodeVersion.major < 16); const result = await runInlineTest({ 'package.json': JSON.stringify({ type: 'module' }), 'playwright.config.ts': ` @@ -124,3 +122,26 @@ test('should respect path resolver in experimental mode', async ({ runInlineTest expect(result.exitCode).toBe(0); }); + +test('should use source maps w/ ESM', async ({ runInlineTest, nodeVersion }) => { + // We only support experimental esm mode on Node 16+ + test.skip(nodeVersion.major < 16); + const result = await runInlineTest({ + 'package.json': `{ "type": "module" }`, + 'playwright.config.ts': ` + export default { projects: [{name: 'foo'}] }; + `, + 'a.test.ts': ` + const { test } = pwt; + + test('check project name', ({}, testInfo) => { + expect(testInfo.project.name).toBe('foo'); + }); + ` + }, { reporter: 'list' }); + + const output = stripAnsi(result.output); + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(1); + expect(output).toContain('a.test.ts:7:7'); +}); diff --git a/tests/playwright-test/playwright-test-fixtures.ts b/tests/playwright-test/playwright-test-fixtures.ts index 0277e9845e..4abf4bdcec 100644 --- a/tests/playwright-test/playwright-test-fixtures.ts +++ b/tests/playwright-test/playwright-test-fixtures.ts @@ -218,6 +218,7 @@ type Fixtures = { writeFiles: (files: Files) => Promise; runInlineTest: (files: Files, params?: Params, env?: Env, options?: RunOptions, beforeRunPlaywrightTest?: ({ baseDir }: { baseDir: string }) => Promise) => Promise; runTSC: (files: Files) => Promise; + nodeVersion: { major: number, minor: number, patch: number }, }; export const test = base @@ -249,6 +250,11 @@ export const test = base return { exitCode, output: tsc.output }; }); }, + + nodeVersion: async ({}, use) => { + const [major, minor, patch] = process.versions.node.split('.'); + await use({ major: +major, minor: +minor, patch: +patch }); + }, }); const TSCONFIG = {