Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

CLI: Add index command / API #30071

Open
wants to merge 6 commits into
base: next
Choose a base branch
from
Open

CLI: Add index command / API #30071

wants to merge 6 commits into from

Conversation

shilman
Copy link
Member

@shilman shilman commented Dec 16, 2024

Closes #

What I did

  • Add new storybook index command that creates index.json without building the storybook
  • Add new buildIndex API to do the same thing as a Node API
  • Documentation & integration test

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

Run storybook index on a project and verify that index.json matches storybook build's storybook-static/index.json

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

🦋 Canary release

This pull request has been released as version 0.0.0-pr-30071-sha-26d114f9. Try it out in a new sandbox by running npx [email protected] sandbox or in an existing project with npx [email protected] upgrade.

More information
Published version 0.0.0-pr-30071-sha-26d114f9
Triggered by @shilman
Repository storybookjs/storybook
Branch shilman/build-index
Commit 26d114f9
Datetime Sun Feb 23 14:33:38 UTC 2025 (1740321218)
Workflow run 13483993611

To request a new release of this pull request, mention the @storybookjs/core team.

core team members can create a new canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=30071

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 80.6 MB 80.6 MB 0 B 1.81 0%
initSize 80.6 MB 80.6 MB 0 B 1.81 0%
diffSize 97 B 97 B 0 B - 0%
buildSize 7.31 MB 7.31 MB 0 B 0.68 0%
buildSbAddonsSize 1.9 MB 1.9 MB 0 B -0.26 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.88 MB 1.88 MB 0 B 0.5 0%
buildSbPreviewSize 0 B 0 B 0 B - -
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.97 MB 3.97 MB 0 B 0.62 0%
buildPreviewSize 3.34 MB 3.34 MB 0 B 0.62 0%
testBuildSize 0 B 0 B 0 B - -
testBuildSbAddonsSize 0 B 0 B 0 B - -
testBuildSbCommonSize 0 B 0 B 0 B - -
testBuildSbManagerSize 0 B 0 B 0 B - -
testBuildSbPreviewSize 0 B 0 B 0 B - -
testBuildStaticSize 0 B 0 B 0 B - -
testBuildPrebuildSize 0 B 0 B 0 B - -
testBuildPreviewSize 0 B 0 B 0 B - -
name before after diff z %
createTime 6.6s 15.1s 8.4s 0.54 55.8%
generateTime 17s 19s 1.9s -0.55 10.1%
initTime 4.2s 4.2s -33ms -1.14 -0.8%
buildTime 9.1s 8.5s -546ms -0.82 -6.4%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 5s 4.6s -444ms -0.88 -9.6%
devManagerResponsive 4.8s 4.4s -412ms -0.48 -9.3%
devManagerHeaderVisible 776ms 794ms 18ms 0.14 2.3%
devManagerIndexVisible 789ms 807ms 18ms -0.06 2.2%
devStoryVisibleUncached 1.7s 1.8s 130ms -0.49 6.9%
devStoryVisible 816ms 830ms 14ms -0.07 1.7%
devAutodocsVisible 681ms 881ms 200ms 1.24 🔺22.7%
devMDXVisible 761ms 795ms 34ms 0.61 4.3%
buildManagerHeaderVisible 876ms 707ms -169ms -0.5 -23.9%
buildManagerIndexVisible 888ms 710ms -178ms -0.65 -25.1%
buildStoryVisible 830ms 686ms -144ms -0.45 -21%
buildAutodocsVisible 758ms 646ms -112ms 0.3 -17.3%
buildMDXVisible 563ms 500ms -63ms -1.13 -12.6%

Greptile Summary

Introduces a new storybook index command and buildIndex API that generates an index.json file containing story metadata without building the full Storybook, improving build efficiency for specific use cases.

  • Added code/core/src/cli/buildIndex.ts implementing core functionality to generate index without full build
  • Added code/core/src/core-server/build-index.ts handling configuration loading, presets, and story index generation
  • Modified code/lib/cli/src/proxy.ts to include 'index' command in CLI command handling
  • Added integration test in code/core/src/core-server/build-index.test.ts verifying index generation
  • Updated telemetry in code/core/src/telemetry/types.ts to track new 'index' command usage

Copy link

nx-cloud bot commented Dec 16, 2024

View your CI Pipeline Execution ↗ for commit 7d99f49.

