diff --git a/workspace/src/major-change-check.js b/workspace/src/major-change-check.js index 9691a8dd34c..3b411aad54d 100644 --- a/workspace/src/major-change-check.js +++ b/workspace/src/major-change-check.js @@ -163,11 +163,24 @@ async function parseManifest(directory) { } } -function extractManifestSurface(manifest) { +export function extractManifestSurface(manifest) { const surface = {commands: {}, envVars: {}} if (!manifest?.commands) return surface for (const [cmdName, cmd] of Object.entries(manifest.commands)) { + // Hidden commands are not part of the stable command surface that + // CONTRIBUTING.md defines, so renaming or removing their flags is not a + // breaking change. Dropping them here excludes them from the removed + // command, flag and env var scans alike, since all three derive from this + // surface. An env var shared with a visible command stays tracked through + // that command. + // + // Hidden-ness is read from each manifest independently, on purpose: a + // command that was visible in the baseline and is hidden now still reports + // as a removed command, so a PR cannot hide a command and strip its flags + // in one move without being flagged. + if (cmd.hidden) continue + const flags = {} if (cmd.flags) { for (const [flagName, flag] of Object.entries(cmd.flags)) { diff --git a/workspace/src/major-change-check.test.js b/workspace/src/major-change-check.test.js index 0af5762bd17..e5ac46440a6 100644 --- a/workspace/src/major-change-check.test.js +++ b/workspace/src/major-change-check.test.js @@ -17,7 +17,13 @@ import {mkdtemp, rm, writeFile, mkdir} from 'node:fs/promises' import os from 'node:os' import * as path from 'pathe' -import {checkChangesets, extractSchemaFields, resolveContext, stripStringsAndComments} from './major-change-check.js' +import { + checkChangesets, + extractManifestSurface, + extractSchemaFields, + resolveContext, + stripStringsAndComments, +} from './major-change-check.js' test('extracts top-level keys from a flat .object({...})', () => { const src = ` @@ -270,3 +276,56 @@ test('resolveContext: no GITHUB_BASE_REF falls back to scanning main (local invo assert.equal(ctx.changedFiles, null) assert.equal(called, false, 'must not shell out to git when no base ref is known') }) + +test('manifest surface omits hidden commands and their flags', () => { + const surface = extractManifestSurface({ + commands: { + 'app:dev': {flags: {store: {type: 'option'}}}, + 'store:create:dev': {hidden: true, flags: {'with-demo-data': {type: 'boolean'}}}, + }, + }) + + assert.deepEqual(Object.keys(surface.commands), ['app:dev']) + assert.equal(surface.commands['store:create:dev'], undefined) +}) + +test('manifest surface omits env vars used only by hidden commands', () => { + const surface = extractManifestSurface({ + commands: { + 'store:create:dev': { + hidden: true, + flags: {'with-demo-data': {type: 'boolean', env: 'SHOPIFY_FLAG_STORE_WITH_DEMO_DATA'}}, + }, + }, + }) + + assert.deepEqual(surface.envVars, {}) +}) + +test('manifest surface keeps an env var shared with a visible command', () => { + // Only the hidden command drops out; the visible command keeps the env var + // under watch, so removing it there is still reported. + const surface = extractManifestSurface({ + commands: { + 'app:dev': {flags: {store: {type: 'option', env: 'SHOPIFY_FLAG_STORE'}}}, + 'store:create:dev': {hidden: true, flags: {store: {type: 'option', env: 'SHOPIFY_FLAG_STORE'}}}, + }, + }) + + assert.deepEqual(surface.envVars.SHOPIFY_FLAG_STORE, [{command: 'app:dev', flag: 'store'}]) +}) + +test('manifest surface still tracks a command that is visible in one manifest and hidden in another', () => { + // Reading `hidden` per manifest is what stops a PR from hiding a command and + // stripping its flags in one move: the baseline still lists it, so the + // removal surfaces. + const baseline = extractManifestSurface({ + commands: {'app:dev': {flags: {store: {type: 'option'}}}}, + }) + const current = extractManifestSurface({ + commands: {'app:dev': {hidden: true, flags: {}}}, + }) + + assert.deepEqual(Object.keys(baseline.commands), ['app:dev']) + assert.deepEqual(Object.keys(current.commands), []) +})