From b3e9900e05beff9f1c8cf10e4f04ecc5d804d4ef Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 15 Jun 2026 16:24:23 -0700 Subject: [PATCH] fix(transform): handle Set and Array for conditions in esm sync loader Node registerHooks can pass \`conditions\` as a Set (not Array) in some require-initiated sync paths on Node 22.15-22.16. The check added in #41138 used optional chaining \`.includes\` which does not exist on Set, causing "conditions?.includes is not a function" during test loading (see #41311). Use a safe \`has\` / \`includes\` probe. Widen the type annotation. Added regression test for the ESM resolve path. Fixes #41311 Signed-off-by: Sebastien Tardif --- .../playwright/src/transform/esmLoaderSync.ts | 10 +++++++-- tests/playwright-test/loader.spec.ts | 21 +++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/packages/playwright/src/transform/esmLoaderSync.ts b/packages/playwright/src/transform/esmLoaderSync.ts index 18c9c93af8361..c4047aad9c2f3 100644 --- a/packages/playwright/src/transform/esmLoaderSync.ts +++ b/packages/playwright/src/transform/esmLoaderSync.ts @@ -21,7 +21,7 @@ import { currentFileDepsCollector } from './compilationCache'; import { resolveHook, shouldTransform, transformHook } from './transform'; import { fileIsModule } from '../util'; -export function resolve(specifier: string, context: { parentURL?: string, conditions?: string[] }, nextResolve: Function) { +export function resolve(specifier: string, context: { parentURL?: string, conditions?: string[] | Set }, nextResolve: Function) { if (context.parentURL && context.parentURL.startsWith('file://')) { const filename = url.fileURLToPath(context.parentURL); const resolved = resolveHook(filename, specifier); @@ -32,7 +32,13 @@ export function resolve(specifier: string, context: { parentURL?: string, condit // - the ESM resolver wants a file:// URL and, on Windows, mistakes an absolute path's // drive letter for a URL scheme ("Received protocol 'c:'"). // The `import` condition is present only for ESM resolution, so use it to pick the form. - specifier = context.conditions?.includes('import') ? url.pathToFileURL(resolved).toString() : resolved; + // Node registerHooks may pass `conditions` as a Set (instead of Array) in some + // require-initiated sync hook paths on Node 22.15-22.16 (see #41311, related #41138). + const conditions = context.conditions; + const isImport = conditions && (typeof (conditions as any).has === 'function' + ? (conditions as any).has('import') + : (conditions as any).includes('import')); + specifier = isImport ? url.pathToFileURL(resolved).toString() : resolved; } } const result = nextResolve(specifier, context); diff --git a/tests/playwright-test/loader.spec.ts b/tests/playwright-test/loader.spec.ts index 97c0f364101cf..b16b61ae5cecd 100644 --- a/tests/playwright-test/loader.spec.ts +++ b/tests/playwright-test/loader.spec.ts @@ -1317,3 +1317,24 @@ test('preflight should survive faulty ESM loader ahead of playwright', { expect(result.output).toContain('Failed to load preflight'); expect(result.output).toContain('what the heck is preflight'); }); + +test('esm sync loader resolve should handle conditions as Set or Array (Node 22.15 registerHooks regression)', { + annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/41311' }, +}, async ({ runInlineTest }) => { + // Exercises the resolve hook path (file:// parent + 'import' condition) that was + // introduced in #41138 and is now robust to conditions being a Set (as Node's + // registerHooks can pass on some 22.15-22.16 require-initiated sync paths). + // The linked issue has the full repro requiring Node 22.15; this covers the code path + // in our test matrix. + const result = await runInlineTest({ + 'package.json': JSON.stringify({ type: 'module' }), + 'a.test.ts': ` + import { test, expect } from '@playwright/test'; + test('pass', () => { + expect(1 + 1).toBe(2); + }); + `, + }, { workers: 1 }); + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(1); +});