Command Status Duration Result
nx affected -t check -c production --parallel=7 ✅ Succeeded 1m 23s View ↗
nx run-many -t build -c production --parallel=3 ✅ Succeeded 3m 32s View ↗

☁️ Nx Cloud last updated this comment at 2025-02-23 16:33:57 UTC

Comment on lines 67 to 71
export const buildIndexStandalone = async (options: BuildIndexOptions) => {
const index = await buildIndex(options);
console.log(`Writing index to ${options.outputFile}`);
await writeFile(options.outputFile, JSON.stringify(index, null, 2));
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this get added to standalone.ts also? I'm not quite sure what's up with that API.

@shilman shilman changed the title CLI: Add index command CLI: Add index command / API Dec 16, 2024
@shilman shilman self-assigned this Feb 23, 2025
@shilman shilman marked this pull request as ready for review February 23, 2025 16:27
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

12 file(s) reviewed, 8 comment(s)
Edit PR Review Bot Settings | Greptile

import { findPackage } from 'fd-package-json';
import invariant from 'tiny-invariant';

export const buildIndex = async (cliOptions: any) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: cliOptions should be properly typed rather than using 'any' to ensure type safety

Comment on lines +13 to +14
configDir: cliOptions.configDir || './.storybook',
outputFile: cliOptions.outputFile || './index.json',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: paths should use path.join() or path.resolve() to handle cross-platform path separators correctly

Comment on lines +157 to +158
outputDir: 'SBCONFIG_OUTPUT_DIR',
configDir: 'SBCONFIG_CONFIG_DIR',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: outputDir env variable is used but not relevant for index command which uses outputFile instead

Suggested change
outputDir: 'SBCONFIG_OUTPUT_DIR',
configDir: 'SBCONFIG_CONFIG_DIR',
configDir: 'SBCONFIG_CONFIG_DIR',

Comment on lines +69 to +70
console.log(`Writing index to ${options.outputFile}`);
await writeFile(options.outputFile, JSON.stringify(index));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Use logger.info instead of console.log to maintain consistent logging throughout the codebase

await index({
...options,
packageJson: pkg,
test: !!options.test || process.env.SB_TESTBUILD === 'true',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: test option is used but not defined in command options (unlike build command)


import { StoryIndexGenerator } from './utils/StoryIndexGenerator';

type BuildIndexOptions = CLIOptions & LoadOptions & BuilderOptions & { outputFile: string };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: BuildIndexOptions type should be exported since it's used by public API functions

Suggested change
type BuildIndexOptions = CLIOptions & LoadOptions & BuilderOptions & { outputFile: string };
export type BuildIndexOptions = CLIOptions & LoadOptions & BuilderOptions & { outputFile: string };

@@ -1,6 +1,7 @@
import { dirname } from 'node:path';

import { buildDevStandalone } from './build-dev';
import { buildIndexStandalone } from './build-index';
import { buildStaticStandalone } from './build-static';

async function build(options: any = {}, frameworkOptions: any = {}) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider adding TypeScript types for options and frameworkOptions parameters instead of 'any'

Comment on lines +312 to +317
| Option | Description |
| --------------------------------- | ----------------------------------------------------- |
| `-o`, `--output-file <file-name>` | JSON file to output index |
| `-c`, `--config-dir <dir-name>` | Directory where to load Storybook configurations from |
| `--quiet` | Suppress verbose build output |
| `--loglevel <level>` | Control level of logging during build |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Should include --disable-telemetry and --enable-crash-reports options for consistency with other commands

Suggested change
| Option | Description |
| --------------------------------- | ----------------------------------------------------- |
| `-o`, `--output-file <file-name>` | JSON file to output index |
| `-c`, `--config-dir <dir-name>` | Directory where to load Storybook configurations from |
| `--quiet` | Suppress verbose build output |
| `--loglevel <level>` | Control level of logging during build |
| Option | Description |
| --------------------------------- | ----------------------------------------------------- |
| `-o`, `--output-file <file-name>` | JSON file to output index |
| `-c`, `--config-dir <dir-name>` | Directory where to load Storybook configurations from |
| `--quiet` | Suppress verbose build output |
| `--loglevel <level>` | Control level of logging during build |
| `--disable-telemetry` | Disables Storybook's telemetry |
| `--enable-crash-reports` | Enables sending crash reports to Storybook's telemetry |

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants