Skip to content

Commit

Permalink
feat: always enable ESM loader with the new API
Browse files Browse the repository at this point in the history
It does not require a process restart anymore, so safe to enable.
  • Loading branch information
dgozman committed Mar 19, 2024
1 parent b41b802 commit 0f9667c
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 14 deletions.
31 changes: 20 additions & 11 deletions packages/playwright/src/common/configLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,25 +334,33 @@ export async function loadEmptyConfigForMergeReports() {
}

export function restartWithExperimentalTsEsm(configFile: string | undefined, force: boolean = false): boolean {
const nodeVersion = +process.versions.node.split('.')[0];
// New experimental loader is only supported on Node 16+.
if (nodeVersion < 16)
return false;
if (!configFile && !force)
return false;
// Opt-out switch.
if (process.env.PW_DISABLE_TS_ESM)
return false;
// Node.js < 20

// There are two esm loader APIs:
// - Older API that needs a process restart. Available in Node 16, 17, and non-latest 18, 19 and 20.
// - Newer API that works in-process. Available in Node 21+ and latest 18, 19 and 20.

// First check whether we have already restarted with the ESM loader from the older API.
if ((globalThis as any).__esmLoaderPortPreV20) {
// clear execArgv after restart, so that childProcess.fork in user code does not inherit our loader.
process.execArgv = execArgvWithoutExperimentalLoaderOptions();
return false;
}
if (!force && !fileIsModule(configFile!))
return false;

// Node.js < 20
// Now check for the newer API presence.
if (!require('node:module').register) {
// Older API is experimental, only supported on Node 16+.
const nodeVersion = +process.versions.node.split('.')[0];
if (nodeVersion < 16)
return false;

// With older API requiring a process restart, do so conditionally on the config.
const configIsModule = !!configFile && fileIsModule(configFile);
if (!force && !configIsModule)
return false;

const innerProcess = (require('child_process') as typeof import('child_process')).fork(require.resolve('../../cli'), process.argv.slice(2), {
env: {
...process.env,
Expand All @@ -367,7 +375,8 @@ export function restartWithExperimentalTsEsm(configFile: string | undefined, for
});
return true;
}
// Nodejs >= 21

// With the newer API, always enable the ESM loader, because it does not need a restart.
registerESMLoader();
return false;
}
20 changes: 17 additions & 3 deletions tests/playwright-test/esm.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ test('should load cjs config and test in non-ESM mode', async ({ runInlineTest }
expect(result.passed).toBe(2);
});

test('should disallow ESM when config is cjs', async ({ runInlineTest }) => {
test('should allow ESM when config is cjs', async ({ runInlineTest }) => {
const result = await runInlineTest({
'package.json': `{ "type": "module" }`,
'playwright.config.cjs': `
Expand All @@ -567,8 +567,22 @@ test('should disallow ESM when config is cjs', async ({ runInlineTest }) => {
`,
});

expect(result.exitCode).toBe(1);
expect(result.output).toContain('Unknown file extension ".ts"');
expect(result.exitCode).toBe(0);
expect(result.passed).toBe(1);
});

test('should load mts without config', async ({ runInlineTest }) => {
const result = await runInlineTest({
'a.test.mts': `
import { test, expect } from '@playwright/test';
test('check project name', ({}, testInfo) => {
expect(true).toBe(true);
});
`,
});

expect(result.exitCode).toBe(0);
expect(result.passed).toBe(1);
});

test('should be able to use use execSync with a Node.js file inside a spec', async ({ runInlineTest }) => {
Expand Down

0 comments on commit 0f9667c

Please sign in to comment.