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

Discussion: CLI Overhaul #10723

Closed
2 tasks
yannbf opened this issue May 10, 2020 · 26 comments
Closed
2 tasks

Discussion: CLI Overhaul #10723

yannbf opened this issue May 10, 2020 · 26 comments
Assignees
Labels
cli discussion maintenance User-facing maintenance tasks

Comments

@yannbf
Copy link
Member

yannbf commented May 10, 2020

What is this?

This is an issue to center the status and discussions about the work that's being done at #10702.

We are making an overhaul of the CLI so that we can automate the generation of templates for the frameworks that Storybook supports.
The work also involves adding and improving E2E tests and making the storybook installation consistent across all frameworks.

This gives us the opportunity to:

  • Drastically reduce code from the monorepo
  • Greatly improve yarn bootstrap performance by only installing the basic, necessary projects
  • Generate templates on demand, with any version/scenario (e.g. Angular@8, Angular@latest, React+Yarn2, etc)
  • Automatically generate and update example repos (to be done in the future)

General

There is already quite some work done, and it has been divided into multiple PRs.

Framework Task owner PR Status
Web Components @tooppaaa #10703 MERGED
Svelte @tooppaaa #10704 MERGED
HTML @tooppaaa #10705 MERGED
RAX @tooppaaa #10706 MERGED
Mithril @tooppaaa #10707 MERGED
Vue @tooppaaa #10708 MERGED
Webpack React @tooppaaa #10709 MERGED
React scripts @tooppaaa #10710 MERGED
React @tooppaaa #10711 MERGED
Preact @tooppaaa #10712 MERGED
Ember @tooppaaa #10713 MERGED
SFC Vue @tooppaaa #10714 MERGED
Riot @tooppaaa #10715 MERGED

Additional tasks

  • Update stories on CSF-MDX to match with the examples of CSF
  • Add E2E to test MDX configurations as well. Currently it's only the default CSF template generated by sb init

The table below will be updated as new tasks arise:

Task Task owner PR Status
#10793 CLI: colocate example components together with generated stories @tooppaaa #10935 MERGED
#10794 CLI: consistent welcome-component in all generators @tooppaaa #11422 MERGED
#11282 CLI: use addon essentials in storybook templates @tooppaaa - MERGED
#10796 CLI: remove addon notes from framework generators @yannbf - CLOSED
#10797 CLI: Improve support of Yarn 2 in E2E tests @gaetanmaisse #10797 #11342 MERGED
#10798 CLI: check for missing/unneeded deps when initializing SB in a project - - CLOSED
## Discussion
  1. Stories being generated under stories vs src/stories.

    • Currently it's inconsistent across frameworks. What's should be the correct path?
  2. Using demo components from @storybook/FRAMEWORK/demo vs shipping example components together with the stories.

    • Currently it's inconsistent across frameworks. Should we keep the stories boilerplate as minimal as possible by just importing components from the demos like React or should we ship example files like it's done with Vue?
  3. Updating Welcome component content.

    • The content of the Welcome component should be consistent across frameworks. One thing we noticed is the message about webpack:
      Have a look at the .storybook/webpack.config.js to add webpack loaders and plugins you are using in this project. although that file is not present. Maybe it should be just rephrased that if you add the file it will be picked up, or rather tell people to add the rules to main.js instead if that's the way to go. Also, in some frameworks there is inconsistency because in the welcome component there is a reference to stories/1-Button.stories.js but the correct path is actually src/stories/1-Button.stories.js and the other way around (to be aligned with point 1 above).
  4. removing notes parameter on template stories

    • Lots of example stories have notes as parameters, although the notes addon has been removed.

I'll be updating this text as we progress. For the ones working on this feel free to update this as well!

@yannbf yannbf changed the title CI Overhaul Discussion: CI Overhaul May 10, 2020
@shilman shilman added the maintenance User-facing maintenance tasks label May 11, 2020
@shilman
Copy link
Member

shilman commented May 11, 2020

Thanks for opening this @yannbf! Agree with your suggestions across the board. The first question about stories vs src/stories is tricky.

  • Ideally we want to the setup to be as close to their final setup as possible. Some users won't have a src directory. Some setups, like CRA, won't work outside of a src directory.
  • We shipped demo components with most of the frameworks, but addon-docs prop tables don't currently work on NPM modules, and we also want to encourage users to colocate their stories with their components.

Given this, perhaps we should:

  • add colocated components/stories as part of the CLI templates
  • add to src/stories if src exists, otherwise stories?

WDYT?

@shilman shilman added the cli label May 11, 2020
@shilman shilman changed the title Discussion: CI Overhaul Discussion: CLI Overhaul May 11, 2020
@ndelangen
Copy link
Member

