Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for --build-command for arbitrary build commands #1109

Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
10 changes: 10 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,10 @@ 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']));
}

if (
typeof potentialOptions.junitReport === 'string' &&
path.extname(potentialOptions.junitReport) !== '.xml'
Expand All @@ -315,6 +321,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).
--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
12 changes: 11 additions & 1 deletion node-src/tasks/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,21 @@ 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}`,
// NOTE: There is a bug in NX that outputs an invalid Storybook if the `--output-dir` flag is
// passed. Therefore, we need to skip that until it's fixed: https://github.com/nrwl/nx/issues/28594
// When that's fixed, we can remove the `!buildCommand &&` below.
!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
Loading