Skip to content

Commit

Permalink
Merge pull request #1109 from chromaui/cody/cap-2103-users-that-have-…
Browse files Browse the repository at this point in the history
…monorepos-such-as-nx-cannot-use-the-vtaddon

Add support for `--build-command` for arbitrary build commands
  • Loading branch information
codykaup authored Oct 25, 2024
2 parents 013a402 + a1c1456 commit 69372e2
Show file tree
Hide file tree
Showing 12 changed files with 102 additions and 13 deletions.
2 changes: 2 additions & 0 deletions action-src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ async function run() {
const autoAcceptChanges = getInput('autoAcceptChanges');
const branchName = getInput('branchName');
const buildScriptName = getInput('buildScriptName');
const buildCommand = getInput('buildCommand');
const configFile = getInput('configFile');
const cypress = getInput('cypress');
const debug = getInput('debug');
Expand Down Expand Up @@ -158,6 +159,7 @@ async function run() {
autoAcceptChanges: maybe(autoAcceptChanges),
branchName: maybe(branchName),
buildScriptName: maybe(buildScriptName),
buildCommand: maybe(buildCommand),
configFile: maybe(configFile),
cypress: maybe(cypress),
debug: maybe(debug),
Expand Down
3 changes: 3 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ inputs:
buildScriptName:
description: 'The npm script that builds your Storybook [build-storybook]'
required: false
buildCommand:
description: 'The command that builds your Storybook (when your build command does not exist in "scripts" of your package.json)'
required: false
configFile:
description: 'Path from where to load the Chromatic config JSON file.'
cypress:
Expand Down
1 change: 1 addition & 0 deletions node-src/lib/getConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const configurationSchema = z
ignoreLastBuildOnBranch: z.string(),

buildScriptName: z.string(),
buildCommand: z.string(),
playwright: z.boolean(),
cypress: z.boolean(),
outputDir: z.string(),
Expand Down
16 changes: 16 additions & 0 deletions node-src/lib/getOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ export default function getOptions(ctx: InitialContext): Options {
preserveMissingSpecs: undefined,

buildScriptName: undefined,
buildCommand: undefined,
playwright: undefined,
cypress: undefined,
outputDir: undefined,
Expand Down Expand Up @@ -158,6 +159,7 @@ export default function getOptions(ctx: InitialContext): Options {
flags.preserveMissing || typeof flags.only === 'string' ? true : undefined,

buildScriptName: flags.buildScriptName,
buildCommand: flags.buildCommand,
playwright: trueIfSet(flags.playwright),
cypress: trueIfSet(flags.cypress),
outputDir: takeLast(flags.outputDir),
Expand Down Expand Up @@ -290,6 +292,16 @@ export default function getOptions(ctx: InitialContext): Options {
throw new Error(incompatibleOptions(['--junit-report', '--exit-once-uploaded']));
}

if (potentialOptions.buildScriptName && potentialOptions.buildCommand) {
throw new Error(incompatibleOptions(['--build-script-name', '--build-command']));
}

// --build-command can put the built Storybook anywhere. Rather than reading through the value,
// we require `--output-dir` to avoid the issue.
if (potentialOptions.buildCommand && !potentialOptions.outputDir) {
throw new Error(dependentOption('--build-command', '--output-dir'));
}

if (
typeof potentialOptions.junitReport === 'string' &&
path.extname(potentialOptions.junitReport) !== '.xml'
Expand All @@ -315,6 +327,10 @@ export default function getOptions(ctx: InitialContext): Options {
return options;
}

if (potentialOptions.buildCommand) {
return options;
}

if (isE2EBuild(options)) {
return options;
}
Expand Down
2 changes: 2 additions & 0 deletions node-src/lib/parseArguments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export default function parseArguments(argv: string[]) {
Storybook options
--build-script-name, -b [name] The npm script that builds your Storybook we should take snapshots against. Use this if your Storybook build script is named differently. [build-storybook]
--build-command <command> The command that builds your Storybook we should take snapshots against. Use this if your Storybook build command does not exist in "scripts" of your package.json (like using NX). Requires --output-dir.
--output-dir, -o <dirname> Relative path to target directory for building your Storybook, in case you want to preserve it. Otherwise a temporary directory is used if possible.
--storybook-build-dir, -d <dirname> If you have already built your Storybook, provide the path to the static build directory.
Expand Down Expand Up @@ -82,6 +83,7 @@ export default function parseArguments(argv: string[]) {

// Storybook options
buildScriptName: { type: 'string', alias: 'b' },
buildCommand: { type: 'string' },
outputDir: { type: 'string', alias: 'o', isMultiple: true },
storybookBuildDir: { type: 'string', alias: 'd', isMultiple: true },

Expand Down
54 changes: 46 additions & 8 deletions node-src/tasks/build.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,31 +12,41 @@ vi.mock('@antfu/ni');
const command = vi.mocked(execaCommand);
const getCliCommand = vi.mocked(getCliCommandDefault);

const baseContext = { options: {}, flags: {} } as any;

beforeEach(() => {
command.mockClear();
});

describe('setSourceDir', () => {
it('sets a random temp directory path on the context', async () => {
const ctx = { options: {}, storybook: { version: '5.0.0' } } as any;
const ctx = { ...baseContext, storybook: { version: '5.0.0' } } as any;
await setSourceDirectory(ctx);
expect(ctx.sourceDir).toMatch(/chromatic-/);
});

it('falls back to the default output dir for older Storybooks', async () => {
const ctx = { options: {}, storybook: { version: '4.0.0' } } as any;
const ctx = { ...baseContext, storybook: { version: '4.0.0' } } as any;
await setSourceDirectory(ctx);
expect(ctx.sourceDir).toBe('storybook-static');
});

it('uses the outputDir option if provided', async () => {
const ctx = { options: { outputDir: 'storybook-out' }, storybook: { version: '5.0.0' } } as any;
const ctx = {
...baseContext,
options: { outputDir: 'storybook-out' },
storybook: { version: '5.0.0' },
} as any;
await setSourceDirectory(ctx);
expect(ctx.sourceDir).toBe('storybook-out');
});

it('uses the outputDir option if provided, even for older Storybooks', async () => {
const ctx = { options: { outputDir: 'storybook-out' }, storybook: { version: '4.0.0' } } as any;
const ctx = {
...baseContext,
options: { outputDir: 'storybook-out' },
storybook: { version: '4.0.0' },
} as any;
await setSourceDirectory(ctx);
expect(ctx.sourceDir).toBe('storybook-out');
});
Expand All @@ -47,6 +57,7 @@ describe('setBuildCommand', () => {
getCliCommand.mockReturnValue(Promise.resolve('npm run build:storybook'));

const ctx = {
...baseContext,
sourceDir: './source-dir/',
options: { buildScriptName: 'build:storybook' },
storybook: { version: '6.2.0' },
Expand All @@ -66,10 +77,11 @@ describe('setBuildCommand', () => {
getCliCommand.mockReturnValue(Promise.resolve('yarn run build:storybook'));

const ctx = {
...baseContext,
sourceDir: './source-dir/',
options: { buildScriptName: 'build:storybook' },
storybook: { version: '6.1.0' },
git: {},
storybook: { version: '6.2.0' },
git: { changedFiles: ['./index.js'] },
} as any;
await setBuildCommand(ctx);

Expand All @@ -85,10 +97,11 @@ describe('setBuildCommand', () => {
getCliCommand.mockReturnValue(Promise.resolve('pnpm run build:storybook'));

const ctx = {
...baseContext,
sourceDir: './source-dir/',
options: { buildScriptName: 'build:storybook' },
storybook: { version: '6.1.0' },
git: {},
storybook: { version: '6.2.0' },
git: { changedFiles: ['./index.js'] },
} as any;
await setBuildCommand(ctx);

Expand All @@ -100,8 +113,27 @@ describe('setBuildCommand', () => {
expect(ctx.buildCommand).toEqual('pnpm run build:storybook');
});

it('uses --build-command, if set', async () => {
getCliCommand.mockReturnValue(Promise.resolve('npm run build:storybook'));

const ctx = {
...baseContext,
sourceDir: './source-dir/',
options: { buildCommand: 'nx run my-app:build-storybook' },
storybook: { version: '6.2.0' },
git: { changedFiles: ['./index.js'] },
} as any;
await setBuildCommand(ctx);

expect(getCliCommand).not.toHaveBeenCalled();
expect(ctx.buildCommand).toEqual(
'nx run my-app:build-storybook --webpack-stats-json=./source-dir/'
);
});

it('warns if --only-changes is not supported', async () => {
const ctx = {
...baseContext,
sourceDir: './source-dir/',
options: { buildScriptName: 'build:storybook' },
storybook: { version: '6.1.0' },
Expand All @@ -118,6 +150,7 @@ describe('setBuildCommand', () => {
describe('buildStorybook', () => {
it('runs the build command', async () => {
const ctx = {
...baseContext,
buildCommand: 'npm run build:storybook --script-args',
env: { STORYBOOK_BUILD_TIMEOUT: 1000 },
log: { debug: vi.fn() },
Expand All @@ -134,6 +167,7 @@ describe('buildStorybook', () => {

it('fails when build times out', async () => {
const ctx = {
...baseContext,
buildCommand: 'npm run build:storybook --script-args',
options: { buildScriptName: '' },
env: { STORYBOOK_BUILD_TIMEOUT: 0 },
Expand All @@ -146,6 +180,7 @@ describe('buildStorybook', () => {

it('passes NODE_ENV=production', async () => {
const ctx = {
...baseContext,
buildCommand: 'npm run build:storybook --script-args',
env: { STORYBOOK_BUILD_TIMEOUT: 1000 },
log: { debug: vi.fn() },
Expand All @@ -160,6 +195,7 @@ describe('buildStorybook', () => {

it('allows overriding NODE_ENV with STORYBOOK_NODE_ENV', async () => {
const ctx = {
...baseContext,
buildCommand: 'npm run build:storybook --script-args',
env: { STORYBOOK_BUILD_TIMEOUT: 1000, STORYBOOK_NODE_ENV: 'test' },
log: { debug: vi.fn() },
Expand Down Expand Up @@ -194,6 +230,7 @@ describe('buildStorybook E2E', () => {
'fails with missing dependency error when error message is $name',
async ({ error }) => {
const ctx = {
...baseContext,
buildCommand: 'npm exec build-archive-storybook',
options: { buildScriptName: '', playwright: true },
env: { STORYBOOK_BUILD_TIMEOUT: 0 },
Expand All @@ -212,6 +249,7 @@ describe('buildStorybook E2E', () => {

it('fails with generic error message when not missing dependency error', async () => {
const ctx = {
...baseContext,
buildCommand: 'npm exec build-archive-storybook',
options: { buildScriptName: '', playwright: true },
env: { STORYBOOK_BUILD_TIMEOUT: 0 },
Expand Down
9 changes: 8 additions & 1 deletion node-src/tasks/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,18 @@ export const setBuildCommand = async (ctx: Context) => {
ctx.log.warn('Storybook version 6.2.0 or later is required to use the --only-changed flag');
}

const buildCommand = ctx.flags.buildCommand || ctx.options.buildCommand;

const buildCommandOptions = [
`--output-dir=${ctx.sourceDir}`,
!buildCommand && `--output-dir=${ctx.sourceDir}`,
ctx.git.changedFiles && webpackStatsSupported && `--webpack-stats-json=${ctx.sourceDir}`,
].filter((c): c is string => !!c);

if (buildCommand) {
ctx.buildCommand = `${buildCommand} ${buildCommandOptions.join(' ')}`;
return;
}

if (isE2EBuild(ctx.options)) {
ctx.buildCommand = await getE2EBuildCommand(
ctx,
Expand Down
2 changes: 1 addition & 1 deletion node-src/tasks/upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ function getPathsInDirectory(ctx: Context, rootDirectory: string, dirname = '.')
});
} catch (err) {
ctx.log.debug(err);
throw new Error(invalid({ sourceDir: rootDirectory } as any, err).output);
throw new Error(invalid({ ...ctx, sourceDir: rootDirectory }, err).output);
}
}

Expand Down
2 changes: 2 additions & 0 deletions node-src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export interface Flags {

// Storybook options
buildScriptName?: string;
buildCommand?: string;
outputDir?: string[];
storybookBuildDir?: string[];

Expand Down Expand Up @@ -102,6 +103,7 @@ export interface Options extends Configuration {
originalArgv: string[];

buildScriptName: Flags['buildScriptName'];
buildCommand: Flags['buildCommand'];
playwright: Flags['playwright'];
cypress: Flags['cypress'];
outputDir: string;
Expand Down
12 changes: 12 additions & 0 deletions node-src/ui/messages/errors/buildFailed.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,15 @@ export const BuildFailed = () =>
{ message: 'Command failed with exit code 1' },
buildLog
);

export const BuildFailedWithCommand = () =>
buildFailed(
{
options: { buildCommand: 'nx run my-app:build-storybook' },
buildCommand,
buildLogFile: '/path/to/project/build-storybook.log',
runtimeMetadata,
} as any,
{ message: 'Command failed with exit code 1' },
buildLog
);
11 changes: 8 additions & 3 deletions node-src/ui/messages/errors/buildFailed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,20 @@ export default (
{ message },
buildLog?: string
) => {
const { buildScriptName } = options;
const { buildScriptName, buildCommand: buildCommandOption } = options;
const lines = buildLog?.split(EOL).filter((line) => line && !line.startsWith('<s>')) || [];

const commandToBuild = buildScriptName || buildCommandOption;
const suggestedRunCommands = buildScriptName
? chalk`{bold npm run ${commandToBuild}} or {bold yarn ${commandToBuild}}`
: chalk`{bold ${commandToBuild}}`;

return [
dedent(chalk`
The CLI tried to run your {bold ${buildScriptName}} script, but the command failed. This indicates a problem with your Storybook. Here's what to do:
The CLI tried to run your {bold ${commandToBuild}} script, but the command failed. This indicates a problem with your Storybook. Here's what to do:
- Check the Storybook build log printed below.
- Run {bold npm run ${buildScriptName}} or {bold yarn ${buildScriptName}} yourself and make sure it outputs a valid Storybook by opening the generated {bold index.html} in your browser.
- Run ${suggestedRunCommands} yourself and make sure it outputs a valid Storybook by opening the generated {bold index.html} in your browser.
- Review the build-storybook CLI options at ${link(
'https://storybook.js.org/docs/configurations/cli-options/#for-build-storybook'
)}
Expand Down
1 change: 1 addition & 0 deletions vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { configDefaults, coverageConfigDefaults, defineConfig } from 'vitest/con
export default defineConfig({
test: {
exclude: [...configDefaults.exclude, '**/getParentCommits.test.ts'],
clearMocks: true, // Clear all mocks between each test
coverage: {
provider: 'v8',
exclude: [
Expand Down

0 comments on commit 69372e2

Please sign in to comment.