From 217fa08cedb4511967093481c841938ac055dc68 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 25 Jul 2022 11:19:26 +1000 Subject: [PATCH 01/28] Add options library, and use it to drive a new example command --- code/jest.config.js | 1 + code/package.json | 1 + scripts/example.ts | 43 ++++++++++++ scripts/ts-run.js | 5 ++ scripts/utils/options.test.ts | 104 ++++++++++++++++++++++++++++ scripts/utils/options.ts | 124 ++++++++++++++++++++++++++++++++++ scripts/utils/register.js | 13 ++++ 7 files changed, 291 insertions(+) create mode 100644 scripts/example.ts create mode 100644 scripts/ts-run.js create mode 100644 scripts/utils/options.test.ts create mode 100644 scripts/utils/options.ts create mode 100644 scripts/utils/register.js diff --git a/code/jest.config.js b/code/jest.config.js index a2ffa022105e..8bb68a3d143a 100644 --- a/code/jest.config.js +++ b/code/jest.config.js @@ -42,6 +42,7 @@ module.exports = { '/addons', '/frameworks', '/lib', + '/scripts', '/examples/official-storybook', '/examples/react-ts', ], diff --git a/code/package.json b/code/package.json index 54ea9ceed446..98c6cecdcaec 100644 --- a/code/package.json +++ b/code/package.json @@ -62,6 +62,7 @@ "clean:dist": "del **/dist", "coverage": "codecov", "danger": "danger", + "example": "node ../scripts/ts-run.js example", "generate-repros": "zx ../scripts/repros-generator/index.mjs", "github-release": "github-release-from-changelog", "linear-export": "ts-node --project=../scripts/tsconfig.json ../scripts/linear-export.ts", diff --git a/scripts/example.ts b/scripts/example.ts new file mode 100644 index 000000000000..e03078796ab0 --- /dev/null +++ b/scripts/example.ts @@ -0,0 +1,43 @@ +import { getOptionsOrPrompt } from './utils/options'; + +const frameworks = ['react']; +const addons = ['a11y', 'storysource']; + +getOptionsOrPrompt('yarn example', { + framework: { + flags: '-f, --framework ', + name: 'Which framework would you like to use?', + values: frameworks, + required: true, + }, + addon: { + flags: '-a, --addon ', + name: 'Which extra addons (beyond the CLI defaults) would you like installed?', + values: addons, + multiple: true, + }, + includeStories: { + flags: '-i, --include-stories', + name: "Include Storybook's own stories (only applies if a react-based framework is used)?", + }, + create: { + flags: '-c, --create', + name: 'Create the example from scratch (rather than degitting it)?', + }, + verdaccio: { + flags: '-v, --verdaccio', + name: 'Use verdaccio rather than yarn linking stories?', + }, + start: { + flags: '-S, --no-start', + name: "Don't start the example app?", + }, + build: { + flags: '-b, --build', + name: 'Build the example app?', + }, + watch: { + flags: '-w, --watch', + name: 'Start building used packages in watch mode as well as the example app?', + }, +}).then((r) => console.log(r)); diff --git a/scripts/ts-run.js b/scripts/ts-run.js new file mode 100644 index 000000000000..038daeca791d --- /dev/null +++ b/scripts/ts-run.js @@ -0,0 +1,5 @@ +require('./utils/register'); + +const scriptName = process.argv[2]; +// eslint-disable-next-line import/no-dynamic-require +require(`./${scriptName}`); diff --git a/scripts/utils/options.test.ts b/scripts/utils/options.test.ts new file mode 100644 index 000000000000..10e802cb6bfd --- /dev/null +++ b/scripts/utils/options.test.ts @@ -0,0 +1,104 @@ +/// ; + +import { createCommand } from 'commander'; + +import type { OptionSpecifier, StringOption, BooleanOption } from './options'; +import { getOptions, areOptionsSatisfied, getCommand } from './options'; + +const allOptions: OptionSpecifier = { + first: { + name: 'first', + flags: '-f, --first', + }, + second: { + name: 'second', + flags: '-s, --second', + }, + third: { + name: 'third', + flags: '-t, --third ', + values: ['one', 'two', 'three'], + required: true, + }, + fourth: { + name: 'fourth', + flags: '-f, --fourth ', + values: ['a', 'b', 'c'], + multiple: true, + }, +}; + +describe('getOptions', () => { + it('deals with boolean options', () => { + expect( + getOptions(createCommand() as any, allOptions, ['command', 'name', '--first']) + ).toMatchObject({ + first: true, + second: undefined, + }); + }); + + it('deals with string options', () => { + expect( + getOptions(createCommand() as any, allOptions, ['command', 'name', '--third', 'one']) + ).toMatchObject({ + third: 'one', + }); + }); + + it('deals with multiple string options', () => { + expect( + getOptions(createCommand() as any, allOptions, ['command', 'name', '--fourth', 'a']) + ).toMatchObject({ + fourth: ['a'], + }); + + expect( + getOptions(createCommand() as any, allOptions, [ + 'command', + 'name', + '--fourth', + 'a', + '--fourth', + 'b', + ]) + ).toMatchObject({ + fourth: ['a', 'b'], + }); + }); +}); + +describe('areOptionsSatisfied', () => { + it('checks each required string option has a value', () => { + expect(areOptionsSatisfied(allOptions, { fourth: ['a', 'c'] })).toBe(false); + expect(areOptionsSatisfied(allOptions, { third: ['one'] })).toBe(true); + }); +}); + +describe('getCommand', () => { + it('works with boolean options', () => { + expect(getCommand('node foo', allOptions, { first: true, second: false })).toBe( + 'node foo --first' + ); + }); + + it('works with string options', () => { + expect(getCommand('node foo', allOptions, { third: 'one' })).toBe('node foo --third one'); + }); + + it('works with multiple string options', () => { + expect(getCommand('node foo', allOptions, { fourth: ['a', 'b'] })).toBe( + 'node foo --fourth a --fourth b' + ); + }); + + it('works with combinations string options', () => { + expect( + getCommand('node foo', allOptions, { + first: true, + second: false, + fourth: ['a', 'b'], + }) + ).toBe('node foo --first --fourth a --fourth b'); + }); +}); diff --git a/scripts/utils/options.ts b/scripts/utils/options.ts new file mode 100644 index 000000000000..3521ffc147b1 --- /dev/null +++ b/scripts/utils/options.ts @@ -0,0 +1,124 @@ +/** + * Use commander and prompts to gather a list of options for a script + */ + +import prompts from 'prompts'; +import type { PromptObject } from 'prompts'; +import program from 'commander'; +import type { Command } from 'commander'; + +export type OptionId = string; +export type OptionValue = string | boolean; +export type BaseOption = { + name: string; + flags: string; +}; + +export type BooleanOption = BaseOption; + +export type StringOption = BaseOption & { + values: string[]; + multiple?: boolean; + required?: boolean; +}; + +export type Option = BooleanOption | StringOption; + +export type OptionSpecifier = Record; +export type OptionValues = Record; + +function isStringOption(option: Option): option is StringOption { + return 'values' in option; +} + +export function getOptions( + command: Command, + options: OptionSpecifier, + argv: string[] +): OptionValues { + Object.values(options) + .reduce((acc, option) => { + if (isStringOption(option) && option.multiple) { + return acc.option(option.flags, option.name, (x, l) => [...l, x], []); + } + return acc.option(option.flags, option.name); + }, command) + .parse(argv); + + return command.opts(); +} + +export function areOptionsSatisfied(options: OptionSpecifier, values: OptionValues) { + return !Object.entries(options) + .filter(([, option]) => isStringOption(option) && option.required) + .find(([key]) => !values[key]); +} + +export async function promptOptions( + options: OptionSpecifier, + values: OptionValues +): Promise { + const questions = Object.entries(options).map(([key, option]): PromptObject => { + if (isStringOption(option)) { + const currentValue = values[key]; + return { + type: option.multiple ? 'multiselect' : 'select', + message: option.name, + name: key, + // warn: ' ', + // pageSize: Object.keys(tasks).length + Object.keys(groups).length, + choices: option.values.map((value) => ({ + title: value, + value, + selected: + currentValue === value || + (Array.isArray(currentValue) && currentValue.includes?.(value)), + })), + }; + } + return { + type: 'toggle', + message: option.name, + name: key, + }; + }); + + const selection = await prompts(questions); + return selection; +} + +function getFlag(option: Option, value: OptionValue | OptionValue[]) { + const longFlag = option.flags.split(' ').find((f) => f.startsWith('--')); + if (isStringOption(option)) { + if (value) { + if (Array.isArray(value)) { + return value.map((v) => `${longFlag} ${v}`).join(' '); + } + return `${longFlag} ${value}`; + } + } + return value ? longFlag : ''; +} + +export function getCommand(prefix: string, options: OptionSpecifier, values: OptionValues) { + const flags = Object.keys(options) + .map((key) => getFlag(options[key], values[key])) + .filter(Boolean); + return `${prefix} ${flags.join(' ')}`; +} + +export async function getOptionsOrPrompt(commandPrefix: string, options: OptionSpecifier) { + const main = program.version('5.0.0'); + const cliValues = getOptions(main as any, options, process.argv); + + if (areOptionsSatisfied(options, cliValues)) { + return cliValues; + } + + const finalValues = await promptOptions(options, cliValues); + + const command = getCommand(commandPrefix, options, finalValues); + console.log(`\nTo run this directly next time, use:\n ${command}\n`); + + return finalValues; +} diff --git a/scripts/utils/register.js b/scripts/utils/register.js new file mode 100644 index 000000000000..ba00888edf4d --- /dev/null +++ b/scripts/utils/register.js @@ -0,0 +1,13 @@ +const { register } = require('esbuild-register/dist/node'); + +register({ + target: `node${process.version.slice(1)}`, + format: 'cjs', + hookIgnoreNodeModules: false, + tsconfigRaw: `{ + "compilerOptions": { + "strict": false, + "skipLibCheck": true, + }, + }`, +}); From 5a5bb0925e673294ee858760a596a67213260b99 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 25 Jul 2022 16:21:49 +1000 Subject: [PATCH 02/28] Automatically infer flags --- scripts/example.ts | 11 ++----- scripts/utils/options.test.ts | 41 ++++++++++++++++++----- scripts/utils/options.ts | 62 ++++++++++++++++++++++++++++------- 3 files changed, 84 insertions(+), 30 deletions(-) diff --git a/scripts/example.ts b/scripts/example.ts index e03078796ab0..00d4dcafac67 100644 --- a/scripts/example.ts +++ b/scripts/example.ts @@ -5,39 +5,32 @@ const addons = ['a11y', 'storysource']; getOptionsOrPrompt('yarn example', { framework: { - flags: '-f, --framework ', name: 'Which framework would you like to use?', values: frameworks, required: true, }, addon: { - flags: '-a, --addon ', name: 'Which extra addons (beyond the CLI defaults) would you like installed?', values: addons, multiple: true, }, includeStories: { - flags: '-i, --include-stories', name: "Include Storybook's own stories (only applies if a react-based framework is used)?", }, create: { - flags: '-c, --create', name: 'Create the example from scratch (rather than degitting it)?', }, verdaccio: { - flags: '-v, --verdaccio', name: 'Use verdaccio rather than yarn linking stories?', }, start: { - flags: '-S, --no-start', - name: "Don't start the example app?", + name: 'Start the example app?', + inverse: true, }, build: { - flags: '-b, --build', name: 'Build the example app?', }, watch: { - flags: '-w, --watch', name: 'Start building used packages in watch mode as well as the example app?', }, }).then((r) => console.log(r)); diff --git a/scripts/utils/options.test.ts b/scripts/utils/options.test.ts index 10e802cb6bfd..f8411ac950bb 100644 --- a/scripts/utils/options.test.ts +++ b/scripts/utils/options.test.ts @@ -8,21 +8,18 @@ import { getOptions, areOptionsSatisfied, getCommand } from './options'; const allOptions: OptionSpecifier = { first: { name: 'first', - flags: '-f, --first', }, second: { name: 'second', - flags: '-s, --second', + inverse: true, }, third: { name: 'third', - flags: '-t, --third ', values: ['one', 'two', 'three'], required: true, }, fourth: { name: 'fourth', - flags: '-f, --fourth ', values: ['a', 'b', 'c'], multiple: true, }, @@ -34,7 +31,25 @@ describe('getOptions', () => { getOptions(createCommand() as any, allOptions, ['command', 'name', '--first']) ).toMatchObject({ first: true, - second: undefined, + second: true, + }); + }); + + it('deals with inverse boolean options', () => { + expect( + getOptions(createCommand() as any, allOptions, ['command', 'name', '--no-second']) + ).toMatchObject({ + first: undefined, + second: false, + }); + }); + + it('deals with short options', () => { + expect( + getOptions(createCommand() as any, allOptions, ['command', 'name', '-f', '-S']) + ).toMatchObject({ + first: true, + second: false, }); }); @@ -77,17 +92,25 @@ describe('areOptionsSatisfied', () => { describe('getCommand', () => { it('works with boolean options', () => { - expect(getCommand('node foo', allOptions, { first: true, second: false })).toBe( + expect(getCommand('node foo', allOptions, { first: true, second: true })).toBe( 'node foo --first' ); }); + it('works with inverse boolean options', () => { + expect(getCommand('node foo', allOptions, { first: false, second: false })).toBe( + 'node foo --no-second' + ); + }); + it('works with string options', () => { - expect(getCommand('node foo', allOptions, { third: 'one' })).toBe('node foo --third one'); + expect(getCommand('node foo', allOptions, { second: true, third: 'one' })).toBe( + 'node foo --third one' + ); }); it('works with multiple string options', () => { - expect(getCommand('node foo', allOptions, { fourth: ['a', 'b'] })).toBe( + expect(getCommand('node foo', allOptions, { second: true, fourth: ['a', 'b'] })).toBe( 'node foo --fourth a --fourth b' ); }); @@ -99,6 +122,6 @@ describe('getCommand', () => { second: false, fourth: ['a', 'b'], }) - ).toBe('node foo --first --fourth a --fourth b'); + ).toBe('node foo --first --no-second --fourth a --fourth b'); }); }); diff --git a/scripts/utils/options.ts b/scripts/utils/options.ts index 3521ffc147b1..c717d9f0bf86 100644 --- a/scripts/utils/options.ts +++ b/scripts/utils/options.ts @@ -6,15 +6,25 @@ import prompts from 'prompts'; import type { PromptObject } from 'prompts'; import program from 'commander'; import type { Command } from 'commander'; +import kebabCase from 'lodash/kebabCase'; export type OptionId = string; export type OptionValue = string | boolean; export type BaseOption = { name: string; - flags: string; + /** + * By default the one-char version of the option key will be used as short flag. Override here, + * e.g. `shortFlag: 'c'` + */ + shortFlag?: string; }; -export type BooleanOption = BaseOption; +export type BooleanOption = BaseOption & { + /** + * Does this option default true? + */ + inverse?: boolean; +}; export type StringOption = BaseOption & { values: string[]; @@ -31,17 +41,43 @@ function isStringOption(option: Option): option is StringOption { return 'values' in option; } +function shortFlag(key: OptionId, option: Option) { + const inverse = !isStringOption(option) && option.inverse; + const defaultShortFlag = inverse ? key.substring(0, 1).toUpperCase() : key.substring(0, 1); + const shortFlag = option.shortFlag || defaultShortFlag; + if (shortFlag.length !== 1) { + throw new Error( + `Invalid shortFlag for ${key}: '${shortFlag}', needs to be a single character (e.g. 's')` + ); + } + return shortFlag; +} + +function longFlag(key: OptionId, option: Option) { + const inverse = !isStringOption(option) && option.inverse; + return inverse ? `no-${kebabCase(key)}` : kebabCase(key); +} + +function optionFlags(key: OptionId, option: Option) { + const base = `-${shortFlag(key, option)}, --${longFlag(key, option)}`; + if (isStringOption(option)) { + return `${base} <${key}>`; + } + return base; +} + export function getOptions( command: Command, options: OptionSpecifier, argv: string[] ): OptionValues { - Object.values(options) - .reduce((acc, option) => { + Object.entries(options) + .reduce((acc, [key, option]) => { + const flags = optionFlags(key, option); if (isStringOption(option) && option.multiple) { - return acc.option(option.flags, option.name, (x, l) => [...l, x], []); + return acc.option(flags, option.name, (x, l) => [...l, x], []); } - return acc.option(option.flags, option.name); + return acc.option(flags, option.name); }, command) .parse(argv); @@ -80,6 +116,7 @@ export async function promptOptions( type: 'toggle', message: option.name, name: key, + initial: option.inverse, }; }); @@ -87,22 +124,23 @@ export async function promptOptions( return selection; } -function getFlag(option: Option, value: OptionValue | OptionValue[]) { - const longFlag = option.flags.split(' ').find((f) => f.startsWith('--')); +function getFlag(key: OptionId, option: Option, value: OptionValue | OptionValue[]) { if (isStringOption(option)) { if (value) { if (Array.isArray(value)) { - return value.map((v) => `${longFlag} ${v}`).join(' '); + return value.map((v) => `--${longFlag(key, option)} ${v}`).join(' '); } - return `${longFlag} ${value}`; + return `--${longFlag(key, option)} ${value}`; } + return ''; } - return value ? longFlag : ''; + const toggled = option.inverse ? !value : value; + return toggled ? `--${longFlag(key, option)}` : ''; } export function getCommand(prefix: string, options: OptionSpecifier, values: OptionValues) { const flags = Object.keys(options) - .map((key) => getFlag(options[key], values[key])) + .map((key) => getFlag(key, options[key], values[key])) .filter(Boolean); return `${prefix} ${flags.join(' ')}`; } From c3c101853576da1df44b1dc77ee56c542677e309 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 25 Jul 2022 16:29:14 +1000 Subject: [PATCH 03/28] Add option validation/massaging --- scripts/utils/options.test.ts | 14 +++++++++++++- scripts/utils/options.ts | 30 +++++++++++++++++++++++++----- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/scripts/utils/options.test.ts b/scripts/utils/options.test.ts index f8411ac950bb..d9e427ce5af0 100644 --- a/scripts/utils/options.test.ts +++ b/scripts/utils/options.test.ts @@ -39,7 +39,7 @@ describe('getOptions', () => { expect( getOptions(createCommand() as any, allOptions, ['command', 'name', '--no-second']) ).toMatchObject({ - first: undefined, + first: false, second: false, }); }); @@ -61,6 +61,12 @@ describe('getOptions', () => { }); }); + it('disallows invalid string options', () => { + expect(() => + getOptions(createCommand() as any, allOptions, ['command', 'name', '--third', 'random']) + ).toThrow(/Invalid option provided/); + }); + it('deals with multiple string options', () => { expect( getOptions(createCommand() as any, allOptions, ['command', 'name', '--fourth', 'a']) @@ -81,6 +87,12 @@ describe('getOptions', () => { fourth: ['a', 'b'], }); }); + + it('disallows invalid multiple string options', () => { + expect(() => + getOptions(createCommand() as any, allOptions, ['command', 'name', '--fourth', 'random']) + ).toThrow(/Invalid option provided/); + }); }); describe('areOptionsSatisfied', () => { diff --git a/scripts/utils/options.ts b/scripts/utils/options.ts index c717d9f0bf86..7767c4366fea 100644 --- a/scripts/utils/options.ts +++ b/scripts/utils/options.ts @@ -7,6 +7,7 @@ import type { PromptObject } from 'prompts'; import program from 'commander'; import type { Command } from 'commander'; import kebabCase from 'lodash/kebabCase'; +import { raw } from 'express'; export type OptionId = string; export type OptionValue = string | boolean; @@ -66,11 +67,7 @@ function optionFlags(key: OptionId, option: Option) { return base; } -export function getOptions( - command: Command, - options: OptionSpecifier, - argv: string[] -): OptionValues { +function getRawOptions(command: Command, options: OptionSpecifier, argv: string[]): OptionValues { Object.entries(options) .reduce((acc, [key, option]) => { const flags = optionFlags(key, option); @@ -84,6 +81,29 @@ export function getOptions( return command.opts(); } +function validateOptions(options: OptionSpecifier, values: OptionValues) { + return Object.fromEntries( + Object.entries(options).map(([key, option]) => { + if (isStringOption(option)) { + const toCheck: string[] = option.multiple + ? (values[key] as string[]) + : [values[key] as string]; + const badValue = toCheck.find((value) => !option.values.includes(value)); + if (badValue) + throw new Error(`Invalid option provided to --${longFlag(key, option)}: '${badValue}'`); + + return [key, values[key]]; + } + return [key, !!values[key]]; + }) + ); +} + +export function getOptions(command: Command, options: OptionSpecifier, argv: string[]) { + const rawValues = getRawOptions(command, options, argv); + return validateOptions(options, rawValues); +} + export function areOptionsSatisfied(options: OptionSpecifier, values: OptionValues) { return !Object.entries(options) .filter(([, option]) => isStringOption(option) && option.required) From 1b447a5a0db100d9a0deb0e9a60723e428f9d2f0 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 25 Jul 2022 16:36:50 +1000 Subject: [PATCH 04/28] Small simplification --- scripts/utils/options.ts | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/scripts/utils/options.ts b/scripts/utils/options.ts index 7767c4366fea..ec90aef9c8a8 100644 --- a/scripts/utils/options.ts +++ b/scripts/utils/options.ts @@ -74,7 +74,7 @@ function getRawOptions(command: Command, options: OptionSpecifier, argv: string[ if (isStringOption(option) && option.multiple) { return acc.option(flags, option.name, (x, l) => [...l, x], []); } - return acc.option(flags, option.name); + return acc.option(flags, option.name, isStringOption(option) ? undefined : !!option.inverse); }, command) .parse(argv); @@ -82,26 +82,22 @@ function getRawOptions(command: Command, options: OptionSpecifier, argv: string[ } function validateOptions(options: OptionSpecifier, values: OptionValues) { - return Object.fromEntries( - Object.entries(options).map(([key, option]) => { - if (isStringOption(option)) { - const toCheck: string[] = option.multiple - ? (values[key] as string[]) - : [values[key] as string]; - const badValue = toCheck.find((value) => !option.values.includes(value)); - if (badValue) - throw new Error(`Invalid option provided to --${longFlag(key, option)}: '${badValue}'`); - - return [key, values[key]]; - } - return [key, !!values[key]]; - }) - ); + Object.entries(options).forEach(([key, option]) => { + if (isStringOption(option)) { + const toCheck: string[] = option.multiple + ? (values[key] as string[]) + : [values[key] as string]; + const badValue = toCheck.find((value) => !option.values.includes(value)); + if (badValue) + throw new Error(`Invalid option provided to --${longFlag(key, option)}: '${badValue}'`); + } + }); } export function getOptions(command: Command, options: OptionSpecifier, argv: string[]) { const rawValues = getRawOptions(command, options, argv); - return validateOptions(options, rawValues); + validateOptions(options, rawValues); + return rawValues; } export function areOptionsSatisfied(options: OptionSpecifier, values: OptionValues) { From 85fac873979949122b3eb41fc8602cfa8afd36bf Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 25 Jul 2022 16:37:54 +1000 Subject: [PATCH 05/28] Simply TS usage --- code/package.json | 2 +- scripts/utils/register.js | 13 ------------- 2 files changed, 1 insertion(+), 14 deletions(-) delete mode 100644 scripts/utils/register.js diff --git a/code/package.json b/code/package.json index 98c6cecdcaec..9e78de564412 100644 --- a/code/package.json +++ b/code/package.json @@ -62,7 +62,7 @@ "clean:dist": "del **/dist", "coverage": "codecov", "danger": "danger", - "example": "node ../scripts/ts-run.js example", + "example": "ts-node ../scripts/example.ts", "generate-repros": "zx ../scripts/repros-generator/index.mjs", "github-release": "github-release-from-changelog", "linear-export": "ts-node --project=../scripts/tsconfig.json ../scripts/linear-export.ts", diff --git a/scripts/utils/register.js b/scripts/utils/register.js deleted file mode 100644 index ba00888edf4d..000000000000 --- a/scripts/utils/register.js +++ /dev/null @@ -1,13 +0,0 @@ -const { register } = require('esbuild-register/dist/node'); - -register({ - target: `node${process.version.slice(1)}`, - format: 'cjs', - hookIgnoreNodeModules: false, - tsconfigRaw: `{ - "compilerOptions": { - "strict": false, - "skipLibCheck": true, - }, - }`, -}); From a742fe79a19c71e7ab44b6a540a190434541cc64 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 25 Jul 2022 17:00:35 +1000 Subject: [PATCH 06/28] Some UX improvements --- scripts/utils/options.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/utils/options.ts b/scripts/utils/options.ts index ec90aef9c8a8..47cb2217161a 100644 --- a/scripts/utils/options.ts +++ b/scripts/utils/options.ts @@ -114,7 +114,7 @@ export async function promptOptions( if (isStringOption(option)) { const currentValue = values[key]; return { - type: option.multiple ? 'multiselect' : 'select', + type: option.multiple ? 'autocompleteMultiselect' : 'select', message: option.name, name: key, // warn: ' ', @@ -133,6 +133,8 @@ export async function promptOptions( message: option.name, name: key, initial: option.inverse, + active: 'yes', + inactive: 'no', }; }); From 3872a0fb32a18141551b692b899721deb2c8a14e Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 25 Jul 2022 22:43:46 +1000 Subject: [PATCH 07/28] Update example command to run SB commands --- scripts/example.ts | 135 +++++++++++++++++++++++++++------- scripts/utils/cli-step.ts | 41 +++++++++++ scripts/utils/options.test.ts | 8 +- scripts/utils/options.ts | 15 ++-- 4 files changed, 163 insertions(+), 36 deletions(-) create mode 100644 scripts/utils/cli-step.ts diff --git a/scripts/example.ts b/scripts/example.ts index 00d4dcafac67..e9df0ad1e489 100644 --- a/scripts/example.ts +++ b/scripts/example.ts @@ -1,36 +1,119 @@ +import path from 'path'; +import { existsSync } from 'fs'; + import { getOptionsOrPrompt } from './utils/options'; +import type { CLIStep } from './utils/cli-step'; +import { executeCLIStep } from './utils/cli-step'; const frameworks = ['react']; const addons = ['a11y', 'storysource']; -getOptionsOrPrompt('yarn example', { - framework: { - name: 'Which framework would you like to use?', - values: frameworks, - required: true, - }, - addon: { - name: 'Which extra addons (beyond the CLI defaults) would you like installed?', - values: addons, - multiple: true, - }, - includeStories: { - name: "Include Storybook's own stories (only applies if a react-based framework is used)?", - }, - create: { - name: 'Create the example from scratch (rather than degitting it)?', - }, - verdaccio: { - name: 'Use verdaccio rather than yarn linking stories?', +async function getOptions() { + return getOptionsOrPrompt('yarn example', { + framework: { + description: 'Which framework would you like to use?', + values: frameworks, + required: true, + }, + addon: { + description: 'Which extra addons (beyond the CLI defaults) would you like installed?', + values: addons, + multiple: true, + }, + includeStories: { + description: + "Include Storybook's own stories (only applies if a react-based framework is used)?", + }, + create: { + description: 'Create the example from scratch (rather than degitting it)?', + }, + verdaccio: { + description: 'Use verdaccio rather than yarn linking stories?', + }, + start: { + description: 'Start the example app?', + inverse: true, + }, + build: { + description: 'Build the example app?', + }, + watch: { + description: 'Start building used packages in watch mode as well as the example app?', + }, + }); +} + +const steps: Record = { + repro: { + command: 'repro', + description: 'Bootstrapping example', + icon: '👷', + hasArgument: true, + options: { + template: { values: frameworks }, + e2e: {}, + }, }, - start: { - name: 'Start the example app?', - inverse: true, + add: { + command: 'add', + description: 'Adding addon', + icon: '+', + hasArgument: true, + options: {}, }, build: { - name: 'Build the example app?', + command: 'build', + description: 'Building example', + icon: '🔨', + options: {}, }, - watch: { - name: 'Start building used packages in watch mode as well as the example app?', + dev: { + command: 'dev', + description: 'Starting example', + icon: '🖥', + options: {}, }, -}).then((r) => console.log(r)); +}; + +async function main() { + const optionValues = await getOptions(); + const examplesDir = path.resolve(__dirname, '../examples'); + + const { framework } = optionValues; + const cwd = path.join(examplesDir, framework as string); + + // TODO -- what to do when the directory already exists? + if (!existsSync(cwd)) { + await executeCLIStep(steps.repro, { + argument: cwd, + optionValues: { template: framework }, + cwd: examplesDir, + }); + + // TODO -- sb add doesn't actually work properly: + // - installs in `deps` not `devDeps` + // - does a `workspace:^` install (what does that mean?) + // - doesn't add to `main.js` + + // eslint-disable-next-line no-restricted-syntax + for (const addon of optionValues.addon as string[]) { + const addonName = `@storybook/addon-${addon}`; + // eslint-disable-next-line no-await-in-loop + await executeCLIStep(steps.add, { argument: addonName, cwd }); + } + + // TODO copy stories + } + + const { start } = optionValues; + if (start) { + await executeCLIStep(steps.dev, { cwd }); + } else { + await executeCLIStep(steps.build, { cwd }); + // TODO serve + } + + // TODO start dev +} + +main().catch((err) => console.error(err)); diff --git a/scripts/utils/cli-step.ts b/scripts/utils/cli-step.ts new file mode 100644 index 000000000000..17048a9be71c --- /dev/null +++ b/scripts/utils/cli-step.ts @@ -0,0 +1,41 @@ +import { getCommand, OptionSpecifier, OptionValues } from './options'; +import { exec } from '../../lib/cli/src/repro-generators/scripts'; + +const cliExecutable = require.resolve('../../lib/cli/bin/index.js'); + +export type CLIStep = { + command: string; + description: string; + hasArgument?: boolean; + icon: string; + // It would be kind of great to be able to share these with `lib/cli/src/generate.ts` + options: OptionSpecifier; +}; + +export async function executeCLIStep( + cliStep: CLIStep, + options: { + argument?: string; + optionValues?: OptionValues; + cwd: string; + } +) { + if (cliStep.hasArgument && !options.argument) + throw new Error(`Argument required for ${cliStep.command} command.`); + + const prefix = `node ${cliExecutable} ${cliStep.command}`; + const command = getCommand( + cliStep.hasArgument ? `${prefix} ${options.argument}` : prefix, + cliStep.options, + options.optionValues || {} + ); + + await exec( + command, + { cwd: options.cwd }, + { + startMessage: `${cliStep.icon} ${cliStep.description}`, + errorMessage: `🚨 ${cliStep.description} failed`, + } + ); +} diff --git a/scripts/utils/options.test.ts b/scripts/utils/options.test.ts index d9e427ce5af0..d8ff0eedd33d 100644 --- a/scripts/utils/options.test.ts +++ b/scripts/utils/options.test.ts @@ -7,19 +7,19 @@ import { getOptions, areOptionsSatisfied, getCommand } from './options'; const allOptions: OptionSpecifier = { first: { - name: 'first', + description: 'first', }, second: { - name: 'second', + description: 'second', inverse: true, }, third: { - name: 'third', + description: 'third', values: ['one', 'two', 'three'], required: true, }, fourth: { - name: 'fourth', + description: 'fourth', values: ['a', 'b', 'c'], multiple: true, }, diff --git a/scripts/utils/options.ts b/scripts/utils/options.ts index 47cb2217161a..41b5d14c4969 100644 --- a/scripts/utils/options.ts +++ b/scripts/utils/options.ts @@ -7,12 +7,11 @@ import type { PromptObject } from 'prompts'; import program from 'commander'; import type { Command } from 'commander'; import kebabCase from 'lodash/kebabCase'; -import { raw } from 'express'; export type OptionId = string; export type OptionValue = string | boolean; export type BaseOption = { - name: string; + description?: string; /** * By default the one-char version of the option key will be used as short flag. Override here, * e.g. `shortFlag: 'c'` @@ -72,9 +71,13 @@ function getRawOptions(command: Command, options: OptionSpecifier, argv: string[ .reduce((acc, [key, option]) => { const flags = optionFlags(key, option); if (isStringOption(option) && option.multiple) { - return acc.option(flags, option.name, (x, l) => [...l, x], []); + return acc.option(flags, option.description, (x, l) => [...l, x], []); } - return acc.option(flags, option.name, isStringOption(option) ? undefined : !!option.inverse); + return acc.option( + flags, + option.description, + isStringOption(option) ? undefined : !!option.inverse + ); }, command) .parse(argv); @@ -115,7 +118,7 @@ export async function promptOptions( const currentValue = values[key]; return { type: option.multiple ? 'autocompleteMultiselect' : 'select', - message: option.name, + message: option.description, name: key, // warn: ' ', // pageSize: Object.keys(tasks).length + Object.keys(groups).length, @@ -130,7 +133,7 @@ export async function promptOptions( } return { type: 'toggle', - message: option.name, + message: option.description, name: key, initial: option.inverse, active: 'yes', From a067ce563be849fb71de532afd6d0d3124aed3f4 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 25 Jul 2022 22:44:37 +1000 Subject: [PATCH 08/28] Don't need this --- scripts/ts-run.js | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 scripts/ts-run.js diff --git a/scripts/ts-run.js b/scripts/ts-run.js deleted file mode 100644 index 038daeca791d..000000000000 --- a/scripts/ts-run.js +++ /dev/null @@ -1,5 +0,0 @@ -require('./utils/register'); - -const scriptName = process.argv[2]; -// eslint-disable-next-line import/no-dynamic-require -require(`./${scriptName}`); From 48e87e39db3df4ea970130dab2fbcc29cb750771 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Tue, 26 Jul 2022 16:12:07 +1000 Subject: [PATCH 09/28] Cleanup --- scripts/example.ts | 2 +- scripts/package.json | 4 ++++ scripts/utils/cli-step.ts | 4 ++-- scripts/yarn.lock | 9 +++++++++ 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/scripts/example.ts b/scripts/example.ts index e9df0ad1e489..ebda5a7593ef 100644 --- a/scripts/example.ts +++ b/scripts/example.ts @@ -7,6 +7,7 @@ import { executeCLIStep } from './utils/cli-step'; const frameworks = ['react']; const addons = ['a11y', 'storysource']; +const examplesDir = path.resolve(__dirname, '../examples'); async function getOptions() { return getOptionsOrPrompt('yarn example', { @@ -77,7 +78,6 @@ const steps: Record = { async function main() { const optionValues = await getOptions(); - const examplesDir = path.resolve(__dirname, '../examples'); const { framework } = optionValues; const cwd = path.join(examplesDir, framework as string); diff --git a/scripts/package.json b/scripts/package.json index 3f0af3afbd6d..044677b5d172 100644 --- a/scripts/package.json +++ b/scripts/package.json @@ -140,6 +140,7 @@ "jest-watch-typeahead": "^0.6.1", "js-yaml": "^3.14.1", "lint-staged": "^10.5.4", + "lodash": "^4.17.21", "mocha-list-tests": "^1.0.5", "node-cleanup": "^2.1.2", "node-fetch": "^2.6.1", @@ -190,5 +191,8 @@ "engines": { "node": ">=10.13.0", "yarn": ">=1.3.2" + }, + "devDependencies": { + "@types/lodash": "^4" } } diff --git a/scripts/utils/cli-step.ts b/scripts/utils/cli-step.ts index 17048a9be71c..63eba1df27e2 100644 --- a/scripts/utils/cli-step.ts +++ b/scripts/utils/cli-step.ts @@ -1,7 +1,7 @@ import { getCommand, OptionSpecifier, OptionValues } from './options'; -import { exec } from '../../lib/cli/src/repro-generators/scripts'; +import { exec } from '../../code/lib/cli/src/repro-generators/scripts'; -const cliExecutable = require.resolve('../../lib/cli/bin/index.js'); +const cliExecutable = require.resolve('../../code/lib/cli/bin/index.js'); export type CLIStep = { command: string; diff --git a/scripts/yarn.lock b/scripts/yarn.lock index 7ebe87564f9f..1dad8d9f04f7 100644 --- a/scripts/yarn.lock +++ b/scripts/yarn.lock @@ -3269,6 +3269,7 @@ __metadata: "@types/fs-extra": ^9.0.6 "@types/jest": ^26.0.16 "@types/js-yaml": ^3.12.6 + "@types/lodash": ^4 "@types/node": ^14.14.20 || ^16.0.0 "@types/node-cleanup": ^2.1.1 "@types/node-fetch": ^2.5.7 @@ -3327,6 +3328,7 @@ __metadata: jest-watch-typeahead: ^0.6.1 js-yaml: ^3.14.1 lint-staged: ^10.5.4 + lodash: ^4.17.21 mocha-list-tests: ^1.0.5 node-cleanup: ^2.1.2 node-fetch: ^2.6.1 @@ -3814,6 +3816,13 @@ __metadata: languageName: node linkType: hard +"@types/lodash@npm:^4": + version: 4.14.182 + resolution: "@types/lodash@npm:4.14.182" + checksum: d6bd4789dfb3be631d5e3277e6a1be5becb21440f3364f5d15b982c2e6b6bb1f8048d46fc5bff5ef0f90bebaf4d07c49b2919ba369d07af72d3beb3fea70c61a + languageName: node + linkType: hard + "@types/mdast@npm:^3.0.0": version: 3.0.10 resolution: "@types/mdast@npm:3.0.10" From 6cf650e816f7e8bf30734e368e0a9586ba2ddf2a Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Tue, 26 Jul 2022 16:20:49 +1000 Subject: [PATCH 10/28] Add a dry-run option to command --- code/lib/cli/src/repro-generators/scripts.ts | 14 +++++++++++--- scripts/example.ts | 14 +++++++++----- scripts/utils/cli-step.ts | 4 ++++ 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/code/lib/cli/src/repro-generators/scripts.ts b/code/lib/cli/src/repro-generators/scripts.ts index 5791c30c9a4f..35ac70ad6454 100644 --- a/code/lib/cli/src/repro-generators/scripts.ts +++ b/code/lib/cli/src/repro-generators/scripts.ts @@ -51,11 +51,19 @@ export interface Options extends Parameters { export const exec = async ( command: string, options: ExecOptions = {}, - { startMessage, errorMessage }: { startMessage?: string; errorMessage?: string } = {} + { + startMessage, + errorMessage, + dryRun, + }: { startMessage?: string; errorMessage?: string; dryRun?: boolean } = {} ) => { - if (startMessage) { - logger.info(startMessage); + if (startMessage) logger.info(startMessage); + + if (dryRun) { + logger.info(`\n> ${command}\n`); + return undefined; } + logger.debug(command); return new Promise((resolve, reject) => { const defaultOptions: ExecOptions = { diff --git a/scripts/example.ts b/scripts/example.ts index ebda5a7593ef..c3493e25a6da 100644 --- a/scripts/example.ts +++ b/scripts/example.ts @@ -41,6 +41,9 @@ async function getOptions() { watch: { description: 'Start building used packages in watch mode as well as the example app?', }, + dryRun: { + description: "Don't execute commands, just list them?", + }, }); } @@ -71,7 +74,7 @@ const steps: Record = { dev: { command: 'dev', description: 'Starting example', - icon: '🖥', + icon: '🖥 ', options: {}, }, }; @@ -79,7 +82,7 @@ const steps: Record = { async function main() { const optionValues = await getOptions(); - const { framework } = optionValues; + const { framework, dryRun } = optionValues; const cwd = path.join(examplesDir, framework as string); // TODO -- what to do when the directory already exists? @@ -88,6 +91,7 @@ async function main() { argument: cwd, optionValues: { template: framework }, cwd: examplesDir, + dryRun, }); // TODO -- sb add doesn't actually work properly: @@ -99,7 +103,7 @@ async function main() { for (const addon of optionValues.addon as string[]) { const addonName = `@storybook/addon-${addon}`; // eslint-disable-next-line no-await-in-loop - await executeCLIStep(steps.add, { argument: addonName, cwd }); + await executeCLIStep(steps.add, { argument: addonName, cwd, dryRun }); } // TODO copy stories @@ -107,9 +111,9 @@ async function main() { const { start } = optionValues; if (start) { - await executeCLIStep(steps.dev, { cwd }); + await executeCLIStep(steps.dev, { cwd, dryRun }); } else { - await executeCLIStep(steps.build, { cwd }); + await executeCLIStep(steps.build, { cwd, dryRun }); // TODO serve } diff --git a/scripts/utils/cli-step.ts b/scripts/utils/cli-step.ts index 63eba1df27e2..6d2f2db3cd98 100644 --- a/scripts/utils/cli-step.ts +++ b/scripts/utils/cli-step.ts @@ -1,6 +1,8 @@ import { getCommand, OptionSpecifier, OptionValues } from './options'; import { exec } from '../../code/lib/cli/src/repro-generators/scripts'; +const logger = console; + const cliExecutable = require.resolve('../../code/lib/cli/bin/index.js'); export type CLIStep = { @@ -18,6 +20,7 @@ export async function executeCLIStep( argument?: string; optionValues?: OptionValues; cwd: string; + dryRun?: boolean; } ) { if (cliStep.hasArgument && !options.argument) @@ -36,6 +39,7 @@ export async function executeCLIStep( { startMessage: `${cliStep.icon} ${cliStep.description}`, errorMessage: `🚨 ${cliStep.description} failed`, + dryRun: options.dryRun, } ); } From 38d53e58bcb83fa4787ceec7d7b6564601f489c9 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Wed, 27 Jul 2022 20:13:14 +1000 Subject: [PATCH 11/28] Add script tests as a separate circle job --- .circleci/config.yml | 17 +++++++++++++++++ scripts/jest.config.js | 1 + scripts/package.json | 4 +++- scripts/yarn.lock | 1 + 4 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 scripts/jest.config.js diff --git a/.circleci/config.yml b/.circleci/config.yml index 2905654d1a78..849cf28105a5 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -410,6 +410,20 @@ jobs: command: | cd code yarn lint + script-unit-tests: + executor: sb_node_14_browsers + steps: + - git-shallow-clone/checkout_advanced: + clone_options: '--depth 1 --verbose' + - attach_workspace: + at: . + - run: + name: Test + command: | + cd scripts + yarn test --coverage --runInBand --ci + - store_test_results: + path: scripts/junit.xml unit-tests: executor: sb_node_14_browsers steps: @@ -462,6 +476,9 @@ workflows: - unit-tests: requires: - build + - script-unit-tests: + requires: + - build - coverage: requires: - unit-tests diff --git a/scripts/jest.config.js b/scripts/jest.config.js new file mode 100644 index 000000000000..f053ebf7976e --- /dev/null +++ b/scripts/jest.config.js @@ -0,0 +1 @@ +module.exports = {}; diff --git a/scripts/package.json b/scripts/package.json index 044677b5d172..791af4510268 100644 --- a/scripts/package.json +++ b/scripts/package.json @@ -6,7 +6,8 @@ "lint": "yarn lint:js && yarn lint:md", "lint:js": "yarn lint:js:cmd . --quiet", "lint:js:cmd": "cross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directives", - "lint:package": "sort-package-json" + "lint:package": "sort-package-json", + "test": "jest --config ./jest.config.js" }, "husky": { "hooks": { @@ -54,6 +55,7 @@ "@compodoc/compodoc": "^1.1.18", "@emotion/babel-plugin": "^11.7.2", "@emotion/jest": "^11.8.0", + "@jest/globals": "^26.6.2", "@linear/sdk": "^1.21.0", "@nicolo-ribaudo/chokidar-2": "^2.1.8", "@nrwl/cli": "12.3.4", diff --git a/scripts/yarn.lock b/scripts/yarn.lock index 1dad8d9f04f7..59a411fc170b 100644 --- a/scripts/yarn.lock +++ b/scripts/yarn.lock @@ -3240,6 +3240,7 @@ __metadata: "@cypress/webpack-preprocessor": ^5.9.1 "@emotion/babel-plugin": ^11.7.2 "@emotion/jest": ^11.8.0 + "@jest/globals": ^26.6.2 "@linear/sdk": ^1.21.0 "@nicolo-ribaudo/chokidar-2": ^2.1.8 "@nrwl/cli": 12.3.4 From c9df02b8a8368d7d1b4cf0340f8f17e4a2697365 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Wed, 27 Jul 2022 20:13:36 +1000 Subject: [PATCH 12/28] Do typing shenanigans for options --- scripts/utils/options.test.ts | 59 ++++++++---- scripts/utils/options.ts | 175 +++++++++++++++++++++++----------- 2 files changed, 160 insertions(+), 74 deletions(-) diff --git a/scripts/utils/options.test.ts b/scripts/utils/options.test.ts index d8ff0eedd33d..c181ca70d7fe 100644 --- a/scripts/utils/options.test.ts +++ b/scripts/utils/options.test.ts @@ -1,11 +1,15 @@ -/// ; - import { createCommand } from 'commander'; +import { describe, it, expect } from '@jest/globals'; -import type { OptionSpecifier, StringOption, BooleanOption } from './options'; -import { getOptions, areOptionsSatisfied, getCommand } from './options'; +import { + getOptions, + areOptionsSatisfied, + getCommand, + OptionValues, + MaybeOptionValues, +} from './options'; -const allOptions: OptionSpecifier = { +const allOptions = { first: { description: 'first', }, @@ -25,6 +29,14 @@ const allOptions: OptionSpecifier = { }, }; +// TS "tests" +function test(mv: MaybeOptionValues, v: OptionValues) { + console.log(mv.first, mv.second, mv.third, mv.fourth); + // console.log(mv.fifth); // not allowed + console.log(v.first, v.second, v.third, v.fourth); + // console.log(v.fifth); // not allowed +} + describe('getOptions', () => { it('deals with boolean options', () => { expect( @@ -54,6 +66,7 @@ describe('getOptions', () => { }); it('deals with string options', () => { + const r = getOptions(createCommand() as any, allOptions, ['command', 'name', '--third', 'one']); expect( getOptions(createCommand() as any, allOptions, ['command', 'name', '--third', 'one']) ).toMatchObject({ @@ -64,7 +77,7 @@ describe('getOptions', () => { it('disallows invalid string options', () => { expect(() => getOptions(createCommand() as any, allOptions, ['command', 'name', '--third', 'random']) - ).toThrow(/Invalid option provided/); + ).toThrow(/Unexpected value/); }); it('deals with multiple string options', () => { @@ -91,38 +104,51 @@ describe('getOptions', () => { it('disallows invalid multiple string options', () => { expect(() => getOptions(createCommand() as any, allOptions, ['command', 'name', '--fourth', 'random']) - ).toThrow(/Invalid option provided/); + ).toThrow(/Unexpected value/); }); }); describe('areOptionsSatisfied', () => { it('checks each required string option has a value', () => { - expect(areOptionsSatisfied(allOptions, { fourth: ['a', 'c'] })).toBe(false); - expect(areOptionsSatisfied(allOptions, { third: ['one'] })).toBe(true); + expect( + areOptionsSatisfied(allOptions, { + first: true, + second: true, + third: undefined, + fourth: ['a', 'c'], + }) + ).toBe(false); + expect( + areOptionsSatisfied(allOptions, { + first: true, + second: true, + third: 'one', + fourth: [], + }) + ).toBe(true); }); }); describe('getCommand', () => { + const { first, second, third, fourth } = allOptions; it('works with boolean options', () => { - expect(getCommand('node foo', allOptions, { first: true, second: true })).toBe( + expect(getCommand('node foo', { first, second }, { first: true, second: true })).toBe( 'node foo --first' ); }); it('works with inverse boolean options', () => { - expect(getCommand('node foo', allOptions, { first: false, second: false })).toBe( + expect(getCommand('node foo', { first, second }, { first: false, second: false })).toBe( 'node foo --no-second' ); }); it('works with string options', () => { - expect(getCommand('node foo', allOptions, { second: true, third: 'one' })).toBe( - 'node foo --third one' - ); + expect(getCommand('node foo', { third }, { third: 'one' })).toBe('node foo --third one'); }); it('works with multiple string options', () => { - expect(getCommand('node foo', allOptions, { second: true, fourth: ['a', 'b'] })).toBe( + expect(getCommand('node foo', { fourth }, { fourth: ['a', 'b'] })).toBe( 'node foo --fourth a --fourth b' ); }); @@ -132,8 +158,9 @@ describe('getCommand', () => { getCommand('node foo', allOptions, { first: true, second: false, + third: 'one', fourth: ['a', 'b'], }) - ).toBe('node foo --first --no-second --fourth a --fourth b'); + ).toBe('node foo --first --no-second --third one --fourth a --fourth b'); }); }); diff --git a/scripts/utils/options.ts b/scripts/utils/options.ts index 41b5d14c4969..07ca42b633ca 100644 --- a/scripts/utils/options.ts +++ b/scripts/utils/options.ts @@ -8,8 +8,9 @@ import program from 'commander'; import type { Command } from 'commander'; import kebabCase from 'lodash/kebabCase'; +// Option types + export type OptionId = string; -export type OptionValue = string | boolean; export type BaseOption = { description?: string; /** @@ -28,96 +29,135 @@ export type BooleanOption = BaseOption & { export type StringOption = BaseOption & { values: string[]; - multiple?: boolean; required?: boolean; }; -export type Option = BooleanOption | StringOption; +export type StringArrayOption = BaseOption & { + values: string[]; + multiple: true; +}; + +// StringArrayOption requires `multiple: true;` but unless you use `as const` an object with +// { multiple: true } will be inferred as `multiple: boolean;` +type StringArrayOptionMatch = Omit & { multiple: boolean }; + +export type Option = BooleanOption | StringOption | StringArrayOption; +export type MaybeOptionValue = TOption extends StringArrayOptionMatch + ? string[] + : TOption extends StringOption + ? string | undefined + : TOption extends BooleanOption + ? boolean + : never; + +// Note we use `required: boolean;` rather than `required: true` here for the same reason +// as `StringArrayOptionMatch` above. In both cases, the field should only ever be set to true +export type OptionValue = TOption extends { required: boolean } + ? string + : MaybeOptionValue; export type OptionSpecifier = Record; -export type OptionValues = Record; +export type MaybeOptionValues = { + [TKey in keyof TOptions]: MaybeOptionValue; +}; + +export type OptionValues = { + [TKey in keyof TOptions]: OptionValue; +}; -function isStringOption(option: Option): option is StringOption { - return 'values' in option; +export function isStringOption(option: Option): option is StringOption { + return 'values' in option && !('multiple' in option); +} + +export function isBooleanOption(option: Option): option is BooleanOption { + return !('values' in option); +} + +export function isStringArrayOption(option: Option): option is StringArrayOption { + return 'values' in option && 'multiple' in option; } function shortFlag(key: OptionId, option: Option) { - const inverse = !isStringOption(option) && option.inverse; + const inverse = isBooleanOption(option) && option.inverse; const defaultShortFlag = inverse ? key.substring(0, 1).toUpperCase() : key.substring(0, 1); - const shortFlag = option.shortFlag || defaultShortFlag; - if (shortFlag.length !== 1) { + const short = option.shortFlag || defaultShortFlag; + if (short.length !== 1) { throw new Error( - `Invalid shortFlag for ${key}: '${shortFlag}', needs to be a single character (e.g. 's')` + `Invalid shortFlag for ${key}: '${short}', needs to be a single character (e.g. 's')` ); } - return shortFlag; + return short; } function longFlag(key: OptionId, option: Option) { - const inverse = !isStringOption(option) && option.inverse; + const inverse = isBooleanOption(option) && option.inverse; return inverse ? `no-${kebabCase(key)}` : kebabCase(key); } function optionFlags(key: OptionId, option: Option) { const base = `-${shortFlag(key, option)}, --${longFlag(key, option)}`; - if (isStringOption(option)) { + if (isStringOption(option) || isStringArrayOption(option)) { return `${base} <${key}>`; } return base; } -function getRawOptions(command: Command, options: OptionSpecifier, argv: string[]): OptionValues { +export function getOptions( + command: Command, + options: TOptions, + argv: string[] +): MaybeOptionValues { Object.entries(options) .reduce((acc, [key, option]) => { const flags = optionFlags(key, option); - if (isStringOption(option) && option.multiple) { - return acc.option(flags, option.description, (x, l) => [...l, x], []); + + if (isBooleanOption(option)) return acc.option(flags, option.description, !!option.inverse); + + const checkStringValue = (raw: string) => { + if (!option.values.includes(raw)) + throw new Error(`Unexpected value '${raw}' for option '${key}'`); + return raw; + }; + + if (isStringOption(option)) + return acc.option(flags, option.description, (raw) => checkStringValue(raw)); + + if (isStringArrayOption(option)) { + return acc.option( + flags, + option.description, + (raw, values) => [...values, checkStringValue(raw)], + [] + ); } - return acc.option( - flags, - option.description, - isStringOption(option) ? undefined : !!option.inverse - ); + + throw new Error(`Unexpected option type '${key}'`); }, command) .parse(argv); - return command.opts(); -} - -function validateOptions(options: OptionSpecifier, values: OptionValues) { - Object.entries(options).forEach(([key, option]) => { - if (isStringOption(option)) { - const toCheck: string[] = option.multiple - ? (values[key] as string[]) - : [values[key] as string]; - const badValue = toCheck.find((value) => !option.values.includes(value)); - if (badValue) - throw new Error(`Invalid option provided to --${longFlag(key, option)}: '${badValue}'`); - } - }); + // Note the code above guarantees the types as they come in, so we cast here. + // Not sure there is an easier way to do this + return command.opts() as MaybeOptionValue; } -export function getOptions(command: Command, options: OptionSpecifier, argv: string[]) { - const rawValues = getRawOptions(command, options, argv); - validateOptions(options, rawValues); - return rawValues; -} - -export function areOptionsSatisfied(options: OptionSpecifier, values: OptionValues) { +export function areOptionsSatisfied( + options: TOptions, + values: MaybeOptionValues +) { return !Object.entries(options) .filter(([, option]) => isStringOption(option) && option.required) .find(([key]) => !values[key]); } -export async function promptOptions( - options: OptionSpecifier, - values: OptionValues -): Promise { +export async function promptOptions( + options: TOptions, + values: MaybeOptionValues +): Promise> { const questions = Object.entries(options).map(([key, option]): PromptObject => { - if (isStringOption(option)) { + if (!isBooleanOption(option)) { const currentValue = values[key]; return { - type: option.multiple ? 'autocompleteMultiselect' : 'select', + type: isStringArrayOption(option) ? 'autocompleteMultiselect' : 'select', message: option.description, name: key, // warn: ' ', @@ -142,36 +182,55 @@ export async function promptOptions( }); const selection = await prompts(questions); - return selection; + // Again the structure of the questions guarantees we get responses of the type we need + return selection as OptionValues; } -function getFlag(key: OptionId, option: Option, value: OptionValue | OptionValue[]) { +function getFlag( + key: OptionId, + option: TOption, + value: OptionValue +) { + if (isBooleanOption(option)) { + const toggled = option.inverse ? !value : value; + return toggled ? `--${longFlag(key, option)}` : ''; + } + + if (isStringArrayOption(option)) { + return value.map((v) => `--${longFlag(key, option)} ${v}`).join(' '); + } + if (isStringOption(option)) { if (value) { - if (Array.isArray(value)) { - return value.map((v) => `--${longFlag(key, option)} ${v}`).join(' '); - } return `--${longFlag(key, option)} ${value}`; } return ''; } - const toggled = option.inverse ? !value : value; - return toggled ? `--${longFlag(key, option)}` : ''; + + throw new Error(`Unknown option type for '${key}'`); } -export function getCommand(prefix: string, options: OptionSpecifier, values: OptionValues) { +export function getCommand( + prefix: string, + options: TOptions, + values: OptionValues +) { const flags = Object.keys(options) .map((key) => getFlag(key, options[key], values[key])) .filter(Boolean); return `${prefix} ${flags.join(' ')}`; } -export async function getOptionsOrPrompt(commandPrefix: string, options: OptionSpecifier) { +export async function getOptionsOrPrompt( + commandPrefix: string, + options: TOptions +): Promise> { const main = program.version('5.0.0'); const cliValues = getOptions(main as any, options, process.argv); if (areOptionsSatisfied(options, cliValues)) { - return cliValues; + // areOptionsSatisfied could be a type predicate but I'm not quite sure how to do it + return cliValues as OptionValues; } const finalValues = await promptOptions(options, cliValues); From b99d16da69c6e79cfa72a6126154eb6210803fd0 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Wed, 27 Jul 2022 21:22:17 +1000 Subject: [PATCH 13/28] Drop scripts from code tests --- code/jest.config.js | 1 - 1 file changed, 1 deletion(-) diff --git a/code/jest.config.js b/code/jest.config.js index 8bb68a3d143a..a2ffa022105e 100644 --- a/code/jest.config.js +++ b/code/jest.config.js @@ -42,7 +42,6 @@ module.exports = { '/addons', '/frameworks', '/lib', - '/scripts', '/examples/official-storybook', '/examples/react-ts', ], From 8e9515691aef1d4ba41a482711610e9de8e7e025 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Wed, 27 Jul 2022 22:09:36 +1000 Subject: [PATCH 14/28] Add basic logic to prompt to reuse or delete existing projects --- scripts/example.ts | 31 ++++++++++++++++++++++++++----- scripts/utils/cli-step.ts | 12 ++++++------ scripts/utils/options.ts | 2 +- 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/scripts/example.ts b/scripts/example.ts index c3493e25a6da..2854fa5d1b4d 100644 --- a/scripts/example.ts +++ b/scripts/example.ts @@ -1,5 +1,6 @@ import path from 'path'; -import { existsSync } from 'fs'; +import { remove, pathExists } from 'fs-extra'; +import prompts from 'prompts'; import { getOptionsOrPrompt } from './utils/options'; import type { CLIStep } from './utils/cli-step'; @@ -28,6 +29,12 @@ async function getOptions() { create: { description: 'Create the example from scratch (rather than degitting it)?', }, + forceDelete: { + description: 'Always delete an existing example, even if it has the same configuration?', + }, + forceReuse: { + description: 'Always reusing an existing example, even if it has a different configuration?', + }, verdaccio: { description: 'Use verdaccio rather than yarn linking stories?', }, @@ -47,7 +54,7 @@ async function getOptions() { }); } -const steps: Record = { +const steps = { repro: { command: 'repro', description: 'Bootstrapping example', @@ -82,11 +89,25 @@ const steps: Record = { async function main() { const optionValues = await getOptions(); - const { framework, dryRun } = optionValues; + const { framework, forceDelete, forceReuse, dryRun } = optionValues; const cwd = path.join(examplesDir, framework as string); - // TODO -- what to do when the directory already exists? - if (!existsSync(cwd)) { + const exists = await pathExists(cwd); + let shouldReuse = exists && forceReuse; + if (exists && !forceDelete && !forceReuse) { + ({ shouldReuse } = await prompts({ + type: 'toggle', + message: `${path.relative(process.cwd(), cwd)} already exists, should we reuse it?`, + name: 'shouldReuse', + initial: true, + active: 'yes', + inactive: 'no', + })); + } + + if (exists && !shouldReuse) await remove(cwd); + + if (!shouldReuse) { await executeCLIStep(steps.repro, { argument: cwd, optionValues: { template: framework }, diff --git a/scripts/utils/cli-step.ts b/scripts/utils/cli-step.ts index 6d2f2db3cd98..52896ebcf3fb 100644 --- a/scripts/utils/cli-step.ts +++ b/scripts/utils/cli-step.ts @@ -5,20 +5,20 @@ const logger = console; const cliExecutable = require.resolve('../../code/lib/cli/bin/index.js'); -export type CLIStep = { +export type CLIStep = { command: string; description: string; hasArgument?: boolean; icon: string; // It would be kind of great to be able to share these with `lib/cli/src/generate.ts` - options: OptionSpecifier; + options: TOptions; }; -export async function executeCLIStep( - cliStep: CLIStep, +export async function executeCLIStep( + cliStep: CLIStep, options: { argument?: string; - optionValues?: OptionValues; + optionValues: OptionValues; cwd: string; dryRun?: boolean; } @@ -30,7 +30,7 @@ export async function executeCLIStep( const command = getCommand( cliStep.hasArgument ? `${prefix} ${options.argument}` : prefix, cliStep.options, - options.optionValues || {} + options.optionValues ); await exec( diff --git a/scripts/utils/options.ts b/scripts/utils/options.ts index 07ca42b633ca..4a73f002ba5b 100644 --- a/scripts/utils/options.ts +++ b/scripts/utils/options.ts @@ -137,7 +137,7 @@ export function getOptions( // Note the code above guarantees the types as they come in, so we cast here. // Not sure there is an easier way to do this - return command.opts() as MaybeOptionValue; + return command.opts() as MaybeOptionValues; } export function areOptionsSatisfied( From dd56d4a436ee4751250dcfe7c201e3934c720baf Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Wed, 27 Jul 2022 22:13:29 +1000 Subject: [PATCH 15/28] Fix typing problem --- scripts/utils/options.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/utils/options.ts b/scripts/utils/options.ts index 4a73f002ba5b..6df01d66dc37 100644 --- a/scripts/utils/options.ts +++ b/scripts/utils/options.ts @@ -197,7 +197,10 @@ function getFlag( } if (isStringArrayOption(option)) { - return value.map((v) => `--${longFlag(key, option)} ${v}`).join(' '); + // I'm not sure why TS isn't able to infer that OptionValue is a + // OptionValue (i.e. a string[]), given that it knows + // option is a StringArrayOption + return (value as string[]).map((v) => `--${longFlag(key, option)} ${v}`).join(' '); } if (isStringOption(option)) { From c928629c6a692c24724c6ddba147a023e10a9443 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Wed, 27 Jul 2022 22:17:39 +1000 Subject: [PATCH 16/28] Fix typing problems --- scripts/utils/cli-step.ts | 4 ++-- scripts/utils/options.test.ts | 7 +++++++ scripts/utils/options.ts | 6 +++--- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/scripts/utils/cli-step.ts b/scripts/utils/cli-step.ts index 52896ebcf3fb..574e116e0d24 100644 --- a/scripts/utils/cli-step.ts +++ b/scripts/utils/cli-step.ts @@ -18,7 +18,7 @@ export async function executeCLIStep( cliStep: CLIStep, options: { argument?: string; - optionValues: OptionValues; + optionValues?: Partial>; cwd: string; dryRun?: boolean; } @@ -30,7 +30,7 @@ export async function executeCLIStep( const command = getCommand( cliStep.hasArgument ? `${prefix} ${options.argument}` : prefix, cliStep.options, - options.optionValues + options.optionValues || {} ); await exec( diff --git a/scripts/utils/options.test.ts b/scripts/utils/options.test.ts index c181ca70d7fe..e2f9c1b55e0a 100644 --- a/scripts/utils/options.test.ts +++ b/scripts/utils/options.test.ts @@ -153,6 +153,13 @@ describe('getCommand', () => { ); }); + // This is for convenience + it('works with partial options', () => { + expect(getCommand('node foo', allOptions, { third: 'one' })).toBe( + 'node foo --no-second --third one' + ); + }); + it('works with combinations string options', () => { expect( getCommand('node foo', allOptions, { diff --git a/scripts/utils/options.ts b/scripts/utils/options.ts index 6df01d66dc37..76ce4256b91a 100644 --- a/scripts/utils/options.ts +++ b/scripts/utils/options.ts @@ -189,7 +189,7 @@ export async function promptOptions( function getFlag( key: OptionId, option: TOption, - value: OptionValue + value?: OptionValue ) { if (isBooleanOption(option)) { const toggled = option.inverse ? !value : value; @@ -200,7 +200,7 @@ function getFlag( // I'm not sure why TS isn't able to infer that OptionValue is a // OptionValue (i.e. a string[]), given that it knows // option is a StringArrayOption - return (value as string[]).map((v) => `--${longFlag(key, option)} ${v}`).join(' '); + return ((value || []) as string[]).map((v) => `--${longFlag(key, option)} ${v}`).join(' '); } if (isStringOption(option)) { @@ -216,7 +216,7 @@ function getFlag( export function getCommand( prefix: string, options: TOptions, - values: OptionValues + values: Partial> ) { const flags = Object.keys(options) .map((key) => getFlag(key, options[key], values[key])) From a8ce72cb0691358ac16b7618c43108a50a50c337 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 28 Jul 2022 19:46:40 +1000 Subject: [PATCH 17/28] Simple updates from @yannbf's comments --- scripts/example.ts | 30 ++++++++++++++++-------------- scripts/utils/options.test.ts | 7 +++++-- scripts/utils/options.ts | 4 +++- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/scripts/example.ts b/scripts/example.ts index 2854fa5d1b4d..419d86341c22 100644 --- a/scripts/example.ts +++ b/scripts/example.ts @@ -33,23 +33,24 @@ async function getOptions() { description: 'Always delete an existing example, even if it has the same configuration?', }, forceReuse: { - description: 'Always reusing an existing example, even if it has a different configuration?', + description: 'Always reuse an existing example, even if it has a different configuration?', }, - verdaccio: { - description: 'Use verdaccio rather than yarn linking stories?', + link: { + description: 'Link the storybook to the local code?', + inverse: true, }, start: { - description: 'Start the example app?', + description: 'Start the example Storybook?', inverse: true, }, build: { - description: 'Build the example app?', + description: 'Build the example Storybook?', }, watch: { - description: 'Start building used packages in watch mode as well as the example app?', + description: 'Start building used packages in watch mode as well as the example Storybook?', }, dryRun: { - description: "Don't execute commands, just list them?", + description: "Don't execute commands, just list them (dry run)?", }, }); } @@ -93,21 +94,22 @@ async function main() { const cwd = path.join(examplesDir, framework as string); const exists = await pathExists(cwd); - let shouldReuse = exists && forceReuse; + let shouldDelete = exists && !forceReuse; if (exists && !forceDelete && !forceReuse) { - ({ shouldReuse } = await prompts({ + const relativePath = path.relative(process.cwd(), cwd); + ({ shouldDelete } = await prompts({ type: 'toggle', - message: `${path.relative(process.cwd(), cwd)} already exists, should we reuse it?`, - name: 'shouldReuse', - initial: true, + message: `${relativePath} already exists, should delete it and create a new one?`, + name: 'shouldDelete', + initial: false, active: 'yes', inactive: 'no', })); } - if (exists && !shouldReuse) await remove(cwd); + if (exists && shouldDelete && !dryRun) await remove(cwd); - if (!shouldReuse) { + if (!exists || shouldDelete) { await executeCLIStep(steps.repro, { argument: cwd, optionValues: { template: framework }, diff --git a/scripts/utils/options.test.ts b/scripts/utils/options.test.ts index e2f9c1b55e0a..35a3d9f7851a 100644 --- a/scripts/utils/options.test.ts +++ b/scripts/utils/options.test.ts @@ -30,11 +30,14 @@ const allOptions = { }; // TS "tests" +// deepscan-disable-next-line function test(mv: MaybeOptionValues, v: OptionValues) { console.log(mv.first, mv.second, mv.third, mv.fourth); - // console.log(mv.fifth); // not allowed + // @ts-expect-error as it's not allowed + console.log(mv.fifth); console.log(v.first, v.second, v.third, v.fourth); - // console.log(v.fifth); // not allowed + // @ts-expect-error as it's not allowed + console.log(v.fifth); } describe('getOptions', () => { diff --git a/scripts/utils/options.ts b/scripts/utils/options.ts index 76ce4256b91a..741629876577 100644 --- a/scripts/utils/options.ts +++ b/scripts/utils/options.ts @@ -200,7 +200,9 @@ function getFlag( // I'm not sure why TS isn't able to infer that OptionValue is a // OptionValue (i.e. a string[]), given that it knows // option is a StringArrayOption - return ((value || []) as string[]).map((v) => `--${longFlag(key, option)} ${v}`).join(' '); + return ((value || []) as OptionValue) + .map((v) => `--${longFlag(key, option)} ${v}`) + .join(' '); } if (isStringOption(option)) { From 4f2dac0cee4006c35f6cf8c6ff21c81b013eb592 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 28 Jul 2022 20:15:57 +1000 Subject: [PATCH 18/28] Address issues with optional prompts --- scripts/example.ts | 9 +++++---- scripts/utils/options.ts | 40 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/scripts/example.ts b/scripts/example.ts index 419d86341c22..efa01c46622f 100644 --- a/scripts/example.ts +++ b/scripts/example.ts @@ -3,10 +3,9 @@ import { remove, pathExists } from 'fs-extra'; import prompts from 'prompts'; import { getOptionsOrPrompt } from './utils/options'; -import type { CLIStep } from './utils/cli-step'; import { executeCLIStep } from './utils/cli-step'; -const frameworks = ['react']; +const frameworks = ['react', 'angular']; const addons = ['a11y', 'storysource']; const examplesDir = path.resolve(__dirname, '../examples'); @@ -23,17 +22,19 @@ async function getOptions() { multiple: true, }, includeStories: { - description: - "Include Storybook's own stories (only applies if a react-based framework is used)?", + description: "Include Storybook's own stories?", + promptType: (_, { framework }) => framework === 'react', }, create: { description: 'Create the example from scratch (rather than degitting it)?', }, forceDelete: { description: 'Always delete an existing example, even if it has the same configuration?', + promptType: false, }, forceReuse: { description: 'Always reuse an existing example, even if it has a different configuration?', + promptType: false, }, link: { description: 'Link the storybook to the local code?', diff --git a/scripts/utils/options.ts b/scripts/utils/options.ts index 741629876577..3a0c6959be3c 100644 --- a/scripts/utils/options.ts +++ b/scripts/utils/options.ts @@ -2,7 +2,7 @@ * Use commander and prompts to gather a list of options for a script */ -import prompts from 'prompts'; +import prompts, { Falsy, PrevCaller, PromptType } from 'prompts'; import type { PromptObject } from 'prompts'; import program from 'commander'; import type { Command } from 'commander'; @@ -18,6 +18,10 @@ export type BaseOption = { * e.g. `shortFlag: 'c'` */ shortFlag?: string; + /** + * What type of prompt to use? (return false to skip, true for default) + */ + promptType?: PromptType | Falsy | PrevCaller; }; export type BooleanOption = BaseOption & { @@ -28,12 +32,24 @@ export type BooleanOption = BaseOption & { }; export type StringOption = BaseOption & { + /** + * What values are allowed for this option? + */ values: string[]; + /** + * Is a value required for this option? + */ required?: boolean; }; export type StringArrayOption = BaseOption & { + /** + * What values are allowed for this option? + */ values: string[]; + /** + * This must be set to true + */ multiple: true; }; @@ -61,7 +77,7 @@ export type MaybeOptionValues = { [TKey in keyof TOptions]: MaybeOptionValue; }; -export type OptionValues = { +export type OptionValues = { [TKey in keyof TOptions]: OptionValue; }; @@ -154,10 +170,26 @@ export async function promptOptions( values: MaybeOptionValues ): Promise> { const questions = Object.entries(options).map(([key, option]): PromptObject => { + let defaultType: PromptType = 'toggle'; + if (!isBooleanOption(option)) + defaultType = isStringArrayOption(option) ? 'autocompleteMultiselect' : 'select'; + + const passedType = option.type; + let type: PromptObject['type'] = defaultType; + // Allow returning `undefined` from `type()` function to fallback to default + if (typeof passedType === 'function') { + type = (...args: Parameters) => { + const chosenType = passedType(...args); + return chosenType === true ? defaultType : chosenType; + }; + } else if (passedType) { + type = passedType; + } + if (!isBooleanOption(option)) { const currentValue = values[key]; return { - type: isStringArrayOption(option) ? 'autocompleteMultiselect' : 'select', + type, message: option.description, name: key, // warn: ' ', @@ -172,7 +204,7 @@ export async function promptOptions( }; } return { - type: 'toggle', + type, message: option.description, name: key, initial: option.inverse, From 7a47b38fea528d18c8bb02e16e919fd245387f80 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 28 Jul 2022 20:30:06 +1000 Subject: [PATCH 19/28] I renamed it --- scripts/utils/options.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/utils/options.ts b/scripts/utils/options.ts index 3a0c6959be3c..ba35a25010e5 100644 --- a/scripts/utils/options.ts +++ b/scripts/utils/options.ts @@ -174,7 +174,7 @@ export async function promptOptions( if (!isBooleanOption(option)) defaultType = isStringArrayOption(option) ? 'autocompleteMultiselect' : 'select'; - const passedType = option.type; + const passedType = option.promptType; let type: PromptObject['type'] = defaultType; // Allow returning `undefined` from `type()` function to fallback to default if (typeof passedType === 'function') { From 4f27fd6b305f9d3b238151ed1cc8a14a21df2857 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 28 Jul 2022 20:30:40 +1000 Subject: [PATCH 20/28] Switch to executing `yarn storybook` in the project --- scripts/example.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/scripts/example.ts b/scripts/example.ts index efa01c46622f..6ad4c61eacf0 100644 --- a/scripts/example.ts +++ b/scripts/example.ts @@ -4,6 +4,7 @@ import prompts from 'prompts'; import { getOptionsOrPrompt } from './utils/options'; import { executeCLIStep } from './utils/cli-step'; +import { exec } from '../code/lib/cli/src/repro-generators/scripts'; const frameworks = ['react', 'angular']; const addons = ['a11y', 'storysource']; @@ -135,7 +136,15 @@ async function main() { const { start } = optionValues; if (start) { - await executeCLIStep(steps.dev, { cwd, dryRun }); + await exec( + 'yarn storybook', + { cwd }, + { + dryRun, + startMessage: `⬆️ Starting Storybook`, + errorMessage: `🚨 Starting Storybook failed`, + } + ); } else { await executeCLIStep(steps.build, { cwd, dryRun }); // TODO serve From 43d142b0840389c5f17ee096c47720f175980c8c Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 28 Jul 2022 21:21:46 +1000 Subject: [PATCH 21/28] Update trailing CSF version --- code/lib/blocks/package.json | 2 +- code/yarn.lock | 11 +---------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/code/lib/blocks/package.json b/code/lib/blocks/package.json index 43f7ce771d0a..8772767ba338 100644 --- a/code/lib/blocks/package.json +++ b/code/lib/blocks/package.json @@ -46,7 +46,7 @@ "@storybook/client-logger": "7.0.0-alpha.17", "@storybook/components": "7.0.0-alpha.17", "@storybook/core-events": "7.0.0-alpha.17", - "@storybook/csf": "0.0.2--canary.7c6c115.0", + "@storybook/csf": "0.0.2--canary.0899bb7.0", "@storybook/docs-tools": "7.0.0-alpha.17", "@storybook/preview-web": "7.0.0-alpha.17", "@storybook/store": "7.0.0-alpha.17", diff --git a/code/yarn.lock b/code/yarn.lock index e41c5bdd9df7..940aa44590bf 100644 --- a/code/yarn.lock +++ b/code/yarn.lock @@ -7522,7 +7522,7 @@ __metadata: "@storybook/client-logger": 7.0.0-alpha.17 "@storybook/components": 7.0.0-alpha.17 "@storybook/core-events": 7.0.0-alpha.17 - "@storybook/csf": 0.0.2--canary.7c6c115.0 + "@storybook/csf": 0.0.2--canary.0899bb7.0 "@storybook/docs-tools": 7.0.0-alpha.17 "@storybook/preview-web": 7.0.0-alpha.17 "@storybook/store": 7.0.0-alpha.17 @@ -8076,15 +8076,6 @@ __metadata: languageName: node linkType: hard -"@storybook/csf@npm:0.0.2--canary.7c6c115.0": - version: 0.0.2--canary.7c6c115.0 - resolution: "@storybook/csf@npm:0.0.2--canary.7c6c115.0" - dependencies: - lodash: ^4.17.15 - checksum: 85a179664d18eeca8462c1b6ff36f9b68b856c9f9c5143aa6f19b17e4cc97bc08ed69921a5287a61d8c90f61366ff6a5ab89930d158402e7c04d07a3ffaad8bb - languageName: node - linkType: hard - "@storybook/csf@npm:^0.0.1": version: 0.0.1 resolution: "@storybook/csf@npm:0.0.1" From 9bd059a33576d66550defe19d4e000e6c4f4c74e Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 28 Jul 2022 21:21:58 +1000 Subject: [PATCH 22/28] Got linking going --- scripts/example.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/scripts/example.ts b/scripts/example.ts index 6ad4c61eacf0..250c489053b4 100644 --- a/scripts/example.ts +++ b/scripts/example.ts @@ -9,6 +9,7 @@ import { exec } from '../code/lib/cli/src/repro-generators/scripts'; const frameworks = ['react', 'angular']; const addons = ['a11y', 'storysource']; const examplesDir = path.resolve(__dirname, '../examples'); +const codeDir = path.resolve(__dirname, '../code'); async function getOptions() { return getOptionsOrPrompt('yarn example', { @@ -75,6 +76,13 @@ const steps = { hasArgument: true, options: {}, }, + link: { + command: 'link', + description: 'Linking packages', + icon: '🔗', + hasArgument: true, + options: { local: {} }, + }, build: { command: 'build', description: 'Building example', @@ -92,7 +100,7 @@ const steps = { async function main() { const optionValues = await getOptions(); - const { framework, forceDelete, forceReuse, dryRun } = optionValues; + const { framework, forceDelete, forceReuse, link, dryRun } = optionValues; const cwd = path.join(examplesDir, framework as string); const exists = await pathExists(cwd); @@ -132,6 +140,15 @@ async function main() { } // TODO copy stories + + if (link) { + await executeCLIStep(steps.link, { + argument: cwd, + cwd: codeDir, + dryRun, + optionValues: { local: true }, + }); + } } const { start } = optionValues; From ce461d696e28e756c1e77d145cdd2f6773fc71a9 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Fri, 29 Jul 2022 14:06:21 +1000 Subject: [PATCH 23/28] Drop unused var --- scripts/utils/cli-step.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/scripts/utils/cli-step.ts b/scripts/utils/cli-step.ts index 574e116e0d24..bff96f6436fe 100644 --- a/scripts/utils/cli-step.ts +++ b/scripts/utils/cli-step.ts @@ -1,8 +1,6 @@ import { getCommand, OptionSpecifier, OptionValues } from './options'; import { exec } from '../../code/lib/cli/src/repro-generators/scripts'; -const logger = console; - const cliExecutable = require.resolve('../../code/lib/cli/bin/index.js'); export type CLIStep = { From 457714dd06a3201fc7d792bcfc986555b9a3a0ab Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Fri, 29 Jul 2022 16:13:29 +1000 Subject: [PATCH 24/28] Add examples to gitignore --- .gitignore | 3 ++- examples/react | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) delete mode 160000 examples/react diff --git a/.gitignore b/.gitignore index 26cd055c6108..4ae2a283700c 100644 --- a/.gitignore +++ b/.gitignore @@ -16,4 +16,5 @@ junit.xml !/**/.yarn/sdks !/**/.yarn/versions /**/.pnp.* -/yarn.lock \ No newline at end of file +/yarn.lock +./examples/ \ No newline at end of file diff --git a/examples/react b/examples/react deleted file mode 160000 index b7ef5bd9f548..000000000000 --- a/examples/react +++ /dev/null @@ -1 +0,0 @@ -Subproject commit b7ef5bd9f548330f7a0c0b0f6b8d285bcd12a6f7 From 5a37d8de3d737247a491321877fda964e2f4701a Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Sun, 31 Jul 2022 13:26:09 +1000 Subject: [PATCH 25/28] Get rid of any casts, thanks @kasperpeulen! --- scripts/utils/options.test.ts | 29 +++++++++-------------------- scripts/utils/options.ts | 3 +-- 2 files changed, 10 insertions(+), 22 deletions(-) diff --git a/scripts/utils/options.test.ts b/scripts/utils/options.test.ts index 35a3d9f7851a..d2fa6c504839 100644 --- a/scripts/utils/options.test.ts +++ b/scripts/utils/options.test.ts @@ -42,9 +42,7 @@ function test(mv: MaybeOptionValues, v: OptionValues { it('deals with boolean options', () => { - expect( - getOptions(createCommand() as any, allOptions, ['command', 'name', '--first']) - ).toMatchObject({ + expect(getOptions(createCommand(), allOptions, ['command', 'name', '--first'])).toMatchObject({ first: true, second: true, }); @@ -52,7 +50,7 @@ describe('getOptions', () => { it('deals with inverse boolean options', () => { expect( - getOptions(createCommand() as any, allOptions, ['command', 'name', '--no-second']) + getOptions(createCommand(), allOptions, ['command', 'name', '--no-second']) ).toMatchObject({ first: false, second: false, @@ -60,18 +58,16 @@ describe('getOptions', () => { }); it('deals with short options', () => { - expect( - getOptions(createCommand() as any, allOptions, ['command', 'name', '-f', '-S']) - ).toMatchObject({ + expect(getOptions(createCommand(), allOptions, ['command', 'name', '-f', '-S'])).toMatchObject({ first: true, second: false, }); }); it('deals with string options', () => { - const r = getOptions(createCommand() as any, allOptions, ['command', 'name', '--third', 'one']); + const r = getOptions(createCommand(), allOptions, ['command', 'name', '--third', 'one']); expect( - getOptions(createCommand() as any, allOptions, ['command', 'name', '--third', 'one']) + getOptions(createCommand(), allOptions, ['command', 'name', '--third', 'one']) ).toMatchObject({ third: 'one', }); @@ -79,26 +75,19 @@ describe('getOptions', () => { it('disallows invalid string options', () => { expect(() => - getOptions(createCommand() as any, allOptions, ['command', 'name', '--third', 'random']) + getOptions(createCommand(), allOptions, ['command', 'name', '--third', 'random']) ).toThrow(/Unexpected value/); }); it('deals with multiple string options', () => { expect( - getOptions(createCommand() as any, allOptions, ['command', 'name', '--fourth', 'a']) + getOptions(createCommand(), allOptions, ['command', 'name', '--fourth', 'a']) ).toMatchObject({ fourth: ['a'], }); expect( - getOptions(createCommand() as any, allOptions, [ - 'command', - 'name', - '--fourth', - 'a', - '--fourth', - 'b', - ]) + getOptions(createCommand(), allOptions, ['command', 'name', '--fourth', 'a', '--fourth', 'b']) ).toMatchObject({ fourth: ['a', 'b'], }); @@ -106,7 +95,7 @@ describe('getOptions', () => { it('disallows invalid multiple string options', () => { expect(() => - getOptions(createCommand() as any, allOptions, ['command', 'name', '--fourth', 'random']) + getOptions(createCommand(), allOptions, ['command', 'name', '--fourth', 'random']) ).toThrow(/Unexpected value/); }); }); diff --git a/scripts/utils/options.ts b/scripts/utils/options.ts index ba35a25010e5..f8cf9b69ff0c 100644 --- a/scripts/utils/options.ts +++ b/scripts/utils/options.ts @@ -5,7 +5,6 @@ import prompts, { Falsy, PrevCaller, PromptType } from 'prompts'; import type { PromptObject } from 'prompts'; import program from 'commander'; -import type { Command } from 'commander'; import kebabCase from 'lodash/kebabCase'; // Option types @@ -119,7 +118,7 @@ function optionFlags(key: OptionId, option: Option) { } export function getOptions( - command: Command, + command: program.Command, options: TOptions, argv: string[] ): MaybeOptionValues { From 9d831be524ef3bc51572aea3b3ec33ae0b1e4510 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Sun, 31 Jul 2022 13:27:11 +1000 Subject: [PATCH 26/28] Use `as const` on booleans rather than weak matching --- scripts/utils/options.test.ts | 4 ++-- scripts/utils/options.ts | 8 ++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/scripts/utils/options.test.ts b/scripts/utils/options.test.ts index d2fa6c504839..c73cb1ac5a74 100644 --- a/scripts/utils/options.test.ts +++ b/scripts/utils/options.test.ts @@ -20,12 +20,12 @@ const allOptions = { third: { description: 'third', values: ['one', 'two', 'three'], - required: true, + required: true as const, }, fourth: { description: 'fourth', values: ['a', 'b', 'c'], - multiple: true, + multiple: true as const, }, }; diff --git a/scripts/utils/options.ts b/scripts/utils/options.ts index f8cf9b69ff0c..2496a0500515 100644 --- a/scripts/utils/options.ts +++ b/scripts/utils/options.ts @@ -52,9 +52,7 @@ export type StringArrayOption = BaseOption & { multiple: true; }; -// StringArrayOption requires `multiple: true;` but unless you use `as const` an object with -// { multiple: true } will be inferred as `multiple: boolean;` -type StringArrayOptionMatch = Omit & { multiple: boolean }; +type StringArrayOptionMatch = Omit & { multiple: true }; export type Option = BooleanOption | StringOption | StringArrayOption; export type MaybeOptionValue = TOption extends StringArrayOptionMatch @@ -65,9 +63,7 @@ export type MaybeOptionValue = TOption extends StringArr ? boolean : never; -// Note we use `required: boolean;` rather than `required: true` here for the same reason -// as `StringArrayOptionMatch` above. In both cases, the field should only ever be set to true -export type OptionValue = TOption extends { required: boolean } +export type OptionValue = TOption extends { required: true } ? string : MaybeOptionValue; From 99d6c5282bd4e357994831847e8b40fffec20f4e Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Sun, 31 Jul 2022 14:26:45 +1000 Subject: [PATCH 27/28] Allow running link without starting --- code/lib/cli/src/generate.ts | 5 +++-- code/lib/cli/src/link.ts | 9 ++++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/code/lib/cli/src/generate.ts b/code/lib/cli/src/generate.ts index d94e4517c432..bdfe9c12a883 100644 --- a/code/lib/cli/src/generate.ts +++ b/code/lib/cli/src/generate.ts @@ -150,8 +150,9 @@ program .command('link ') .description('Pull down a repro from a URL (or a local directory), link it, and run storybook') .option('--local', 'Link a local directory already in your file system') - .action((target, { local }) => - link({ target, local }).catch((e) => { + .option('--no-start', 'Start the storybook', true) + .action((target, { local, start }) => + link({ target, local, start }).catch((e) => { logger.error(e); process.exit(1); }) diff --git a/code/lib/cli/src/link.ts b/code/lib/cli/src/link.ts index 9ceff0afbb7b..ce3e419dcf98 100644 --- a/code/lib/cli/src/link.ts +++ b/code/lib/cli/src/link.ts @@ -7,9 +7,10 @@ import { exec } from './repro-generators/scripts'; interface LinkOptions { target: string; local?: boolean; + start: boolean; } -export const link = async ({ target, local }: LinkOptions) => { +export const link = async ({ target, local, start }: LinkOptions) => { const storybookDir = process.cwd(); try { const packageJson = JSON.parse(fse.readFileSync('package.json', 'utf8')); @@ -58,6 +59,8 @@ export const link = async ({ target, local }: LinkOptions) => { ); await exec(`yarn add -D webpack-hot-middleware`, { cwd: reproDir }); - logger.info(`Running ${reproName} storybook`); - await exec(`yarn run storybook`, { cwd: reproDir }); + if (start) { + logger.info(`Running ${reproName} storybook`); + await exec(`yarn run storybook`, { cwd: reproDir }); + } }; From 93f3e0e5b5e7177440ebae3d514147fe83d50c6d Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Sun, 31 Jul 2022 14:27:09 +1000 Subject: [PATCH 28/28] Get react18 example working via preserve missing --- scripts/example.ts | 72 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 67 insertions(+), 5 deletions(-) diff --git a/scripts/example.ts b/scripts/example.ts index 250c489053b4..27f83b1d751b 100644 --- a/scripts/example.ts +++ b/scripts/example.ts @@ -1,10 +1,14 @@ import path from 'path'; -import { remove, pathExists } from 'fs-extra'; +import { remove, pathExists, readJSON, writeJSON } from 'fs-extra'; import prompts from 'prompts'; import { getOptionsOrPrompt } from './utils/options'; import { executeCLIStep } from './utils/cli-step'; import { exec } from '../code/lib/cli/src/repro-generators/scripts'; +import type { Parameters } from '../code/lib/cli/src/repro-generators/configs'; +import { getInterpretedFile } from '../code/lib/core-common'; +import { readConfig, writeConfig } from '../code/lib/csf-tools'; +import { babelParse } from '../code/lib/csf-tools/src/babelParse'; const frameworks = ['react', 'angular']; const addons = ['a11y', 'storysource']; @@ -16,12 +20,12 @@ async function getOptions() { framework: { description: 'Which framework would you like to use?', values: frameworks, - required: true, + required: true as const, }, addon: { description: 'Which extra addons (beyond the CLI defaults) would you like installed?', values: addons, - multiple: true, + multiple: true as const, }, includeStories: { description: "Include Storybook's own stories?", @@ -81,7 +85,7 @@ const steps = { description: 'Linking packages', icon: '🔗', hasArgument: true, - options: { local: {} }, + options: { local: {}, start: { inverse: true } }, }, build: { command: 'build', @@ -97,6 +101,46 @@ const steps = { }, }; +const logger = console; + +export const overrideMainConfig = async ({ + cwd, + mainOverrides, +}: { + cwd: string; + mainOverrides: Parameters['mainOverrides']; +}) => { + logger.info(`📝 Overwriting main.js with the following configuration:`); + const configDir = path.join(cwd, '.storybook'); + const mainConfigPath = getInterpretedFile(path.resolve(configDir, 'main')); + logger.debug(mainOverrides); + const mainConfig = await readConfig(mainConfigPath); + + Object.keys(mainOverrides).forEach((field) => { + // NOTE: using setFieldNode and passing the output of babelParse() + mainConfig.setFieldNode([field], mainOverrides[field]); + }); + + await writeConfig(mainConfig); +}; + +const addPackageScripts = async ({ + cwd, + scripts, +}: { + cwd: string; + scripts: Record; +}) => { + logger.info(`🔢 Adding package resolutions:`); + const packageJsonPath = path.join(cwd, 'package.json'); + const packageJson = await readJSON(packageJsonPath); + packageJson.scripts = { + ...packageJson.scripts, + ...scripts, + }; + await writeJSON(packageJsonPath, packageJson, { spaces: 2 }); +}; + async function main() { const optionValues = await getOptions(); @@ -146,7 +190,25 @@ async function main() { argument: cwd, cwd: codeDir, dryRun, - optionValues: { local: true }, + optionValues: { local: true, start: false }, + }); + + // TODO -- work out exactly where this should happen + const code = '(c) => ({ ...c, resolve: { ...c.resolve, symlinks: false } })'; + const mainOverrides = { + // @ts-ignore (not sure why TS complains here, it does exist) + webpackFinal: babelParse(code).program.body[0].expression, + }; + await overrideMainConfig({ cwd, mainOverrides } as any); + + await addPackageScripts({ + cwd, + scripts: { + storybook: + 'NODE_OPTIONS="--preserve-symlinks --preserve-symlinks-main" storybook dev -p 6006', + 'build-storybook': + 'NODE_OPTIONS="--preserve-symlinks --preserve-symlinks-main" storybook build', + }, }); } }