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 command to generate annotations file #17783

Closed
wants to merge 10 commits into from

Conversation

yannbf
Copy link
Member

@yannbf yannbf commented Mar 22, 2022

Issue: N/A

What I did

The CLI now has a command to generate preset annotations file. This will get framework presets, addon presets and project annotation (preview.js) and write to a file that might look like this:

import * as frameworkPreset from "@storybook/react/dist/esm/client/docs/config";
import * as frameworkPreset2 from "@storybook/react/dist/esm/client/preview/config";
import * as addonPreset from "@storybook/addon-docs/dist/esm/frameworks/common/config";
import * as addonPreset2 from "@storybook/addon-actions/dist/esm/preset/addDecorator.js";
import * as addonPreset3 from "@storybook/addon-actions/dist/esm/preset/addArgs.js";
import * as addonPreset4 from "@storybook/addon-backgrounds/dist/esm/preset/addDecorator.js";
import * as addonPreset5 from "@storybook/addon-backgrounds/dist/esm/preset/addParameter.js";
import * as projectAnnotations from "./preview.ts";

export default [addonPreset, frameworkPreset, frameworkPreset2, addonPreset2, addonPreset3, addonPreset4, addonPreset5, projectAnnotations];

How to test

1 - Build CLI
2 - Use it locally with sb generate-annotations in a Storybook project

  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

@nx-cloud
Copy link

nx-cloud bot commented Mar 22, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit a644602. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

We should probably wait to merge this until #17755 is merged

lib/cli/src/generate-annotations.ts Outdated Show resolved Hide resolved
Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

Generally good! Some nitpicks

lib/cli/src/generate-annotations.test.ts Outdated Show resolved Hide resolved
lib/cli/src/generate-annotations.ts Outdated Show resolved Hide resolved
lib/cli/src/generate-annotations.ts Outdated Show resolved Hide resolved
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

I'm unclear on the workflow here. Is the idea that the user will run this to generate an annotations file, and then will edit it down by hand to only include the pieces that they want? Based on our discussions I thought we concluded that typically users would build these files "constructively" by adding the minimal number of decorators etc. that they might need, rather than "destructively" by deleting. But maybe I misunderstood!

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

(oops one more dupe... 🤷 )

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

(oops duplicate)

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

(oops another duplicate?)

@yannbf
Copy link
Member Author

yannbf commented Mar 23, 2022

I'm unclear on the workflow here. Is the idea that the user will run this to generate an annotations file, and then will edit it down by hand to only include the pieces that they want? Based on our discussions I thought we concluded that typically users would build these files "constructively" by adding the minimal number of decorators etc. that they might need, rather than "destructively" by deleting. But maybe I misunderstood!

Generally yes, but I thought we were going to get this PR in because it would make our experimentation easier?

cc @tmeasday

@shilman shilman changed the title CLI: add command to generate annotations file CLI: Add command to generate annotations file Mar 25, 2022
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Works for me. I'm not enthusiastic about it, but I'm fine if it's convenient.

We can merge & re-address in 7.0 CLI refactor if needed

@yannbf
Copy link
Member Author

yannbf commented Mar 28, 2022

@tmeasday after discussing more about this with @shilman we decided to keep this open until #17755 is resolved.

@yannbf yannbf changed the base branch from next to future/base April 14, 2022 08:58
@github-actions
Copy link
Contributor

github-actions bot commented Apr 14, 2022

Fails
🚫

PR is not labeled with one of: ["cleanup","BREAKING CHANGE","feature request","bug","documentation","maintenance","dependencies","other"]

Generated by 🚫 dangerJS against a644602

@yannbf
Copy link
Member Author

yannbf commented Apr 22, 2022

Closing this as might be too impactful for the changes we are planning to do in 7.0. If we ever want this again we can revive the PR.

@yannbf yannbf closed this Apr 22, 2022
@stof stof deleted the feat/generate-presets-file branch August 31, 2022 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants