From f2b3a8aab3d13066329de3205f319c990af7d1b0 Mon Sep 17 00:00:00 2001 From: Josema Sar Date: Sat, 12 Feb 2022 23:55:11 +0000 Subject: [PATCH 1/7] open arg default set to false --- packages/cli/src/commands/storybook.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/commands/storybook.js b/packages/cli/src/commands/storybook.js index f58369c83a33..73d3d2ae2bee 100644 --- a/packages/cli/src/commands/storybook.js +++ b/packages/cli/src/commands/storybook.js @@ -14,7 +14,7 @@ export const builder = (yargs) => { .option('open', { describe: 'Open storybooks in your browser on start', type: 'boolean', - default: true, + default: false, }) .option('build', { describe: 'Build Storybook', @@ -33,7 +33,7 @@ export const builder = (yargs) => { }) .option('manager-cache', { describe: - "Cache the manager UI. Disable this when you're making changes to `storybook.manager.js`.", + "Cache the manager UI. Disable this when you're making changes to `storybook.manager.js`.", type: 'boolean', default: true, }) @@ -85,7 +85,6 @@ export const handler = ({ !build && '--no-version-updates', !managerCache && '--no-manager-cache', build && `--output-dir "${buildDirectory}"`, - !open && !smokeTest && `--ci`, smokeTest && `--ci --smoke-test`, ].filter(Boolean), { From 5fea0c558af0fcd2349900b93fe679ccece60548 Mon Sep 17 00:00:00 2001 From: Josema Sar Date: Sun, 13 Feb 2022 00:03:54 +0000 Subject: [PATCH 2/7] small indentation correction --- packages/cli/src/commands/storybook.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/commands/storybook.js b/packages/cli/src/commands/storybook.js index 73d3d2ae2bee..e29dd3f28243 100644 --- a/packages/cli/src/commands/storybook.js +++ b/packages/cli/src/commands/storybook.js @@ -33,7 +33,7 @@ export const builder = (yargs) => { }) .option('manager-cache', { describe: - "Cache the manager UI. Disable this when you're making changes to `storybook.manager.js`.", + "Cache the manager UI. Disable this when you're making changes to `storybook.manager.js`.", type: 'boolean', default: true, }) From 1b6e09f9b4f088f8eca6f5c68393927ad5ad4702 Mon Sep 17 00:00:00 2001 From: Josema Sar Date: Tue, 15 Feb 2022 18:41:13 +0100 Subject: [PATCH 3/7] separate logic for 3 cases --- packages/cli/src/commands/storybook.js | 129 +++++++++++++++++++------ 1 file changed, 97 insertions(+), 32 deletions(-) diff --git a/packages/cli/src/commands/storybook.js b/packages/cli/src/commands/storybook.js index e29dd3f28243..80804fb00327 100644 --- a/packages/cli/src/commands/storybook.js +++ b/packages/cli/src/commands/storybook.js @@ -1,20 +1,24 @@ import path from 'path' import execa from 'execa' +import terminalLink from 'terminal-link' import { getPaths } from '@redwoodjs/internal' +import { errorTelemetry } from '@redwoodjs/telemetry' + +import c from '../lib/colors' export const command = 'storybook' export const aliases = ['sb'] export const description = - 'Launch Storybook: An isolated component development environment' + 'Launch Storybook: a tool for building UI components and pages in isolation' export const builder = (yargs) => { yargs .option('open', { describe: 'Open storybooks in your browser on start', type: 'boolean', - default: false, + default: true, }) .option('build', { describe: 'Build Storybook', @@ -43,24 +47,30 @@ export const builder = (yargs) => { type: 'boolean', default: false, }) - .check((argv) => { - if (argv.build && argv.smokeTest) { - throw new Error('Can not provide both "--build" and "--smoke-test"') - } - if (argv.build && argv.open) { - throw new Error('Can not provide both "--build" or "--open"') - } - return true - }) + // .check((argv) => { + // if (argv.build && argv.smokeTest) { + // throw new Error('Can not provide both "--build" and "--smoke-test"') + // } + // if (argv.build && argv.open) { + // throw new Error('Can not provide both "--build" or "--open"') + // } + // return true + // }) + .epilogue( + `Also see the ${terminalLink( + 'Redwood CLI Reference', + 'https://redwoodjs.com/reference/command-line-interface#storybook' + )}` + ) } export const handler = ({ - open, - port, - build, - buildDirectory, - managerCache, - smokeTest, + open = true, + port = 7910, + build = false, + buildDirectory = 'public/storybook', + managerCache = true, + smokeTest = false, }) => { const cwd = getPaths().web.base @@ -77,20 +87,75 @@ export const handler = ({ require.resolve('@redwoodjs/testing/config/storybook/main.js') ) - execa( - `yarn ${build ? 'build' : 'start'}-storybook`, - [ - `--config-dir "${storybookConfig}"`, - !build && `--port ${port}`, - !build && '--no-version-updates', - !managerCache && '--no-manager-cache', - build && `--output-dir "${buildDirectory}"`, - smokeTest && `--ci --smoke-test`, - ].filter(Boolean), - { - stdio: 'inherit', - shell: true, - cwd, + // Case 1: yarn rw sb + // run start-storybook + // open in browser (default true) + // set port (default 7910) + // directory options are hard-coded + //NOTE: `--no-manager-cache` option is only available for `start-storybook` + + if (open) { + execa( + `yarn start-storybook`, + [ + `--config-dir "${storybookConfig}"`, + `--port ${port}`, + !managerCache && '--no-manager-cache', + ].filter(Boolean), + { + stdio: 'inherit', + shell: true, + cwd, + } + ) + } + + // Case 2: yarn rw sb --build + // build-directory option (default web/public/storybook) + // directory for config is hard-coded + // manager-cache option (default true) + //NOTE: `--no-manager-cache` option is only available for `start-storybook` + + if (build) { + execa( + `yarn build-storybook`, + [ + `--config-dir "${storybookConfig}"`, + `--output-dir "${buildDirectory}"`, + `--open=false`, + ].filter(Boolean), + { + stdio: 'inherit', + shell: true, + cwd, + } + ) + } + + // Case 3: yarn rw sb --smoke-test + // runs the start-storybook command; exits gracefully if started successful, throws otherwise + // passes --smoke-test and --ci options + try { + if (smokeTest) { + execa( + `yarn start-storybook`, + [ + `--config-dir "${storybookConfig}"`, + `--port ${port}`, + // !managerCache && '--no-manager-cache', + `--smoke-test`, + `--ci`, + ].filter(Boolean), + { + stdio: 'inherit', + shell: true, + cwd, + } + ) } - ) + } catch (e) { + console.log(c.error(e.message)) + errorTelemetry(process.argv, e.message) + process.exit(1) + } } From 6743acfab5b914c02ca9a6bf05687a3956403bf3 Mon Sep 17 00:00:00 2001 From: Josema Sar Date: Tue, 15 Feb 2022 18:55:33 +0100 Subject: [PATCH 4/7] small fix: param deleted in build --- packages/cli/src/commands/storybook.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cli/src/commands/storybook.js b/packages/cli/src/commands/storybook.js index 80804fb00327..d8e59a884866 100644 --- a/packages/cli/src/commands/storybook.js +++ b/packages/cli/src/commands/storybook.js @@ -122,7 +122,6 @@ export const handler = ({ [ `--config-dir "${storybookConfig}"`, `--output-dir "${buildDirectory}"`, - `--open=false`, ].filter(Boolean), { stdio: 'inherit', From d609a755e50349c9f0415a5d2bf55c21c22eadf7 Mon Sep 17 00:00:00 2001 From: Josema Sar Date: Wed, 16 Feb 2022 22:37:25 +0100 Subject: [PATCH 5/7] PR comments resolved --- packages/cli/src/commands/storybook.js | 115 +++++++++++-------------- 1 file changed, 51 insertions(+), 64 deletions(-) diff --git a/packages/cli/src/commands/storybook.js b/packages/cli/src/commands/storybook.js index d8e59a884866..32f4e97591d8 100644 --- a/packages/cli/src/commands/storybook.js +++ b/packages/cli/src/commands/storybook.js @@ -47,15 +47,19 @@ export const builder = (yargs) => { type: 'boolean', default: false, }) - // .check((argv) => { - // if (argv.build && argv.smokeTest) { - // throw new Error('Can not provide both "--build" and "--smoke-test"') - // } - // if (argv.build && argv.open) { - // throw new Error('Can not provide both "--build" or "--open"') - // } - // return true - // }) + .check((argv) => { + if (argv.build && argv.smokeTest) { + throw new Error('Can not provide both "--build" and "--smoke-test"') + } + if (argv.build && argv.open) { + console.warn( + c.warning( + 'Warning: --open option has no effect when running Storybook build' + ) + ) + } + return true + }) .epilogue( `Also see the ${terminalLink( 'Redwood CLI Reference', @@ -65,12 +69,13 @@ export const builder = (yargs) => { } export const handler = ({ - open = true, - port = 7910, - build = false, - buildDirectory = 'public/storybook', - managerCache = true, - smokeTest = false, + // eslint-disable-next-line no-unused-vars + open, + port, + build, + buildDirectory, + managerCache, + smokeTest, }) => { const cwd = getPaths().web.base @@ -87,63 +92,45 @@ export const handler = ({ require.resolve('@redwoodjs/testing/config/storybook/main.js') ) - // Case 1: yarn rw sb - // run start-storybook - // open in browser (default true) - // set port (default 7910) - // directory options are hard-coded - //NOTE: `--no-manager-cache` option is only available for `start-storybook` - - if (open) { - execa( - `yarn start-storybook`, - [ - `--config-dir "${storybookConfig}"`, - `--port ${port}`, - !managerCache && '--no-manager-cache', - ].filter(Boolean), - { - stdio: 'inherit', - shell: true, - cwd, - } - ) - } - - // Case 2: yarn rw sb --build - // build-directory option (default web/public/storybook) - // directory for config is hard-coded - // manager-cache option (default true) - //NOTE: `--no-manager-cache` option is only available for `start-storybook` - - if (build) { - execa( - `yarn build-storybook`, - [ - `--config-dir "${storybookConfig}"`, - `--output-dir "${buildDirectory}"`, - ].filter(Boolean), - { - stdio: 'inherit', - shell: true, - cwd, - } - ) - } - - // Case 3: yarn rw sb --smoke-test - // runs the start-storybook command; exits gracefully if started successful, throws otherwise - // passes --smoke-test and --ci options try { - if (smokeTest) { + if (build) { + execa( + `yarn build-storybook`, + [ + `--config-dir "${storybookConfig}"`, + `--output-dir "${buildDirectory}"`, + !managerCache && '--no-manager-cache', + ].filter(Boolean), + { + stdio: 'inherit', + shell: true, + cwd, + } + ) + } else if (smokeTest) { execa( `yarn start-storybook`, [ `--config-dir "${storybookConfig}"`, `--port ${port}`, - // !managerCache && '--no-manager-cache', `--smoke-test`, `--ci`, + `--no-version-updates`, + ].filter(Boolean), + { + stdio: 'inherit', + shell: true, + cwd, + } + ) + } else { + execa( + `yarn start-storybook`, + [ + `--config-dir "${storybookConfig}"`, + `--port ${port}`, + !managerCache && '--no-manager-cache', + '--no-version-updates', ].filter(Boolean), { stdio: 'inherit', From 0a6dc3cf87f2e5447da68618ba61c5a10b8130d4 Mon Sep 17 00:00:00 2001 From: Josema Sar Date: Wed, 16 Feb 2022 23:10:46 +0100 Subject: [PATCH 6/7] fixing quote marks --- packages/cli/src/commands/storybook.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/commands/storybook.js b/packages/cli/src/commands/storybook.js index 32f4e97591d8..1ca6c362c599 100644 --- a/packages/cli/src/commands/storybook.js +++ b/packages/cli/src/commands/storybook.js @@ -99,7 +99,7 @@ export const handler = ({ [ `--config-dir "${storybookConfig}"`, `--output-dir "${buildDirectory}"`, - !managerCache && '--no-manager-cache', + !managerCache && `--no-manager-cache`, ].filter(Boolean), { stdio: 'inherit', @@ -129,8 +129,8 @@ export const handler = ({ [ `--config-dir "${storybookConfig}"`, `--port ${port}`, - !managerCache && '--no-manager-cache', - '--no-version-updates', + !managerCache && `--no-manager-cache`, + `--no-version-updates`, ].filter(Boolean), { stdio: 'inherit', From 6f534c2c52c6409a58a694d986f3e30aaedabf98 Mon Sep 17 00:00:00 2001 From: Josema Sar Date: Thu, 17 Feb 2022 16:58:29 +0100 Subject: [PATCH 7/7] no open argument included --- packages/cli/src/commands/storybook.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/commands/storybook.js b/packages/cli/src/commands/storybook.js index 1ca6c362c599..040ba4afb823 100644 --- a/packages/cli/src/commands/storybook.js +++ b/packages/cli/src/commands/storybook.js @@ -69,7 +69,6 @@ export const builder = (yargs) => { } export const handler = ({ - // eslint-disable-next-line no-unused-vars open, port, build, @@ -131,6 +130,7 @@ export const handler = ({ `--port ${port}`, !managerCache && `--no-manager-cache`, `--no-version-updates`, + !open && `--no-open`, ].filter(Boolean), { stdio: 'inherit',