diff --git a/packages/cli/src/constructs/playwright-check.ts b/packages/cli/src/constructs/playwright-check.ts index ef9ad906..f2c44016 100644 --- a/packages/cli/src/constructs/playwright-check.ts +++ b/packages/cli/src/constructs/playwright-check.ts @@ -26,7 +26,7 @@ export interface PlaywrightCheckProps extends Omit { if (!this.engine && Session.basePath) { - Session.detectedEnginePromise ??= detectEngine(Session.basePath).then(e => e ?? null) + Session.detectedEnginePromise ??= detectEngine(Session.basePath, Session.contextPath) + .then(e => e ?? null) const result = await Session.detectedEnginePromise if (result) { this.engine = result.engine @@ -392,7 +393,7 @@ export class PlaywrightCheck extends RuntimeCheck { notice + '\n\n' + `Hint: The value was automatically detected from your ` - + `project's version files (.node-version, .nvmrc, etc.). ` + + `project's version files (.node-version, .nvmrc, package.json volta/engines, etc.). ` + `To override, set "engine" explicitly using Engine.node() ` + `or Engine.bun().`), )) diff --git a/packages/cli/src/services/__tests__/engine-detector.spec.ts b/packages/cli/src/services/__tests__/engine-detector.spec.ts new file mode 100644 index 00000000..39f01d9b --- /dev/null +++ b/packages/cli/src/services/__tests__/engine-detector.spec.ts @@ -0,0 +1,172 @@ +import fs from 'node:fs/promises' +import os from 'node:os' +import path from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { detectEngine } from '../engine-detector.js' +import { Engine } from '../../constructs/engine.js' + +async function writeFiles (root: string, files: Record): Promise { + for (const [relPath, contents] of Object.entries(files)) { + const filePath = path.join(root, relPath) + await fs.mkdir(path.dirname(filePath), { recursive: true }) + const data = typeof contents === 'string' ? contents : JSON.stringify(contents) + await fs.writeFile(filePath, data) + } +} + +describe('detectEngine', () => { + let root: string + + beforeEach(async () => { + root = await fs.mkdtemp(path.join(os.tmpdir(), 'engine-detector-')) + }) + + afterEach(async () => { + await fs.rm(root, { recursive: true, force: true }) + }) + + it('should still detect Node from .node-version', async () => { + await writeFiles(root, { '.node-version': '24.1.0\n' }) + const result = await detectEngine(root) + expect(result?.engine).toEqual(Engine.node('24')) + expect(result?.notices).toEqual([]) + }) + + it('should skip an engines.node value that is not a semver range', async () => { + await writeFiles(root, { 'package.json': { engines: { node: 'lts', bun: 'latest' } } }) + expect(await detectEngine(root)).toBeUndefined() + }) + + it('should return undefined when no source is present', async () => { + await writeFiles(root, { 'package.json': { name: 'x' } }) + expect(await detectEngine(root)).toBeUndefined() + }) + + describe('volta.node', () => { + it('should detect Node from a volta pin', async () => { + await writeFiles(root, { 'package.json': { volta: { node: '24.17.0', pnpm: '10.30.0' } } }) + const result = await detectEngine(root) + expect(result?.engine).toEqual(Engine.node('24')) + expect(result?.notices).toEqual([]) + }) + + it('should let a version file win over the volta pin', async () => { + await writeFiles(root, { + '.nvmrc': '22\n', + 'package.json': { volta: { node: '24.17.0' } }, + }) + const result = await detectEngine(root) + expect(result?.engine).toEqual(Engine.node('22')) + }) + + it('should let the volta pin win over engines.node', async () => { + await writeFiles(root, { + 'package.json': { volta: { node: '24.17.0' }, engines: { node: '>=22' } }, + }) + const result = await detectEngine(root) + expect(result?.engine).toEqual(Engine.node('24')) + }) + + it('should accept a semver range as the pin value', async () => { + await writeFiles(root, { 'package.json': { volta: { node: '>=24' } } }) + const result = await detectEngine(root) + expect(result?.engine).toEqual(Engine.node('24')) + }) + + it('should not affect Bun detection', async () => { + await writeFiles(root, { + '.bun-version': '1.3.0\n', + 'package.json': { volta: { node: '24.17.0' } }, + }) + const result = await detectEngine(root) + expect(result?.engine).toEqual(Engine.node('24')) + }) + + it('should follow a volta.extends chain', async () => { + await writeFiles(root, { + 'project/package.json': { volta: { extends: '../shared/volta.json' } }, + 'shared/volta.json': { volta: { extends: './base.json' } }, + 'shared/base.json': { volta: { node: '24.17.0' } }, + }) + const result = await detectEngine(path.join(root, 'project')) + expect(result?.engine).toEqual(Engine.node('24')) + }) + + it('should prefer the manifest pin over the extended manifest', async () => { + await writeFiles(root, { + 'package.json': { volta: { node: '22.0.0', extends: './base.json' } }, + 'base.json': { volta: { node: '24.17.0' } }, + }) + const result = await detectEngine(root) + expect(result?.engine).toEqual(Engine.node('22')) + }) + + it('should terminate a cyclic volta.extends chain and fall through to engines', async () => { + await writeFiles(root, { + 'package.json': { volta: { extends: './package.json' }, engines: { node: '>=22' } }, + }) + const result = await detectEngine(root) + expect(result?.engine).toEqual(Engine.node('22')) + }) + + it('should skip a missing extended manifest', async () => { + await writeFiles(root, { + 'package.json': { volta: { extends: './missing.json' } }, + }) + expect(await detectEngine(root)).toBeUndefined() + }) + + it('should skip a pin that is not a semver version or range', async () => { + await writeFiles(root, { 'package.json': { volta: { node: 'lts' }, engines: { node: '>=22' } } }) + const result = await detectEngine(root) + expect(result?.engine).toEqual(Engine.node('22')) + }) + + it('should ignore non-string pin values', async () => { + await writeFiles(root, { 'package.json': { volta: { node: 24 } } }) + expect(await detectEngine(root)).toBeUndefined() + }) + + describe('workspace lookup', () => { + it('should find the workspace-root pin from a nested context path', async () => { + await writeFiles(root, { + 'package.json': { volta: { node: '24.17.0' } }, + 'apps/x/package.json': { name: 'x' }, + }) + const result = await detectEngine(root, path.join(root, 'apps', 'x')) + expect(result?.engine).toEqual(Engine.node('24')) + }) + + it('should find a pin in the nested context package', async () => { + await writeFiles(root, { + 'package.json': { name: 'root' }, + 'apps/x/package.json': { volta: { node: '24.17.0' } }, + }) + const contextPath = path.join(root, 'apps', 'x') + expect((await detectEngine(root, contextPath))?.engine).toEqual(Engine.node('24')) + expect(await detectEngine(root)).toBeUndefined() + }) + + it('should stop at the nearest manifest with a volta key even without a node pin', async () => { + await writeFiles(root, { + 'package.json': { volta: { node: '24.17.0' } }, + 'apps/x/package.json': { volta: { pnpm: '10.30.0' } }, + }) + const result = await detectEngine(root, path.join(root, 'apps', 'x')) + expect(result).toBeUndefined() + }) + + it('should not walk outside the project root', async () => { + const outside = await fs.mkdtemp(path.join(os.tmpdir(), 'engine-detector-outside-')) + try { + await writeFiles(outside, { 'package.json': { volta: { node: '22.0.0' } } }) + await writeFiles(root, { 'package.json': { volta: { node: '24.17.0' } } }) + const result = await detectEngine(root, outside) + expect(result?.engine).toEqual(Engine.node('24')) + } finally { + await fs.rm(outside, { recursive: true, force: true }) + } + }) + }) + }) +}) diff --git a/packages/cli/src/services/engine-detector.ts b/packages/cli/src/services/engine-detector.ts index 3f7202cf..dd6ee4fc 100644 --- a/packages/cli/src/services/engine-detector.ts +++ b/packages/cli/src/services/engine-detector.ts @@ -2,6 +2,8 @@ import fs from 'node:fs/promises' import path from 'node:path' import semver from 'semver' import { Engine } from '../constructs/engine.js' +import { JsonSourceFile } from './check-parser/package-files/json-source-file.js' +import { lineage } from './check-parser/package-files/walk.js' import { resolveEngineVersion } from './engine-resolver.js' export interface EngineDetectionResult { @@ -17,6 +19,13 @@ async function readFileIfExists (filePath: string): Promise } } +// Untyped read: the `volta` field is not part of the package.json schema the +// bundler uses. Missing or malformed files resolve to undefined and are skipped. +async function readJsonIfExists (filePath: string): Promise { + const file = await JsonSourceFile.loadFromFilePath(filePath) + return file?.data +} + function resolveNodeMajor (raw: string): string | undefined { const stripped = raw.trim().replace(/^v/, '') const major = stripped.split('.')[0] @@ -52,16 +61,69 @@ function parseToolVersions (content: string): { nodeVersion?: string, bunVersion return { nodeVersion, bunVersion } } +// semver.minVersion throws on values that are not a range at all (for +// example "lts"); such values are skipped so that detection falls through to +// the next source. +function minVersionOfRange (range: string): semver.SemVer | undefined { + try { + return semver.minVersion(range) ?? undefined + } catch { + return undefined + } +} + function resolveEngineFromSemverRange (range: string): string | undefined { - const min = semver.minVersion(range) + const min = minVersionOfRange(range) return min ? String(min.major) : undefined } function resolveBunFromSemverRange (range: string): string | undefined { - const min = semver.minVersion(range) + const min = minVersionOfRange(range) return min ? `${min.major}.${min.minor}` : undefined } +// Volta manifests can delegate to another JSON file through `volta.extends`, +// which is resolved relative to the manifest that declares it. The chain is +// depth-bounded, which also terminates cyclic `extends` references. +const MAX_VOLTA_EXTENDS_DEPTH = 10 + +/** + * Returns the raw `volta.node` value of an already-parsed Volta manifest, + * following its `volta.extends` chain when the manifest itself carries no + * `node` pin. Any JSON file can be extended, not only a package.json. + */ +async function resolveVoltaNode (manifest: any, manifestPath: string, depth = 0): Promise { + const volta = manifest?.volta + if (!volta || typeof volta !== 'object') return undefined + if (typeof volta.node === 'string') return volta.node + if (typeof volta.extends === 'string' && depth < MAX_VOLTA_EXTENDS_DEPTH) { + const extendedPath = path.resolve(path.dirname(manifestPath), volta.extends) + return resolveVoltaNode(await readJsonIfExists(extendedPath), extendedPath, depth + 1) + } + return undefined +} + +/** + * Mirrors how Volta itself locates a pin: walk up from the config's package + * directory and use the nearest package.json that carries a `volta` key. That + * manifest is authoritative (including its `extends` chain), so the walk stops + * there even when it yields no Node version. The walk never leaves + * `projectRoot`; a start path outside it falls back to `projectRoot` alone. + */ +async function detectVoltaNode (projectRoot: string, startPath: string): Promise { + const root = path.resolve(projectRoot) + const start = path.resolve(startPath) + const contained = start === root || start.startsWith(root + path.sep) + for (const dir of lineage(contained ? start : root, { root })) { + const manifestPath = path.join(dir, 'package.json') + const manifest = await readJsonIfExists(manifestPath) + if (manifest?.volta && typeof manifest.volta === 'object') { + return resolveVoltaNode(manifest, manifestPath) + } + } + return undefined +} + interface ResolvedVersion { version: string notices: string[] @@ -82,7 +144,20 @@ async function resolveBun (rawVersion: string): Promise { +/** + * Detects the engine from the project's version files. + * + * @param projectRoot Directory holding the version files (the workspace root + * when the project is part of one). + * @param contextPath Directory of the package that contains the Checkly + * config. Only the Volta source uses it: mirroring Volta's own lookup, the + * pin is searched from here upwards to `projectRoot`. All other sources are + * read from `projectRoot` only. + */ +export async function detectEngine ( + projectRoot: string, + contextPath: string = projectRoot, +): Promise { let nodeResult: ResolvedVersion | undefined let bunResult: ResolvedVersion | undefined @@ -117,23 +192,26 @@ export async function detectEngine (projectRoot: string): Promise