ndelangen commented May 11, 2020

Using demo components from @storybook/FRAMEWORK/demo vs shipping example components together with the stories.

agreed

add to src/stories if src exists, otherwise stories?

agreed

The content of the Welcome component should be consistent across frameworks. One thing we noticed is the message about webpack

We should update it so it mentions main.js and links to the docs.

@yannbf
Copy link
Member Author

yannbf commented May 11, 2020

add to src/stories if src exists, otherwise stories?

Downside to this is we would need to change the implementation of the generators and make it more complex, because right now it just does a copySync of the entire template folder, very straight forward (but less dynamic).

Also, we'd need to change the text on the Welcome story based on that, because in that text there is a reference to one of the stories like this:
Try editing the Button stories located at src/stories/1-Button.stories.js
But of course if we revisit the content of the Welcome component we might not need to do that.

@ndelangen
Copy link
Member

ndelangen commented May 11, 2020

Downside to this is we would need to change the implementation of the generators and make it more complex

true, good point, that is a downside..

We could say we copy the stories into the project dir, then OPTIONALLY move the stories dir into the src directory?

And make sure the glob specified in main.js matches both.

Also, we'd need to change the text on the Welcome story based on that,

I think we can drop the src/ in that, because it's kinda obvious if the user already had a src directory we'd place it there.

@tooppaaa
Copy link
Contributor

tooppaaa commented May 11, 2020

Here's my suggestion for the one-for-all wording of the introduction story.
I took the react one as basis and tried to remain cross-framework, "src-independent", and provide useful links to the docs

Should we add a section that encourages the usage of docs ?


Welcome to storybook

This is a UI component dev environment for your app.

We've added some basic stories inside your source code.

A story is a single state of one or more UI components. You can have as many stories as you want.

(Basically a story is like a visual test case.)

Stories

You can add your own components as stories.
Have a look at the Writing Stories section in our documentation.

Use the left panel to switch between stories.

Create new stories using one of the following format:

You can edit components and see changes right away.

(Try editing the Button stories located in the file called 1-Button.stories.js.)

Configuration

Your app may require additional configurations. Check our configuration documentation for more information

Tips

  • Usually we create stories with smaller UI components in the app.
  • Check addons out!
  • Call your stories using *.stories.* format such as Button.stories.js

@yannbf
Copy link
Member Author

yannbf commented May 11, 2020

So @shilman mentioned in one of the PRs:

Agree we should get rid of notes and pick a better example. Maybe backgrounds and/or docs?

What do we want to use? I think it would be nice to use docs because then eventually docs becomes part of storybook as a default experience and more people get to enjoy it out of the box!

@tooppaaa
Copy link
Contributor

Docs is a different approach. I can see projects where I am not yet sure I'll install docs. (simpler is better). Same for backgrounds. So this leads to:
Do we really need to demonstrate another addon in the CLI generator ?

This increase the maintenance cost (Lots of PR to harmonize only links & actions)
In my opinion, the welcome page with links to addons and the button story does the job.
We can later add a ref to a "discover storybook" instance using composition ? There will be a single place to maintain to add a demo of what storybook can do ?

Concerning @storybook/FRAMEWORK/demo I'm more in favor of colocating components & stories, mostly because that's what end users will have.

@yannbf
Copy link
Member Author

yannbf commented May 11, 2020

Do we really need to demonstrate another addon in the CLI generator ?

That's a great question!

We can later add a ref to a "discover storybook" instance using composition ? There will be a single place to maintain to add a demo of what storybook can do ?

Sounds like a great idea!

@shilman
Copy link
Member

shilman commented May 12, 2020

@tooppaaa i think it's fine if the user has to delete some or even all of the code generated by the template -- that's already the case with stories. its main purposes are (1) to give users something useful out of the box and (2) to educate them about storybook concepts. what's nice specifically about the notes example is that it was both useful (prior to docs) and also demonstrated story parameters, which are a key concept in storybook.

  • docs educates users about a major feature in storybook (documentation)
  • backgrounds is the simplest possible way to educate them about parameters

@ndelangen
Copy link
Member

it's fine if the user has to delete some or even all of the code generated by the template

But less is better.

referencing demo components from @storybook/<framework>/demo is better IMHO.

We can later add a ref to a "discover storybook" instance using composition ? There will be a single place to maintain to add a demo of what storybook can do

The issue is that (as @shilman mentions as well), that part of the purpose of this copied template is to give the user something to play with code-wise. If it's a ref they can see the storybook, but they cannot experience changing the component & story.

