Merge pull request #20067 from storybookjs/fix/in-development-flag

Build: improve inDevelopment mode for yarn task

code/lib/cli/src/generate.ts

@@ -25,14 +25,15 @@ import { parseList, getEnvConfig } from './utils';
 const pkg = readUpSync({ cwd: __dirname }).packageJson;
 const consoleLogger = console;
 
-program.option(
-  '--disable-telemetry',
-  'disable sending telemetry data',
-  // default value is false, but if the user sets STORYBOOK_DISABLE_TELEMETRY, it can be true
-  process.env.STORYBOOK_DISABLE_TELEMETRY && process.env.STORYBOOK_DISABLE_TELEMETRY !== 'false'
-);
-
-program.option('--enable-crash-reports', 'enable sending crash reports to telemetry data');
+program
+  .option(
+    '--disable-telemetry',
+    'disable sending telemetry data',
+    // default value is false, but if the user sets STORYBOOK_DISABLE_TELEMETRY, it can be true
+    process.env.STORYBOOK_DISABLE_TELEMETRY && process.env.STORYBOOK_DISABLE_TELEMETRY !== 'false'
+  )
+  .option('--debug', 'Get more logs in debug mode', false)
+  .option('--enable-crash-reports', 'enable sending crash reports to telemetry data');
 
 program
   .command('init')

code/lib/cli/src/repro-templates.ts

