diff --git a/.github/.cache-key b/.github/.cache-key index d05221f2f..eb15e7070 100644 --- a/.github/.cache-key +++ b/.github/.cache-key @@ -6,5 +6,5 @@ ; ; / \ _____________/_ __ \_____________ -Times we have broken CI: 4 +Times we have broken CI: 5 Times Windows has broken CI: 99+ diff --git a/.github/workflows/executable-check.yml b/.github/workflows/executable-check.yml index 1b4719914..f6614d2fb 100644 --- a/.github/workflows/executable-check.yml +++ b/.github/workflows/executable-check.yml @@ -37,7 +37,7 @@ jobs: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0 with: - node-version: 14 + node-version: 22 architecture: x64 - name: Build executables (no signing, no upload) run: ./scripts/executable.sh diff --git a/.github/workflows/executable.yml b/.github/workflows/executable.yml index 939f1dd3f..05c2dd0cf 100644 --- a/.github/workflows/executable.yml +++ b/.github/workflows/executable.yml @@ -14,7 +14,7 @@ jobs: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0 with: - node-version: 14 + node-version: 22 architecture: x64 - run: ./scripts/executable.sh env: @@ -53,7 +53,7 @@ jobs: - name: Set up Node.js uses: actions/setup-node@3235b876344d2a9aa001b8d1453c930bba69e610 # v3.9.1 with: - node-version: 14 + node-version: 22 - name: Install resedit run: npm install resedit - name: Update exe metadata diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index c86ab011a..8421b6fa1 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -24,7 +24,7 @@ jobs: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - uses: actions/setup-node@3235b876344d2a9aa001b8d1453c930bba69e610 # v3.9.1 with: - node-version: 14 + node-version: 22 - uses: actions/cache@f4b3439a656ba812b8cb417d2d49f9c810103092 # v3.4.0 with: path: | @@ -32,11 +32,11 @@ jobs: packages/*/node_modules packages/core/.local-chromium key: > - ${{ runner.os }}/node-14/ + ${{ runner.os }}/node-22/ ${{ hashFiles('.github/.cache-key') }}/ ${{ hashFiles('**/yarn.lock') }} restore-keys: > - ${{ runner.os }}/node-14/ + ${{ runner.os }}/node-22/ ${{ hashFiles('.github/.cache-key') }}/ - run: yarn - run: yarn lint diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ad7389b22..6a0d213db 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -25,7 +25,7 @@ jobs: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - uses: actions/setup-node@3235b876344d2a9aa001b8d1453c930bba69e610 # v3.9.1 with: - node-version: 14 + node-version: 22 - uses: actions/cache@f4b3439a656ba812b8cb417d2d49f9c810103092 # v3.4.0 with: path: | @@ -33,11 +33,11 @@ jobs: packages/*/node_modules packages/core/.local-chromium key: > - ${{ runner.os }}/node-14/ + ${{ runner.os }}/node-22/ ${{ hashFiles('.github/.cache-key') }}/ ${{ hashFiles('**/yarn.lock') }} restore-keys: > - ${{ runner.os }}/node-14/ + ${{ runner.os }}/node-22/ ${{ hashFiles('.github/.cache-key') }}/ - run: yarn - run: yarn build @@ -58,7 +58,7 @@ jobs: strategy: matrix: os: [ubuntu-latest] - node: [14] + node: [22] package: - '@percy/env' - '@percy/client' @@ -171,7 +171,7 @@ jobs: fetch-depth: 50 - uses: actions/setup-node@3235b876344d2a9aa001b8d1453c930bba69e610 # v3.9.1 with: - node-version: 14 + node-version: 22 - uses: actions/cache@f4b3439a656ba812b8cb417d2d49f9c810103092 # v3.4.0 with: path: | @@ -179,11 +179,11 @@ jobs: packages/*/node_modules packages/core/.local-chromium key: > - ${{ runner.os }}/node-14/ + ${{ runner.os }}/node-22/ ${{ hashFiles('.github/.cache-key') }}/ ${{ hashFiles('**/yarn.lock') }} restore-keys: > - ${{ runner.os }}/node-14/ + ${{ runner.os }}/node-22/ ${{ hashFiles('.github/.cache-key') }}/ - uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5.0.0 with: diff --git a/.github/workflows/typecheck.yml b/.github/workflows/typecheck.yml index 28284162e..9e07dabc2 100644 --- a/.github/workflows/typecheck.yml +++ b/.github/workflows/typecheck.yml @@ -24,7 +24,7 @@ jobs: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - uses: actions/setup-node@3235b876344d2a9aa001b8d1453c930bba69e610 # v3.9.1 with: - node-version: 14 + node-version: 22 - uses: actions/cache@f4b3439a656ba812b8cb417d2d49f9c810103092 # v3.4.0 with: path: | @@ -32,11 +32,11 @@ jobs: packages/*/node_modules packages/core/.local-chromium key: > - ${{ runner.os }}/node-14/ + ${{ runner.os }}/node-22/ ${{ hashFiles('.github/.cache-key') }}/ ${{ hashFiles('**/yarn.lock') }} restore-keys: > - ${{ runner.os }}/node-14/ + ${{ runner.os }}/node-22/ ${{ hashFiles('.github/.cache-key') }}/ - run: yarn - run: yarn test:types diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index ff9f836d2..f16b56148 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -24,7 +24,7 @@ jobs: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - uses: actions/setup-node@3235b876344d2a9aa001b8d1453c930bba69e610 # v3.9.1 with: - node-version: 14 + node-version: 22 - uses: actions/cache@f4b3439a656ba812b8cb417d2d49f9c810103092 # v3.4.0 with: path: | @@ -32,11 +32,11 @@ jobs: packages/*/node_modules packages/core/.local-chromium key: > - ${{ runner.os }}/node-14/ + ${{ runner.os }}/node-22/ ${{ hashFiles('.github/.cache-key') }}/ ${{ hashFiles('**/yarn.lock') }} restore-keys: > - ${{ runner.os }}/node-14/ + ${{ runner.os }}/node-22/ ${{ hashFiles('.github/.cache-key') }}/ - run: yarn - run: yarn build @@ -85,7 +85,7 @@ jobs: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - uses: actions/setup-node@3235b876344d2a9aa001b8d1453c930bba69e610 # v3.9.1 with: - node-version: 14 + node-version: 22 - uses: actions/cache@f4b3439a656ba812b8cb417d2d49f9c810103092 # v3.4.0 with: path: | @@ -93,11 +93,11 @@ jobs: packages/*/node_modules packages/core/.local-chromium key: > - ${{ runner.os }}/node-14/ + ${{ runner.os }}/node-22/ ${{ hashFiles('.github/.cache-key') }}/ ${{ hashFiles('**/yarn.lock') }} restore-keys: > - ${{ runner.os }}/node-14/ + ${{ runner.os }}/node-22/ ${{ hashFiles('.github/.cache-key') }}/ - uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5.0.0 with: diff --git a/packages/cli-doctor/test/helpers.js b/packages/cli-doctor/test/helpers.js index 8869d9d12..639a738ad 100644 --- a/packages/cli-doctor/test/helpers.js +++ b/packages/cli-doctor/test/helpers.js @@ -55,6 +55,11 @@ export function createHttpServer(handler) { * @param {'block'} [opts.mode] Return 502 for everything * @returns {Promise<{server: net.Server, url: string, port: number, close: function}>} */ +// Minimal proxy double. Every response below closes the socket immediately, so +// each one must say `Connection: close`. Node's http.globalAgent enables +// keepAlive by default from Node 19, and without that header the client pools +// the socket and reuses it for the next request — which then fails with +// ECONNRESET ("socket hang up") because the server already ended it. export function createProxyServer(opts = {}) { return new Promise((resolve, reject) => { const sockets = new Set(); @@ -78,7 +83,9 @@ export function createProxyServer(opts = {}) { // ── block mode ────────────────────────────────────────────────────── if (opts.mode === 'block') { - clientSocket.end('HTTP/1.1 502 Bad Gateway\r\nContent-Length: 0\r\n\r\n'); + clientSocket.end( + 'HTTP/1.1 502 Bad Gateway\r\nConnection: close\r\nContent-Length: 0\r\n\r\n' + ); return; } @@ -89,6 +96,7 @@ export function createProxyServer(opts = {}) { clientSocket.end( 'HTTP/1.1 407 Proxy Authentication Required\r\n' + 'Proxy-Authenticate: Basic realm="proxy"\r\n' + + 'Connection: close\r\n' + 'Content-Length: 0\r\n\r\n' ); return; @@ -97,7 +105,8 @@ export function createProxyServer(opts = {}) { const [user, pass] = decoded.split(':'); if (user !== opts.auth.user || pass !== opts.auth.pass) { clientSocket.end( - 'HTTP/1.1 407 Proxy Authentication Required\r\nContent-Length: 0\r\n\r\n' + 'HTTP/1.1 407 Proxy Authentication Required\r\n' + + 'Connection: close\r\nContent-Length: 0\r\n\r\n' ); return; } @@ -126,7 +135,9 @@ export function createProxyServer(opts = {}) { // ── Plain HTTP proxy ──────────────────────────────────────────────── // For non-CONNECT requests just return 200 (sufficient for our tests) - clientSocket.end('HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n'); + clientSocket.end( + 'HTTP/1.1 200 OK\r\nConnection: close\r\nContent-Length: 0\r\n\r\n' + ); }; clientSocket.on('data', onData); diff --git a/packages/cli-exec/test/exec.test.js b/packages/cli-exec/test/exec.test.js index b29cac633..e22eb05c3 100644 --- a/packages/cli-exec/test/exec.test.js +++ b/packages/cli-exec/test/exec.test.js @@ -17,8 +17,13 @@ describe('percy exec', () => { spyOn(process, 'exit').and.callFake(c => c); process.env.PERCY_CLIENT_ERROR_LOGS = false; - // Ensure global.__MOCK_IMPORTS__ is defined - global.__MOCK_IMPORTS__ = global.__MOCK_IMPORTS__ || new Map(); + // The loader defines this registry when scripts/loader-register.js is + // imported. Substituting a plain Map when it is missing would be worse than + // useless: the loader would not consult it, so every mock below would + // silently no-op while the suite still reported green. + if (!global.__MOCK_IMPORTS__) { + throw new Error('global.__MOCK_IMPORTS__ is undefined — the test loader is not registered'); + } }); afterEach(() => { @@ -275,9 +280,14 @@ describe('percy exec', () => { '[percy] Finalized build #1: https://percy.io/test/test/123' ])); - expect(logger.instance.query(log => log.debug === 'ci')[0].message).toContain([ - 'Some error with secret: [REDACTED]' - ]); + // Each 'data' event on the child's stderr becomes its own ci log entry, so + // indexing [0] assumes the error arrives in the very first chunk. Node 22 + // emits an "[UNDICI-EHPA] EnvHttpProxyAgent is experimental" warning ahead + // of it, which took that slot; stream chunking makes the position + // unreliable in general. Assert the redacted secret is captured somewhere. + expect(logger.instance.query(log => log.debug === 'ci') + .map(log => log.message).join('') + ).toContain('Some error with secret: [REDACTED]'); expect(stderrSpy).toHaveBeenCalled(); }); @@ -325,15 +335,21 @@ describe('percy exec', () => { it('throws when the command receives an error event and stops percy', async () => { let { default: EventEmitter } = await import('events'); let [e, err] = [new EventEmitter(), new Error('spawn error')]; - let crossSpawn = () => (setImmediate(() => e.emit('error', err)), e); + // A spy, not a bare function: spawning the nonexistent `foobar` below fails + // with the real cross-spawn too, so without an assertion on the double this + // spec would pass even when the loader mock never applied. + let crossSpawn = jasmine.createSpy('crossSpawn').and.callFake(() => ( + setImmediate(() => e.emit('error', err)), e + )); global.__MOCK_IMPORTS__.set('cross-spawn', { default: crossSpawn }); let stdinSpy = spyOn(process.stdin, 'pipe').and.resolveTo('some response'); await expectAsync(exec(['--', 'foobar'])).toBeRejected(); + // proves the loader mock actually applied (see §9.1 of the migration plan) + expect(crossSpawn).toHaveBeenCalled(); expect(stdinSpy).toHaveBeenCalled(); - console.log(logger.stderr); expect(logger.stderr).toEqual(jasmine.arrayContaining([ '[percy] Detected error for percy build', '[percy] Failure: Snapshot command was not called', @@ -386,7 +402,7 @@ describe('percy exec', () => { }); it('provides the child process with a percy server address env var', async () => { - let args = ['--no-warnings', '--input-type=module', '--loader=../../scripts/loader.js']; + let args = ['--no-warnings', '--input-type=module', '--import=../../scripts/loader-register.js']; await exec(['--port=4567', '--', 'node', ...args, '--eval', [ 'import { request } from "../cli-command/src/utils.js";', diff --git a/packages/client/test/unit/proxy.test.js b/packages/client/test/unit/proxy.test.js index 7bc93a695..e5f96fbb5 100644 --- a/packages/client/test/unit/proxy.test.js +++ b/packages/client/test/unit/proxy.test.js @@ -86,7 +86,10 @@ describe('proxy', () => { const url = 'http://example.com'; const options = {}; process.env.PERCY_PAC_FILE_URL = 'invalid-url'; - expect(() => proxyAgentFor(url, options)).toThrowError('Failed to initialize PAC proxy: Invalid URL: invalid-url'); + // Matched as a prefix: Node <18 appends the offending input to the + // `Invalid URL` TypeError ("Invalid URL: invalid-url"), Node >=18 does + // not. Asserting the full string pins this test to one Node version. + expect(() => proxyAgentFor(url, options)).toThrowError(/^Failed to initialize PAC proxy: Invalid URL/); }); }); }); diff --git a/packages/config/test/helpers.js b/packages/config/test/helpers.js index a7bd40736..4a17363fc 100644 --- a/packages/config/test/helpers.js +++ b/packages/config/test/helpers.js @@ -31,6 +31,28 @@ const INTERNAL_FILE_REG = new RegExp( // Used to mock javascript modules const JS_FILE_REG = /\.(c|m)?js$/; +// Normalize an fs path argument before matching it against the bypass list. +// +// `fs` accepts a path as a string, a Buffer, a `file:` URL, or a file +// descriptor, but every bypass matcher below is written against a string +// (`p.includes('node_modules')`, `p.match(INTERNAL_FILE_REG)`). On a URL object +// those are `undefined`, so the matcher silently returns falsy and the read is +// routed into the in-memory volume instead of being let through. +// +// This matters from Node 22: `module.registerHooks` intercepts `require()` as +// well as `import` (the old `--experimental-loader` did not), and Node reads +// CommonJS sources through the public `fs` using a URL. Without this, the first +// lazy `require()` of a real dependency inside a mockfs block -- cosmiconfig +// requiring js-yaml to parse a config file -- throws ENOENT. +// +// File descriptors are numbers and are passed through untouched, since the +// descriptor matcher below tests them directly. +function bypassTarget(filepath) { + if (filepath instanceof URL) return url.fileURLToPath(filepath); + if (Buffer.isBuffer(filepath)) return filepath.toString('utf8'); + return filepath; +} + // Mock and spy on fs methods using an in-memory filesystem export async function mockfs({ // set `true` to allow mocking files within `node_modules` (may cause dynamic import issues) @@ -76,9 +98,14 @@ export async function mockfs({ let installFakes = (og, fake) => { for (let k in og) { if (k in fake && typeof og[k] === 'function' && !FS_CLASSES.includes(k)) { - spyOn(og, k).and.callFake((...args) => bypass.some(p => ( - typeof p === 'function' ? p(...args) : (p === args[0]) - )) ? og[k].and.originalFn(...args) : fake[k](...args)); + spyOn(og, k).and.callFake((...args) => { + let [filepath, ...rest] = args; + let target = bypassTarget(filepath); + + return bypass.some(p => ( + typeof p === 'function' ? p(target, ...rest) : (p === target) + )) ? og[k].and.originalFn(...args) : fake[k](...args); + }); } } }; diff --git a/packages/core/post-install.js b/packages/core/post-install.js index c1c491025..ab60d899b 100644 --- a/packages/core/post-install.js +++ b/packages/core/post-install.js @@ -7,7 +7,9 @@ try { } else if (!process.send && fs.existsSync('./src')) { // In development, fork this script with the development loader and always install await import('child_process').then(cp => cp.fork('./post-install.js', { - execArgv: ['--no-warnings', '--loader=../../scripts/loader.js'], + // --import (not --loader): the hooks are registered in-process via + // module.registerHooks. See scripts/loader-register.js. + execArgv: ['--no-warnings', '--import=../../scripts/loader-register.js'], env: { PERCY_POSTINSTALL_BROWSER: true } })); } diff --git a/scripts/executable.sh b/scripts/executable.sh index a195ac8f0..22535f938 100755 --- a/scripts/executable.sh +++ b/scripts/executable.sh @@ -8,7 +8,11 @@ function cleanup { } brew install gnu-sed -npm install -g pkg +# vercel/pkg is archived; its final release (5.8.1) ships no Node 22 base +# binary, so it cannot build this CLI once the toolchain moves off Node 14. +# @yao-pkg/pkg is the maintained fork; its pkg-fetch v3.6 provides prebuilt +# Node 22 binaries for linux, macos and win on both x64 and arm64. +npm install -g @yao-pkg/pkg@6.22.0 yarn install yarn build @@ -47,7 +51,13 @@ cp -R ./build/* packages/ # Create executables. (No `-d`/`--debug`: it only adds per-file "included as # DISCLOSED code / asset content" logging — thousands of lines — without # changing the output binaries.) -pkg ./packages/cli/bin/run.js +# +# Targets are pinned explicitly. Unpinned, pkg infers them from the host, so a +# macOS arm64 runner would silently start emitting an arm64 `percy-osx` — a +# change to what customers download, which is not this migration's call to make. +# Keep the published matrix at x64 and move only the embedded Node to 22; +# whether to add arm64 assets is a separate release decision. +pkg --targets node22-linux-x64,node22-macos-x64,node22-win-x64 ./packages/cli/bin/run.js # Rename executables mv run-linux percy && chmod +x percy diff --git a/scripts/loader-register.js b/scripts/loader-register.js new file mode 100644 index 000000000..b7369b383 --- /dev/null +++ b/scripts/loader-register.js @@ -0,0 +1,25 @@ +// Registers the test loader's module customization hooks. +// +// Loaded with `node --import` (see scripts/test.js) rather than +// `--loader scripts/loader.js`, because `--loader` runs its hooks on a +// dedicated module thread from Node 20.6 onward. The suite's mocking works by +// sharing `global.__MOCK_IMPORTS__` between the hooks and the specs, which a +// separate thread cannot do -- tests would see `undefined` and every +// `__MOCK_IMPORTS__.set()` would throw. +// +// `module.registerHooks` (Node >=22.15) runs the hooks synchronously, in-process +// and in the same realm, which is the behaviour the old `--experimental-loader` +// had on Node 14. That is why this migration targets 22 and not 20: Node 20 only +// has the off-thread `module.register`, so it would need the mock registry moved +// onto a MessagePort with an ack before every dynamic import. +import { registerHooks } from 'module'; +import { resolve, load } from './loader.js'; + +if (typeof registerHooks !== 'function') { + throw new Error( + 'The test loader requires module.registerHooks (Node >=22.15). ' + + `Running Node ${process.version}.` + ); +} + +registerHooks({ resolve, load }); diff --git a/scripts/loader.js b/scripts/loader.js index 136d17bb6..195de001d 100644 --- a/scripts/loader.js +++ b/scripts/loader.js @@ -36,21 +36,36 @@ export const LOADER_ALIAS = { }; // resolve specifier file url -export async function resolve(specifier, context, defaultResolve) { +export function resolve(specifier, context, nextResolve) { + // `module.registerHooks` intercepts require() as well as import, which the + // old --experimental-loader did not. Mock interception has to stay + // import-only: applying it to require() redirects internal CommonJS requires + // into the memfs volume and breaks fs mocking (@percy/config). + let isRequire = context.conditions?.includes('require'); + // check for import or filesystem mocks - if (MOCK_IMPORTS.has(specifier)) { - return { url: `mock://${specifier}?__mock__=${MOCK_IMPORTS.__uid__}&module` }; - } else if (context.parentURL && '$vol' in fs) { + if (!isRequire && MOCK_IMPORTS.has(specifier)) { + return { + url: `mock://${specifier}?__mock__=${MOCK_IMPORTS.__uid__}&module`, + shortCircuit: true + }; + } else if (!isRequire && context.parentURL && '$vol' in fs) { let filename = specifier.startsWith('file:') ? url.fileURLToPath(specifier) : specifier; let filepath = path.resolve(path.dirname(url.fileURLToPath(context.parentURL)), filename); if (fs.$vol.existsSync(filepath)) { let fmt = CJS_REG.test(fs.$vol.readFileSync(filepath)) ? 'commonjs' : 'module'; - return { url: `${url.pathToFileURL(filepath)}?__mock__=${MOCK_IMPORTS.__uid__}&${fmt}` }; + + return { + url: `${url.pathToFileURL(filepath)}?__mock__=${MOCK_IMPORTS.__uid__}&${fmt}`, + shortCircuit: true + }; } } // rewrite dist to src in development + let original = specifier; + if (specifier.startsWith('#')) { let pkgRoot = url.fileURLToPath(context.parentURL.replace(/(packages\/[^/]+\/).+$/, '$1')); let pkgJSON = JSON.parse(fs.readFileSync(path.resolve(pkgRoot, 'package.json'))); @@ -60,18 +75,16 @@ export async function resolve(specifier, context, defaultResolve) { specifier = specifier.replace(LOADER_ALIAS.find, LOADER_ALIAS.replace); } - // transform absolute filepaths into absolute file urls - if (specifier.startsWith(ROOT)) specifier = url.pathToFileURL(specifier).href; + // Transform absolute filepaths into absolute file urls, but only for + // specifiers we actually rewrote. `module.registerHooks` also intercepts + // `require()`, whose resolver rejects a file: URL -- converting every + // in-repo path unconditionally broke requires like babel.config.cjs. + if (specifier !== original && specifier.startsWith(ROOT)) { + specifier = url.pathToFileURL(specifier).href; + } // use default resolve when not mocked - return defaultResolve(specifier, context, defaultResolve); -} - -// get module format for loader mocks -export async function getFormat(srcURL, context, defaultGetFormat) { - return srcURL.includes('?__mock__') - ? { format: srcURL.split('?')[1].split('&')[1] } - : defaultGetFormat(srcURL, context, defaultGetFormat); + return nextResolve(specifier, context); } // generate mock sources for mocked modules @@ -87,26 +100,144 @@ function mockSource(mockURL) { } } -// return loader mocks as module sources -export async function getSource(srcURL, context, defaultGetSource) { - if (srcURL.includes('?__mock__')) return { source: mockSource(srcURL) }; - return defaultGetSource(srcURL, context, defaultGetSource); +// Nearest package.json `type`, mirroring how babel.config.cjs decides between +// its `modules: false` and `modules: 'commonjs'` overrides. The format we hand +// back to Node has to agree with what Babel actually emitted, so it is derived +// the same way rather than sniffed off the output. +const typeCache = new Map(); + +function packageType(filename) { + let dir = path.dirname(filename); + + while (dir.startsWith(ROOT)) { + if (typeCache.has(dir)) return typeCache.get(dir); + let pkg = path.join(dir, 'package.json'); + + if (fs.existsSync(pkg)) { + let type = JSON.parse(fs.readFileSync(pkg, 'utf8')).type === 'module' + ? 'module' : 'commonjs'; + typeCache.set(dir, type); + return type; + } + + let parent = path.dirname(dir); + if (parent === dir) break; + dir = parent; + } + + return 'commonjs'; +} + +// Restore `require.cache` / `require.extensions` on Node 22. +// +// Registering a `load` hook makes Node translate CommonJS reached through +// ESM interop (`import` of a CJS package) via the ESM pipeline rather than the +// classic CJS loader. The `require` that pipeline builds is missing `cache` and +// `extensions` -- it carries only `main` and `resolve`. Verified against a +// bare pass-through `load` hook, so it is not caused by anything this loader +// does, and it is fixed in Node 24. +// +// Nothing can be done from the hook itself: returning no `source` for CommonJS +// is rejected with ERR_INVALID_RETURN_PROPERTY_VALUE. So patch the two +// properties back from inside the module, and only for sources that actually +// reference them -- in practice `import-fresh`, which cosmiconfig requires to +// load .percy.js config files, and which throws +// "Cannot read properties of undefined" on `require.cache[filePath]`. +const REQUIRE_STATICS_REG = /require\.(cache|extensions)\b/; + +const REQUIRE_STATICS_PRELUDE = + 'if(!require.cache){const m=require("module");' + + 'require.cache=m._cache;require.extensions=m._extensions;}'; + +function restoreRequireStatics(result, format) { + if (format !== 'commonjs' || result.source == null) return result; + + let source = typeof result.source === 'string' + ? result.source : Buffer.from(result.source).toString('utf8'); + if (!REQUIRE_STATICS_REG.test(source)) return result; + + return { ...result, source: REQUIRE_STATICS_PRELUDE + source, shortCircuit: true }; } -// return loader mocks or transform sources using babel -export async function transformSource(source, context, defaultTransformSource) { - let callback = (src = source) => defaultTransformSource(src, context, defaultTransformSource); - if (context.format !== 'module' && context.format !== 'commonjs') return callback(); - if (context.url.startsWith('mock://')) return callback(); +// Read a module's source directly, for the cases where Node does not hand +// `source` to the load hook (notably CommonJS). +// +// Deliberately uses the real filesystem via realpath-free readFileSync on the +// *unspied* binding: while a test has fs mocked, `fs.readFileSync` is a jasmine +// spy backed by memfs, so reading through it would serve in-memory content for +// ordinary source files. Memfs-backed modules are handled separately, via the +// `?__mock__` branch in load(). +const realReadFileSync = fs.readFileSync; + +function readSource(loadURL) { + return realReadFileSync(url.fileURLToPath(loadURL.split('?')[0]), 'utf8'); +} + +// Return loader mocks, or transform sources using babel. +// +// Replaces the getFormat/getSource/transformSource trio this file used to +// export. Those hooks were removed in Node 16.12 and silently stopped being +// called, which is what pinned the suite to Node 14: without the Babel step +// the test files load as native ESM (frozen namespaces, no `__dirname`), and +// without a shared realm the `__MOCK_IMPORTS__` interception never matched. +// A single synchronous `load` registered via `module.registerHooks` restores +// both behaviours. See scripts/loader-register.js. +export function load(loadURL, context, nextLoad) { + // synthesized mock module, or a memfs-backed file + if (loadURL.includes('?__mock__')) { + let format = loadURL.split('?')[1].split('&')[1]; + let source = mockSource(loadURL); + + // `mock://` modules are generated re-export shims and must be left alone, + // but memfs-backed files are real sources -- the old transformSource hook + // skipped only the former, so keep transforming the latter. + if (!loadURL.startsWith('mock://')) { + source = transform(source, url.fileURLToPath(loadURL.split('?')[0]), format) ?? source; + } + + return { format, source, shortCircuit: true }; + } + + let result = nextLoad(loadURL, context); + let format = result.format ?? context.format; + + // only our own src/test files get transformed + if (format !== 'module' && format !== 'commonjs') return result; + if (!loadURL.startsWith('file:')) return result; + + let filename = url.fileURLToPath(loadURL.split('?')[0]); + if (!BABEL_REG.test(filename)) return restoreRequireStatics(result, format); + + let source = result.source; + + if (source == null) { + try { source = readSource(loadURL); } catch { return result; } + } + + let transformed = transform(source, filename, format); + + // `only` misses turn into a null result -- keep Node's original module. + if (transformed == null) return result; + + return { + format: packageType(filename), + source: transformed, + shortCircuit: true + }; +} - if (typeof source !== 'string') source = Buffer.from(source); - if (Buffer.isBuffer(source)) source = source.toString(); +// Babel-transform a module source, or return null when Babel's `only` filter +// does not match it. `unambiguous` lets Babel classify the input itself; the +// old code forwarded Node's format, but Babel's sourceType has no 'commonjs' +// member, so a CommonJS file would have been mis-declared. +function transform(source, filename, format) { + if (typeof source !== 'string') source = Buffer.from(source).toString('utf8'); - return callback((await babel.transformAsync(source, { - filename: url.fileURLToPath(context.url), - sourceType: context.format, + return babel.transformSync(source, { + filename, + sourceType: 'unambiguous', babelrcRoots: ['.'], rootMode: 'upward', only: [BABEL_REG] - }))?.code); + })?.code ?? null; } diff --git a/scripts/test.js b/scripts/test.js index 64cde2f8a..eb30227bf 100644 --- a/scripts/test.js +++ b/scripts/test.js @@ -88,8 +88,11 @@ async function main({ } else if (!process.send) { // test runners assume they have control over the entire process, so give them each forks let flags = flagify({ coverage, karma: karmaArgs }); - let loader = url.pathToFileURL(path.resolve(filename, '../loader.js')).href; - let opts = { execArgv: ['--loader', loader, ...process.execArgv] }; + // --import (not --loader): the hooks must run in-process and share a realm + // with the specs so global.__MOCK_IMPORTS__ is the same object on both + // sides. See scripts/loader-register.js. + let loader = url.pathToFileURL(path.resolve(filename, '../loader-register.js')).href; + let opts = { execArgv: ['--import', loader, ...process.execArgv] }; if (testNode) { await child('fork', filename, ['--node', ...flags], opts);