diff --git a/.github/workflows/tests-pr.yml b/.github/workflows/tests-pr.yml index 0a22e920304..67b82a9b2f1 100644 --- a/.github/workflows/tests-pr.yml +++ b/.github/workflows/tests-pr.yml @@ -291,3 +291,53 @@ jobs: header: Type-diff message: ${{ steps.type-diff.outputs.report }} recreate: true + + major-change-check: + if: github.event.pull_request.head.repo.full_name == github.repository + name: 'Breaking change detection' + runs-on: ubuntu-latest + timeout-minutes: 30 + outputs: + has_breaking_changes: ${{ steps.check.outputs.has_breaking_changes }} + steps: + - uses: actions/checkout@v3 + with: + repository: ${{ github.event.pull_request.head.repo.full_name || github.event.repository.full_name }} + ref: ${{ github.event.pull_request.head.ref || github.event.merge_group.head_ref }} + fetch-depth: 1 + - name: Setup deps + uses: ./.github/actions/setup-cli-deps + with: + node-version: ${{ env.DEFAULT_NODE_VERSION }} + - name: Build + run: pnpm nx run-many --all --skip-nx-cache --target=build --output-style=stream + - name: Check for breaking changes + id: check + run: node workspace/src/major-change-check.js + - uses: marocchino/sticky-pull-request-comment@fcf6fe9e4a0409cd9316a5011435be0f3327f1e1 # v2.3.1 + if: steps.check.outputs.has_breaking_changes == 'true' + with: + header: Breaking-change-detection + message: ${{ steps.check.outputs.report }} + recreate: true + - uses: marocchino/sticky-pull-request-comment@fcf6fe9e4a0409cd9316a5011435be0f3327f1e1 # v2.3.1 + if: steps.check.outputs.has_breaking_changes != 'true' + with: + header: Breaking-change-detection + delete: true + - name: Fail if breaking changes detected + if: steps.check.outputs.has_breaking_changes == 'true' + run: | + echo '::error::Breaking changes detected. A member of @shopify/dev_experience must approve the breaking-change-approval environment.' + exit 1 + + major-change-approval: + name: 'Breaking change approval' + needs: major-change-check + if: always() && needs.major-change-check.outputs.has_breaking_changes == 'true' + runs-on: ubuntu-latest + timeout-minutes: 5 + environment: breaking-change-approval + steps: + - name: Approved + run: echo '✅ Breaking changes approved by dev_experience team member' diff --git a/workspace/src/major-change-check.js b/workspace/src/major-change-check.js new file mode 100644 index 00000000000..0baf21178fa --- /dev/null +++ b/workspace/src/major-change-check.js @@ -0,0 +1,575 @@ +#!/usr/bin/env node + +/** + * Detects potential breaking changes in a PR by checking: + * + * 1. Changesets requesting a major version bump + * 2. OCLIF manifest changes: removed commands, removed flags, or removed flag env vars + * 3. Zod schema changes: removed or renamed fields in app/extension config schemas + * + * Compares the current branch against the main branch baseline. + * Outputs a GitHub Actions summary and sets a `has_breaking_changes` output. + */ + +import * as path from 'pathe' +import * as url from 'url' +import {mkdtemp, rm} from 'fs/promises' +import os from 'os' +import {promises as fs} from 'fs' +import {setOutput} from '@actions/core' +import {cloneCLIRepository} from './utils/git.js' +import {logMessage, logSection} from './utils/log.js' +import fg from 'fast-glob' + +const currentDirectory = path.join(url.fileURLToPath(new URL('.', import.meta.url)), '../..') + +// --------------------------------------------------------------------------- +// 1. Check changesets for major bumps +// --------------------------------------------------------------------------- + +async function checkChangesets() { + logSection('Checking changesets for major bumps') + + const changesetFiles = await fg('.changeset/*.md', { + cwd: currentDirectory, + absolute: true, + ignore: ['**/README.md'], + }) + + const majorChangesets = [] + + for (const file of changesetFiles) { + const content = (await fs.readFile(file, 'utf-8')).trim() + // Changeset format: YAML frontmatter between --- markers, with 'package: major' + const frontmatterMatch = content.match(/^---\n([\s\S]*?)\n---/) + if (!frontmatterMatch) continue + + const frontmatter = frontmatterMatch[1] + const lines = frontmatter.split('\n') + + for (const line of lines) { + if (line.match(/:\s*major\s*$/)) { + majorChangesets.push({ + file: path.basename(file), + line: line.trim(), + }) + } + } + } + + if (majorChangesets.length > 0) { + logMessage(`Found ${majorChangesets.length} changeset(s) with major bump`) + for (const cs of majorChangesets) { + logMessage(` ${cs.file}: ${cs.line}`) + } + } else { + logMessage('No major changesets found') + } + + return majorChangesets +} + +// --------------------------------------------------------------------------- +// 2. Check OCLIF manifest for removed commands, flags, or flag env vars +// --------------------------------------------------------------------------- + +async function parseManifest(directory) { + const manifestPath = path.join(directory, 'packages/cli/oclif.manifest.json') + try { + const content = await fs.readFile(manifestPath, 'utf-8') + return JSON.parse(content) + } catch { + return null + } +} + +function extractManifestSurface(manifest) { + const surface = {commands: {}, envVars: {}} + if (!manifest?.commands) return surface + + for (const [cmdName, cmd] of Object.entries(manifest.commands)) { + const flags = {} + if (cmd.flags) { + for (const [flagName, flag] of Object.entries(cmd.flags)) { + flags[flagName] = { + type: flag.type, + env: flag.env || null, + } + if (flag.env) { + surface.envVars[flag.env] = surface.envVars[flag.env] || [] + surface.envVars[flag.env].push({command: cmdName, flag: flagName}) + } + } + } + surface.commands[cmdName] = {flags} + } + + return surface +} + +async function checkManifest(baselineDirectory) { + logSection('Checking OCLIF manifest for removed commands/flags') + + const baselineManifest = await parseManifest(baselineDirectory) + const currentManifest = await parseManifest(currentDirectory) + + if (!baselineManifest || !currentManifest) { + logMessage('Could not read one or both manifests, skipping') + return {removedCommands: [], removedFlags: [], removedEnvVars: []} + } + + const baseline = extractManifestSurface(baselineManifest) + const current = extractManifestSurface(currentManifest) + + // Removed commands + const removedCommands = Object.keys(baseline.commands).filter( + (cmd) => !current.commands[cmd], + ) + + // Removed flags (in commands that still exist) + const removedFlags = [] + for (const [cmdName, cmd] of Object.entries(baseline.commands)) { + if (!current.commands[cmdName]) continue + for (const flagName of Object.keys(cmd.flags)) { + if (!current.commands[cmdName].flags[flagName]) { + removedFlags.push({command: cmdName, flag: flagName}) + } + } + } + + // Removed env vars + const removedEnvVars = [] + for (const [envVar, usages] of Object.entries(baseline.envVars)) { + if (!current.envVars[envVar]) { + removedEnvVars.push({envVar, usages}) + } + } + + if (removedCommands.length > 0) { + logMessage(`Removed commands: ${removedCommands.join(', ')}`) + } + if (removedFlags.length > 0) { + logMessage(`Removed flags: ${removedFlags.map((f) => `${f.command} --${f.flag}`).join(', ')}`) + } + if (removedEnvVars.length > 0) { + logMessage(`Removed env vars: ${removedEnvVars.map((e) => e.envVar).join(', ')}`) + } + if (removedCommands.length === 0 && removedFlags.length === 0 && removedEnvVars.length === 0) { + logMessage('No removed commands, flags, or env vars') + } + + return {removedCommands, removedFlags, removedEnvVars} +} + +// --------------------------------------------------------------------------- +// 3. Check Zod schemas for removed/renamed fields +// --------------------------------------------------------------------------- + +/** + * Returns a copy of `source` with strings, template literals, and comments + * replaced by spaces (newlines preserved). This lets the caller do brace / + * paren / bracket counting without false positives from `{`, `}`, `:` etc. + * inside string or comment content. Indices are preserved 1:1. + * + * Note: this is a best-effort lexer, not a full TS grammar. It does NOT + * track template-literal `${ ... }` interpolation expressions — anything + * inside a backtick-quoted template (including the `${...}` expression + * itself) is blanked out. That's safe for this use case because we only + * collect identifiers at depth==0 of `.object(...)` / `.extend(...)` + * argument blocks, and template literals are never used as keys. + */ +export function stripStringsAndComments(source) { + const out = source.split('') + let i = 0 + const len = out.length + while (i < len) { + const c = out[i] + const n = i + 1 < len ? out[i + 1] : '' + + // Line comment + if (c === '/' && n === '/') { + out[i] = ' ' + out[i + 1] = ' ' + i += 2 + while (i < len && out[i] !== '\n') { + out[i] = ' ' + i++ + } + continue + } + + // Block comment + if (c === '/' && n === '*') { + out[i] = ' ' + out[i + 1] = ' ' + i += 2 + while (i < len) { + if (out[i] === '*' && i + 1 < len && out[i + 1] === '/') { + out[i] = ' ' + out[i + 1] = ' ' + i += 2 + break + } + if (out[i] !== '\n') out[i] = ' ' + i++ + } + continue + } + + // String / template literal + if (c === '"' || c === "'" || c === '`') { + const quote = c + out[i] = ' ' + i++ + while (i < len && out[i] !== quote) { + if (out[i] === '\\' && i + 1 < len) { + out[i] = ' ' + if (out[i + 1] !== '\n') out[i + 1] = ' ' + i += 2 + continue + } + if (out[i] !== '\n') out[i] = ' ' + i++ + } + if (i < len) { + out[i] = ' ' + i++ + } + continue + } + + i++ + } + return out.join('') +} + +/** + * Given `source` and an index `openIdx` pointing at `{`, returns the index + * of the matching `}`, or -1 if unbalanced. Caller is expected to pass a + * source that has already been run through stripStringsAndComments. + */ +function findMatchingBrace(source, openIdx) { + let depth = 0 + for (let i = openIdx; i < source.length; i++) { + const c = source[i] + if (c === '{') depth++ + else if (c === '}') { + depth-- + if (depth === 0) return i + } + } + return -1 +} + +/** + * Walks `source` from `start` (inclusive) to `end` (exclusive) and adds + * every key declared at depth 0 to `out`. Recognizes: + * - bare identifiers: foo: ... + * - quoted identifiers: 'foo': ... or "foo": ... + * + * This walks the original source (not the stripped version) so quoted keys + * survive — but it tracks string and comment state inline so braces / + * parens / colons inside string bodies don't confuse the depth counter. + * Skips anything nested inside `{...}`, `[...]`, or `(...)`. + */ +function collectTopLevelKeys(source, start, end, out) { + const keyRegex = /(?:(['"])([A-Za-z_]\w*)\1|([A-Za-z_]\w*))\s*:/y + let depth = 0 + let i = start + while (i < end) { + const c = source[i] + const n = i + 1 < end ? source[i + 1] : '' + + // Comments + if (c === '/' && n === '/') { + i += 2 + while (i < end && source[i] !== '\n') i++ + continue + } + if (c === '/' && n === '*') { + i += 2 + while (i < end && !(source[i] === '*' && i + 1 < end && source[i + 1] === '/')) i++ + i += 2 + continue + } + + // Template literals — never used as keys, just skip the whole thing. + if (c === '`') { + i++ + while (i < end && source[i] !== '`') { + if (source[i] === '\\' && i + 1 < end) i += 2 + else i++ + } + i++ + continue + } + + // Quoted strings: at depth 0, check whether this is a quoted KEY (i.e. + // followed by `:` after the closing quote). If so, the keyRegex below + // will match it cleanly. Otherwise, skip the string body so its + // contents don't perturb the depth counter. + if (c === '"' || c === "'") { + if (depth === 0) { + keyRegex.lastIndex = i + const km = keyRegex.exec(source) + if (km && km.index === i) { + out.add(km[2] || km[3]) + i += km[0].length + continue + } + } + const quote = c + i++ + while (i < end && source[i] !== quote) { + if (source[i] === '\\' && i + 1 < end) i += 2 + else i++ + } + i++ + continue + } + + if (c === '{' || c === '(' || c === '[') { + depth++ + i++ + continue + } + if (c === '}' || c === ')' || c === ']') { + depth-- + i++ + continue + } + + // Bare-identifier key at depth 0 + if (depth === 0 && /[A-Za-z_]/.test(c)) { + keyRegex.lastIndex = i + const km = keyRegex.exec(source) + if (km && km.index === i) { + out.add(km[2] || km[3]) + i += km[0].length + continue + } + } + i++ + } +} + +/** + * Extracts top-level field names from Zod schema source files. + * + * Walks every `.object({...})` and `.extend({...})` block with a + * brace-counting scan (so nested object literals don't truncate the parent + * block) and collects the identifiers declared at depth 0 of each block. + * + * Both call sites matter: + * - `.object({...})` is the canonical Zod object literal. + * - `.extend({...})` is how spec files in + * `packages/app/src/cli/models/extensions/specifications/**` add their + * top-level fields onto a base schema (e.g. `BaseSchemaWithoutHandle.extend({...})`). + * Without this, removing a field added via `.extend` is undetectable. + * + * This is still a heuristic — it doesn't understand TypeScript types, only + * the surface of the source. For full correctness, swap it out for an + * AST-based extraction (`@typescript-eslint/parser` or the TS compiler API). + * The brace-counted version is a strict superset of the previous regex; + * any field the old extractor saw, this one sees too. + */ +export function extractSchemaFields(content) { + const fields = new Set() + const stripped = stripStringsAndComments(content) + const callerRegex = /\.(?:object|extend)\s*\(\s*\{/g + let m + while ((m = callerRegex.exec(stripped)) !== null) { + const openBraceIdx = m.index + m[0].length - 1 + const closeIdx = findMatchingBrace(stripped, openBraceIdx) + if (closeIdx === -1) continue + // Walk the ORIGINAL source within the brace-balanced range so quoted + // keys survive — `stripped` was used only for safe brace counting. + collectTopLevelKeys(content, openBraceIdx + 1, closeIdx, fields) + callerRegex.lastIndex = closeIdx + 1 + } + return fields +} + +async function checkSchemas(baselineDirectory) { + logSection('Checking Zod schemas for removed fields') + + const schemaGlob = 'packages/app/src/cli/models/**/specifications/**/*.ts' + const appModelGlob = 'packages/app/src/cli/models/app/app.ts' + const ignorePatterns = ['**/*.test.ts', '**/*.test-data.ts'] + + const baselineSchemaFiles = await fg([schemaGlob, appModelGlob], { + cwd: baselineDirectory, + ignore: ignorePatterns, + }) + + const removedFields = [] + + for (const file of baselineSchemaFiles) { + const baselinePath = path.join(baselineDirectory, file) + const currentPath = path.join(currentDirectory, file) + + let baselineContent + let currentContent + try { + baselineContent = await fs.readFile(baselinePath, 'utf-8') + } catch { + continue + } + try { + currentContent = await fs.readFile(currentPath, 'utf-8') + } catch { + // File was deleted — all fields are removed + const fields = extractSchemaFields(baselineContent) + if (fields.size > 0) { + removedFields.push({file, fields: [...fields], type: 'file_deleted'}) + } + continue + } + + // Only check files that actually contain a Zod object/extend block. + // (`.object(` already subsumes `zod.object(`.) + if (!baselineContent.includes('.object(') && !baselineContent.includes('.extend(')) continue + + const baselineFields = extractSchemaFields(baselineContent) + const currentFields = extractSchemaFields(currentContent) + + const removed = [...baselineFields].filter((f) => !currentFields.has(f)) + if (removed.length > 0) { + removedFields.push({file, fields: removed, type: 'fields_removed'}) + } + } + + if (removedFields.length > 0) { + for (const entry of removedFields) { + logMessage(`${entry.file}: removed fields [${entry.fields.join(', ')}] (${entry.type})`) + } + } else { + logMessage('No removed schema fields detected') + } + + return removedFields +} + +// --------------------------------------------------------------------------- +// Report +// --------------------------------------------------------------------------- + +function buildReport({majorChangesets, manifestChanges, schemaChanges}) { + const hasBreaking = + majorChangesets.length > 0 || + manifestChanges.removedCommands.length > 0 || + manifestChanges.removedFlags.length > 0 || + manifestChanges.removedEnvVars.length > 0 || + schemaChanges.length > 0 + + if (!hasBreaking) { + return null + } + + let report = `## ⚠️ Potential Breaking Changes Detected + +This PR contains changes that may break the existing contract. + +**@shopify/dev_experience** — this PR contains breaking changes that require coordination for the next major release. This check will remain failed until a member of the team approves the workflow run. + +> 💬 Head to [#help-dev-platform](https://shopify.enterprise.slack.com/archives/C07UJ7UNMTK) to discuss timing and plan the release. + +` + + if (majorChangesets.length > 0) { + report += `### 📦 Major Version Changesets +The following changesets request a **major** version bump: + +| Changeset | Package | +|-----------|---------| +` + for (const cs of majorChangesets) { + report += `| \`${cs.file}\` | ${cs.line} |\n` + } + report += '\n' + } + + if (manifestChanges.removedCommands.length > 0) { + report += `### 🗑️ Removed Commands +The following commands were removed from the OCLIF manifest: + +${manifestChanges.removedCommands.map((cmd) => `- \`${cmd}\``).join('\n')} + +` + } + + if (manifestChanges.removedFlags.length > 0) { + report += `### 🏳️ Removed Flags +The following flags were removed from existing commands: + +| Command | Flag | +|---------|------| +` + for (const f of manifestChanges.removedFlags) { + report += `| \`${f.command}\` | \`--${f.flag}\` |\n` + } + report += '\n' + } + + if (manifestChanges.removedEnvVars.length > 0) { + report += `### 🔧 Removed Environment Variables +The following env vars are no longer referenced in command flags: + +| Env Var | Previously Used By | +|---------|-------------------| +` + for (const e of manifestChanges.removedEnvVars) { + const usages = e.usages.map((u) => `\`${u.command} --${u.flag}\``).join(', ') + report += `| \`${e.envVar}\` | ${usages} |\n` + } + report += '\n' + } + + if (schemaChanges.length > 0) { + report += `### 📝 Removed Schema Fields +The following Zod schema fields were removed or their files deleted: + +| File | Removed Fields | Reason | +|------|---------------|--------| +` + for (const entry of schemaChanges) { + const fields = entry.fields.map((f) => `\`${f}\``).join(', ') + const reason = entry.type === 'file_deleted' ? 'File deleted' : 'Fields removed' + report += `| \`${entry.file}\` | ${fields} | ${reason} |\n` + } + report += '\n' + } + + return report +} + +// --------------------------------------------------------------------------- +// Main +// --------------------------------------------------------------------------- + +const tmpDir = await mkdtemp(path.join(os.tmpdir(), 'major-change-check-')) + +try { + // This script consumes only git-tracked files (oclif.manifest.json + .ts + // sources). It does not need the baseline's node_modules or dist output, + // so we skip pnpm install and pnpm build to save ~5–10 minutes of CI + // per PR. type-diff.js (which diffs `dist/**/*.d.ts`) keeps the default. + const baselineDirectory = await cloneCLIRepository(tmpDir, {install: false, build: false}) + + const majorChangesets = await checkChangesets() + const manifestChanges = await checkManifest(baselineDirectory) + const schemaChanges = await checkSchemas(baselineDirectory) + + const report = buildReport({majorChangesets, manifestChanges, schemaChanges}) + + if (report) { + logSection('\n⚠️ Breaking changes detected!') + setOutput('has_breaking_changes', 'true') + setOutput('report', report) + } else { + logSection('\n✅ No breaking changes detected') + setOutput('has_breaking_changes', 'false') + } +} finally { + await rm(tmpDir, {recursive: true, force: true, maxRetries: 2}) +} diff --git a/workspace/src/major-change-check.test.js b/workspace/src/major-change-check.test.js new file mode 100644 index 00000000000..1f20037e789 --- /dev/null +++ b/workspace/src/major-change-check.test.js @@ -0,0 +1,137 @@ +/** + * Regression tests for the schema field extractor in major-change-check.js. + * + * These cover the three failure modes Isaac flagged in the review of #7351: + * + * 1. Nested `.object(...)` siblings dropped by the non-greedy regex. + * 2. `.extend({...})` blocks ignored entirely by the substring guard. + * 3. Quoted keys (`'foo': ...`, `"foo": ...`) ignored. + * + * Run with `node --test workspace/src/major-change-check.test.js`. + */ + +import {test} from 'node:test' +import assert from 'node:assert/strict' + +import {extractSchemaFields, stripStringsAndComments} from './major-change-check.js' + +test('extracts top-level keys from a flat .object({...})', () => { + const src = ` + const Schema = zod.object({ + name: zod.string(), + version: zod.string().optional(), + }) + ` + assert.deepEqual([...extractSchemaFields(src)].sort(), ['name', 'version']) +}) + +test('captures sibling fields after a nested .object({...})', () => { + // This was the primary regression: the old regex was non-greedy and + // closed the outer object at the FIRST `}\s*)`, so `qux` was silently + // dropped and removing it would not be detected. + const src = ` + const Schema = zod.object({ + foo: zod.string(), + bar: zod.object({ + baz: zod.number(), + }), + qux: zod.boolean(), + }) + ` + const fields = extractSchemaFields(src) + assert.ok(fields.has('foo'), 'foo') + assert.ok(fields.has('bar'), 'bar') + assert.ok(fields.has('qux'), 'qux (post-nested sibling)') + // We deliberately do NOT include `baz` — it's nested, not top-level. + assert.equal(fields.has('baz'), false, 'baz is nested, must not appear') +}) + +test('captures fields added via BaseSchema.extend({...})', () => { + // Mirrors `app_config_app_home.ts`: top-level fields are added through + // `.extend(...)`, never `.object(...)`. The previous heuristic missed + // these entirely because `.extend(` did not match its substring guard. + const src = ` + import {BaseSchemaWithoutHandle} from '../base' + export const AppHomeSpec = BaseSchemaWithoutHandle.extend({ + application_url: zod.string().url(), + embedded: zod.boolean().default(true), + }) + ` + assert.deepEqual([...extractSchemaFields(src)].sort(), ['application_url', 'embedded']) +}) + +test('handles chained .extend({...}).extend({...})', () => { + const src = ` + const S = Base.extend({ + first: zod.string(), + }).extend({ + second: zod.string(), + }) + ` + assert.deepEqual([...extractSchemaFields(src)].sort(), ['first', 'second']) +}) + +test('handles quoted keys', () => { + const src = ` + const S = zod.object({ + 'kebab-case-ish': zod.string(), + "double_quoted": zod.string(), + bare: zod.string(), + }) + ` + // Note: keys with hyphens are not valid bare identifiers, so kebab-case-ish + // would only match if quoted. Our regex requires \w-only identifiers, so + // this serves as a known-limitation test. + const fields = extractSchemaFields(src) + assert.ok(fields.has('double_quoted'), 'double_quoted') + assert.ok(fields.has('bare'), 'bare') +}) + +test('ignores keys inside nested arrays and function calls', () => { + const src = ` + const S = zod.object({ + tags: zod.array(zod.object({ value: zod.string() })), + url: zod.string().refine((v) => v.startsWith('https://'), { message: 'bad' }), + }) + ` + const fields = extractSchemaFields(src) + assert.deepEqual([...fields].sort(), ['tags', 'url']) + assert.equal(fields.has('value'), false, 'nested object inside array') + assert.equal(fields.has('message'), false, 'object inside refine() call') +}) + +test('ignores keys inside string literals and comments', () => { + const src = ` + // foo: zod.string(), + /* bar: zod.string(), */ + const S = zod.object({ + real: zod.string(), + example: zod.literal('fake: zod.string()'), + }) + ` + const fields = extractSchemaFields(src) + assert.ok(fields.has('real')) + assert.ok(fields.has('example')) + assert.equal(fields.has('foo'), false, 'line comment') + assert.equal(fields.has('bar'), false, 'block comment') + assert.equal(fields.has('fake'), false, 'inside string literal') +}) + +test('returns empty set on an unbalanced/incomplete file', () => { + // Don't crash if the file is mid-edit / truncated. + const src = `const S = zod.object({ foo: zod.string(),` + const fields = extractSchemaFields(src) + assert.equal(fields.size, 0) +}) + +test('stripStringsAndComments preserves length and newlines', () => { + const src = `const a = "hello\\nworld" // trailing\nconst b = /* x */ 1\n` + const stripped = stripStringsAndComments(src) + assert.equal(stripped.length, src.length, 'length preserved') + // Newlines must survive so line numbers in error messages still align. + const newlinesIn = (s) => (s.match(/\n/g) || []).length + assert.equal(newlinesIn(stripped), newlinesIn(src), 'newlines preserved') + // The contents of strings and comments must be gone. + assert.equal(stripped.includes('hello'), false) + assert.equal(stripped.includes('trailing'), false) +}) diff --git a/workspace/src/utils/git.js b/workspace/src/utils/git.js index 02a2bf7c40b..6d08d7dca2f 100644 --- a/workspace/src/utils/git.js +++ b/workspace/src/utils/git.js @@ -3,14 +3,33 @@ import * as path from 'pathe' import git from 'simple-git' import {logMessage, logSection} from './log.js' -export async function cloneCLIRepository(tmpDir) { +/** + * Clone the Shopify/cli `main` branch into `tmpDir/cli`. + * + * `install` and `build` default to `true` to preserve behavior for + * `type-diff.js`, which diffs `dist/**\/*.d.ts` and therefore needs the + * baseline built. Pass `{install: false, build: false}` from callers that + * only consume git-tracked sources or committed artifacts (e.g. + * `major-change-check.js`, which reads `oclif.manifest.json` and `.ts` + * source files only). Skipping install+build saves ~5–10 minutes of CI per + * PR for those callers. + * + * @param {string} tmpDir + * @param {{install?: boolean, build?: boolean}} [options] + * @returns {Promise} path to the cloned repository + */ +export async function cloneCLIRepository(tmpDir, {install = true, build = true} = {}) { logSection('Setting up baseline: main branch') const directory = path.join(tmpDir, 'cli') logMessage('Cloning repository') await git().clone('https://github.com/Shopify/cli.git', directory) - logMessage('Installing dependencies') - await execa('pnpm', ['install'], {cwd: directory}) - logMessage('Building the project') - await execa('pnpm', ['build'], {cwd: directory}) + if (install) { + logMessage('Installing dependencies') + await execa('pnpm', ['install'], {cwd: directory}) + } + if (build) { + logMessage('Building the project') + await execa('pnpm', ['build'], {cwd: directory}) + } return directory }