@@ -1,4 +1,39 @@
-export const allTemplates = {
+export type SkippableTask = 'smoke-test' | 'test-runner' | 'chromatic' | 'e2e-tests';
+export type TemplateKey = keyof typeof allTemplates;
+export type Cadence = keyof typeof templatesByCadence;
+export type Template = {
+  /**
+   * Readable name for the template, which will be used for feedback and the status page
+   */
+  name: string;
+  /**
+   * Script used to generate the base project of a template.
+   * The Storybook CLI will then initialize Storybook on top of that template.
+   * This is used to generate projects which are pushed to https://github.com/storybookjs/repro-templates-temp
+   */
+  script: string;
+  /**
+   * Used to assert various things about the generated template.
+   * If the template is generated with a different expected framework, it will fail, detecting a possible regression.
+   */
+  expected: {
+    framework: string;
+    renderer: string;
+    builder: string;
+  };
+  /**
+   * Some sandboxes might not work properly in specific tasks temporarily, but we might
+   * still want to run the other tasks. Set the ones to skip in this property.
+   */
+  skipTasks?: SkippableTask[];
+  /**
+   * Set this only while developing a newly created framework, to avoid using it in CI.
+   * NOTE: Make sure to always add a TODO comment to remove this flag in a subsequent PR.
+   */
+  inDevelopment?: boolean;
+};
+
+export const allTemplates: Record<string, Template> = {
   'cra/default-js': {
     name: 'Create React App (Javascript)',
     script: 'npx create-react-app .',
@@ -33,7 +68,7 @@ export const allTemplates = {
   },
   'nextjs/default-js': {
     name: 'Next.js (JavaScript)',
-    script: 'yarn create next-app {{beforeDir}}',
+    script: 'yarn create next-app {{beforeDir}} --javascript --eslint',
     expected: {
       framework: '@storybook/nextjs',
       renderer: '@storybook/react',
@@ -42,7 +77,7 @@ export const allTemplates = {
   },
   'nextjs/default-ts': {
     name: 'Next.js (TypeScript)',
-    script: 'yarn create next-app {{beforeDir}} --typescript',
+    script: 'yarn create next-app {{beforeDir}} --typescript --eslint',
     expected: {
       framework: '@storybook/nextjs',
       renderer: '@storybook/react',
@@ -266,8 +301,6 @@ export const allTemplates = {
   },
 };
 
-type TemplateKey = keyof typeof allTemplates;
-
 export const ci: TemplateKey[] = ['cra/default-ts', 'react-vite/default-ts'];
 export const pr: TemplateKey[] = [
   ...ci,

docs/contribute/code.md

@@ -216,23 +216,25 @@ The **`key`** `cra/default-js` consists of two parts:
 - The prefix is the tool that was used to generate the repro app
 - The suffix is options that modify the default install, e.g. a specific version or options
 
-The **`script`** field is what generates the application environment. The `.` argument is “the current working directory” which is auto-generated based on the key (e.g. `repros/cra/default-js/before-storybook`).
+The **`script`** field is what generates the application environment. The `.` argument is “the current working directory” which is auto-generated based on the key (e.g. `repros/cra/default-js/before-storybook`). The `{{beforeDir}}` key can also be used, which will be replaced by the path of that directory.
 
 The rest of the fields are self-explanatory:
 
-- **`name`**: Human readable name/description
-- **`cadence`:** How often this runs in CI (for now these are the same for every template)
-- **`expected`**: What framework/renderer/builder we expect `sb init` to generate
+The **`skipTasks`** field exists because some sandboxes might not work properly in specific tasks temporarily, but we might still want to run the other tasks. For instance, a bug was introduced outside of our control, which fails only in the `test-runner` task.
+
+The **`name`** field should contain a human readable name/description of the template.
+
+The **`expected`** field reflects what framework/renderer/builder we expect `sb init` to generate. This is useful for assertions while generating sandboxes. If the template is generated with a different expected framework, for instance, it will fail, serving as a way to detect regressions.
 
 ### Running a sandbox
 
-If your template has a `inDevelopment` flag, it will be generated (locally) as part of the sandbox process. You can create the sandbox with:
+If your template has a `inDevelopment` flag, it will be generated (locally) as part of the sandbox process. You can create the sandbox with the following command, where `<template-key>` is replaced by the id of the selected template e.g. `cra/default-js`:
 
 ```bash
-yarn task --task dev --template cra/default-js --no-link --start-from=install
+yarn task --task dev --template <template-key> --start-from=install
 ```
 
-Make sure you pass the `--no-link` flag as it is required for the local template generation to work.
+Templates with `inDevelopment` will automatically run with `--no-link` flag as it is required for the local template generation to work.
 
 Once the PR is merged, the template will be generated on a nightly cadence and you can remove the `inDevelopment` flag and the sandbox will pull the code from our templates repository.
 

scripts/get-template.ts

@@ -1,16 +1,17 @@
 import { readdir } from 'fs/promises';
 import { pathExists } from 'fs-extra';
 import { resolve } from 'path';
-import { allTemplates, templatesByCadence } from '../code/lib/cli/src/repro-templates';
+import {
+  allTemplates,
+  templatesByCadence,
+  type Cadence,
+  type Template as TTemplate,
+  type SkippableTask,
+} from '../code/lib/cli/src/repro-templates';
 
 const sandboxDir = process.env.SANDBOX_ROOT || resolve(__dirname, '../sandbox');
 
-export type Cadence = keyof typeof templatesByCadence;
-export type Template = {
-  cadence?: readonly Cadence[];
-  skipTasks?: string[];
-  // there are other fields but we don't use them here
-};
+type Template = Pick<TTemplate, 'inDevelopment' | 'skipTasks'>;
 export type TemplateKey = keyof typeof allTemplates;
 export type Templates = Record<TemplateKey, Template>;
 
@@ -44,9 +45,13 @@ export async function getTemplate(
     potentialTemplateKeys = cadenceTemplates.map(([k]) => k) as TemplateKey[];
   }
 
-  potentialTemplateKeys = potentialTemplateKeys.filter(
-    (t) => !(allTemplates[t] as Template).skipTasks?.includes(scriptName)
-  );
+  potentialTemplateKeys = potentialTemplateKeys.filter((t) => {
+    const currentTemplate = allTemplates[t] as Template;
+    return (
+      currentTemplate.inDevelopment !== true &&
+      !currentTemplate.skipTasks?.includes(scriptName as SkippableTask)
+    );
+  });
 
   if (potentialTemplateKeys.length !== total) {
     throw new Error(`Circle parallelism set incorrectly.

scripts/next-repro-generators/generate-repros.ts

@@ -27,12 +27,12 @@ const BEFORE_DIR_NAME = 'before-storybook';
 const AFTER_DIR_NAME = 'after-storybook';
 const SCRIPT_TIMEOUT = 5 * 60 * 1000;
 
-const sbInit = async (cwd: string, flags?: string[]) => {
+const sbInit = async (cwd: string, flags?: string[], debug?: boolean) => {
   const sbCliBinaryPath = join(__dirname, `../../code/lib/cli/bin/index.js`);
   console.log(`🎁 Installing storybook`);
   const env = { STORYBOOK_DISABLE_TELEMETRY: 'true', STORYBOOK_REPRO_GENERATOR: 'true' };
   const fullFlags = ['--yes', ...(flags || [])];
-  await runCommand(`${sbCliBinaryPath} init ${fullFlags.join(' ')}`, { cwd, env });
+  await runCommand(`${sbCliBinaryPath} init ${fullFlags.join(' ')}`, { cwd, env }, debug);
 };
 
 const LOCAL_REGISTRY_URL = 'http://localhost:6001';
@@ -56,7 +56,17 @@ const withLocalRegistry = async (packageManager: JsPackageManager, action: () =>
   }
 };
 
-const addStorybook = async (baseDir: string, localRegistry: boolean, flags?: string[]) => {
+const addStorybook = async ({
+  baseDir,
+  localRegistry,
+  flags,
+  debug,
+}: {
+  baseDir: string;
+  localRegistry: boolean;
+  flags?: string[];
+  debug?: boolean;
+}) => {
   const beforeDir = join(baseDir, BEFORE_DIR_NAME);
   const afterDir = join(baseDir, AFTER_DIR_NAME);
   const tmpDir = join(baseDir, 'tmp');
@@ -71,23 +81,21 @@ const addStorybook = async (baseDir: string, localRegistry: boolean, flags?: str
     await withLocalRegistry(packageManager, async () => {
       packageManager.addPackageResolutions(storybookVersions);
 
-      await sbInit(tmpDir, flags);
+      await sbInit(tmpDir, flags, debug);
     });
   } else {
-    await sbInit(tmpDir, flags);
+    await sbInit(tmpDir, flags, debug);
   }
   await rename(tmpDir, afterDir);
 };
 
-export const runCommand = async (script: string, options: ExecaOptions) => {
-  const shouldDebug = !!process.env.DEBUG;
-
-  if (shouldDebug) {
+export const runCommand = async (script: string, options: ExecaOptions, debug: boolean) => {
+  if (debug) {
     console.log(`Running command: ${script}`);
   }
 
   return execaCommand(script, {
-    stdout: shouldDebug ? 'inherit' : 'ignore',
+    stdout: debug ? 'inherit' : 'ignore',
     shell: true,
     ...options,
   });
@@ -113,7 +121,8 @@ const addDocumentation = async (
 
 const runGenerators = async (
   generators: (GeneratorConfig & { dirName: string })[],
-  localRegistry = true
+  localRegistry = true,
+  debug = false
 ) => {
   console.log(`🤹‍♂️ Generating repros with a concurrency of ${maxConcurrentTasks}`);
 
@@ -142,10 +151,17 @@ const runGenerators = async (
         // handle different modes of operation.
         if (script.includes('{{beforeDir}}')) {
           const scriptWithBeforeDir = script.replace('{{beforeDir}}', BEFORE_DIR_NAME);
-          await runCommand(scriptWithBeforeDir, { cwd: createBaseDir, timeout: SCRIPT_TIMEOUT });
+          await runCommand(
+            scriptWithBeforeDir,
+            {
+              cwd: createBaseDir,
+              timeout: SCRIPT_TIMEOUT,
+            },
+            debug
+          );
         } else {
           await ensureDir(createBeforeDir);
-          await runCommand(script, { cwd: createBeforeDir, timeout: SCRIPT_TIMEOUT });
+          await runCommand(script, { cwd: createBeforeDir, timeout: SCRIPT_TIMEOUT }, debug);
         }
 
         await localizeYarnConfigFiles(createBaseDir, createBeforeDir);
@@ -156,7 +172,7 @@ const runGenerators = async (
         // Make sure there are no git projects in the folder
         await remove(join(beforeDir, '.git'));
 
-        await addStorybook(baseDir, localRegistry, flags);
+        await addStorybook({ baseDir, localRegistry, flags, debug });
 
         await addDocumentation(baseDir, { name, dirName });
 
@@ -191,9 +207,18 @@ export const options = createOptions({
     description: 'Generate reproduction from local registry?',
     promptType: false,
   },
+  debug: {
+    type: 'boolean',
+    description: 'Print all the logs to the console',
+    promptType: false,
+  },
 });
 
-export const generate = async ({ template, localRegistry }: OptionValues<typeof options>) => {
+export const generate = async ({
+  template,
+  localRegistry,
+  debug,
+}: OptionValues<typeof options>) => {
   const generatorConfigs = Object.entries(reproTemplates)
     .map(([dirName, configuration]) => ({
       dirName,
@@ -207,13 +232,14 @@ export const generate = async ({ template, localRegistry }: OptionValues<typeof
       return true;
     });
 
-  await runGenerators(generatorConfigs, localRegistry);
+  await runGenerators(generatorConfigs, localRegistry, debug);
 };
 
 if (require.main === module) {
   program
     .description('Create a reproduction from a set of possible templates')
-    .option('--template <template>', 'Create a single template') // change this to allow multiple templates or regex
+    .option('--template <template>', 'Create a single template')
+    .option('--debug', 'Print all the logs to the console')
     .option('--local-registry', 'Use local registry', false)
     .action((optionValues) => {
       generate(optionValues)

scripts/task.ts

@@ -23,16 +23,18 @@ import { testRunner } from './tasks/test-runner';
 import { chromatic } from './tasks/chromatic';
 import { e2eTests } from './tasks/e2e-tests';
 
-import { allTemplates as TEMPLATES } from '../code/lib/cli/src/repro-templates';
+import {
+  allTemplates as TEMPLATES,
+  type TemplateKey,
+  type Template,
+} from '../code/lib/cli/src/repro-templates';
 
 const sandboxDir = process.env.SANDBOX_ROOT || resolve(__dirname, '../sandbox');
 const codeDir = resolve(__dirname, '../code');
 const junitDir = resolve(__dirname, '../test-results');
 
 export const extraAddons = ['a11y', 'storysource'];
 
-export type TemplateKey = keyof typeof TEMPLATES;
-export type Template = typeof TEMPLATES[TemplateKey];
 export type Path = string;
 export type TemplateDetails = {
   key: TemplateKey;

scripts/tasks/sandbox-parts.ts

@@ -53,7 +53,7 @@ export const create: Task['run'] = async (
   } else {
     await executeCLIStep(steps.repro, {
       argument: key,
-      optionValues: { output: sandboxDir, branch: 'next' },
+      optionValues: { output: sandboxDir, branch: 'next', debug },
       cwd: parentDir,
       dryRun,
       debug,

scripts/tasks/sandbox.ts

@@ -8,8 +8,6 @@ export const sandbox: Task = {
   description: 'Create the sandbox from a template',
   dependsOn: ({ template }, { link }) => {
     if ('inDevelopment' in template && template.inDevelopment) {
-      if (link) throw new Error('Cannot link an in development template');
-
       return ['run-registry', 'generate'];
     }
 
@@ -20,6 +18,13 @@ export const sandbox: Task = {
     return pathExists(sandboxDir);
   },
   async run(details, options) {
+    if (options.link && details.template.inDevelopment) {
+      logger.log(
+        `The ${options.template} has inDevelopment property enabled, therefore the sandbox for that template cannot be linked. Enabling --no-link mode..`
+      );
+      // eslint-disable-next-line no-param-reassign
+      options.link = false;
+    }
     if (await this.ready(details)) {
       logger.info('🗑  Removing old sandbox dir');
       await remove(details.sandboxDir);

scripts/utils/cli-step.ts

@@ -23,6 +23,7 @@ export const steps = {
       output: { type: 'string' },
       // TODO allow default values for strings
       branch: { type: 'string', values: ['next'] },
+      debug: { type: 'boolean' },
     }),
   },
   add: {