But I'm not apposed to the idea of adding a ref by default... WDYT? (asking to anyone reading this)

@shilman
Copy link
Member

shilman commented May 12, 2020

@yannbf @tooppaaa

In 5.3 I wrote a meta-addon called Storybook Essentials, which is a curated collection of zero-config addons for Storybook. I think Essentials should be the basis for the updated CLI template and related work.

Essentials had a technical limitation about package imports. However, I've fixed the limitation in in 6.0, and it's now available in #10737

Over the course of the 6.x milestones, we will actively promote essentials as the easiest way to get started with a high-quality Storybook experience. If all the CLI templates use it and all the tests make sure that it works smoothly with all the existing frameworks, it will be a huge win for users and for Storybook maintenance.

@tooppaaa
Copy link
Contributor

Reusing what has been done ? Love the idea !
I didn't know about it but I've seen this experience in Apollo. You have a "simple" starter, with guidelines if you want to "eject".
Is this ready to be used in CLI ?

@shilman
Copy link
Member

shilman commented May 12, 2020

Huzzah!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.0.0-beta.6 containing PR #10729 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed May 12, 2020
@shilman shilman reopened this May 12, 2020
@yannbf
Copy link
Member Author

yannbf commented May 12, 2020

Hey @shilman what tasks based on this discussion do you think can be created so far? Or should we mature it a bit more before creating tasks?

@shilman
Copy link
Member

shilman commented May 12, 2020

@tooppaaa the updated essentials is now available as part of 6.0.0-beta.6. it's relatively untested, so i'm sure we'll find some issues, but hopefully nothing big.

@yannbf go for it! 💯

@shilman shilman added this to the 6.0 essentials milestone May 13, 2020
@yannbf
Copy link
Member Author

yannbf commented May 14, 2020

Ok @shilman @ndelangen I'd love to create tasks however I'd like to know which ones should be tasks, which ones should not, and have a more concrete idea about them:

1 - Remove addon notes usage from CLI generated stories.
2 - Make CLI generated setup use only addon-essentials. Update E2E tests accordingly.
3 - Change the Welcome component content (please check the suggestion by @tooppaaa) and make it consistent in every template.
4 - From @ndelangen about sb init: We could say we copy the stories into the project dir, then OPTIONALLY move the stories dir into the src directory?

And this one doesn't seem to be decided
5 - Use demo components in generated stories
OR
5 - Colocate components together with generated stories

If you could help me out shape these into proper tasks would be awesome!

@ndelangen
Copy link
Member

my preferences:
✅ remove addon notes from generators
✅ add addon-essentials to the generators
❓ Use demo components in generated stories
❓ generate into the src directory if we can find one
✅ consistent welcome-component (and button) in all generators
(let's show a pretty one, with:

  • storybook logo
  • link to storybook docs
  • link to discord invite
  • link to storybook repo on github
  • some message about the ability to add & write addons

@domyen would you be able to design a welcome-to-storybook component that can be re-used for all framework generators?

@domyen
Copy link
Member

domyen commented May 14, 2020 via email

@shilman
Copy link
Member

shilman commented May 14, 2020

@yannbf @ndelangen we need to ship components with the stories. if we do:

  • props tables will work out of the box, which is super important
  • stories and components will be colocated per our recommendations
  • it will also make tutorials easier because we can ask users to edit the added components

it adds more complexity to the templates, but we just need to deal with it

@yannbf
Copy link
Member Author

yannbf commented May 15, 2020

@gaetanmaisse would you be so kind and create issues about the following points you mentioned?

  • Check that no deps or peerDeps added when running sb init are missing (Yarn 2 will help on that as it is way more strict)
  • Yarn 2 Improvements

@tooppaaa
Copy link
Contributor

@domyen Did you had a chance to work on that Welcome page ?

@domyen
Copy link
Member

domyen commented May 30, 2020

Hey @tooppaaa will work on that this coming week. I'm freed up some time to focus on SB design tasks. 👍

@airtonix
Copy link

Is this where we ask for the Progress spam to be turned off with a simple true statement in our main.js ?
image

@ndelangen
Copy link
Member

@airtonix yes, but that's really unrelated the the issue currently being discussed..

add --quiet to the start script

@shilman
Copy link
Member

shilman commented Jul 16, 2020

Jeepers creepers!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.0.0-rc.8 containing PR #11505 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Jul 16, 2020
@shilman shilman reopened this Jul 16, 2020
@ndelangen
Copy link
Member

@shilman can we close this issue?

maybe create a new issue to track the remaining bits if there are any?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli discussion maintenance User-facing maintenance tasks
Projects
None yet
Development

No branches or pull requests

7 participants