storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.23k stars 9.26k forks source link

Create an official template for CRA #9327

Open kevin940726 opened 4 years ago

kevin940726 commented 4 years ago

Is your feature request related to a problem? Please describe. Since CRA now supports custom template, I think it makes sense for us to create official template for CRA. It would make new comers easier to get started with storybook and CRA.

Describe the solution you'd like We can create a new package cra-template-storybook and maintain it in this mono. We just have to follow the guide here, and the official template in cra-template and cra-template-typescript.

Describe alternatives you've considered npx -p @storybook/cli sb init works just fine, and we can continue support that for existing projects and for non-CRA projects. For new projects though, we can recommend users to just use cra-template-storybook to get started.

Are you able to assist bring the feature to reality? Absolutely! I think it's fairly straight-forward. There are only some minor issues, like where should we put this project? A new directory templates? Inside examples (but it's not an example)? Or maybe just in lib?

Also since we can control the template now, we could create example component directly inside the template rather than using @storybook/react/demo. Would love to hear some ideas about this 🙂.

Additional context In short for what it would look like when landed. Users can simply run this command and get an CRA with storybook.

npx create-react-app --template storybook
shilman commented 4 years ago

@kevin940726 this sounds awesome! @mrmckeb WDYT?

kevin940726 commented 4 years ago

First of all, I think in the first stage we can support for both cra-template-storybook and cra-template-storybook-typescript.

  1. Naming Do we want to name it cra-template-* or @storybook/cra-template-*?

    IMO cra-template-* seems cleaner, since we can install the template just by --template * and not --template @storybook/*. For example, --template storybook, not --template @storybook/storybook

  2. Directory I think we can just put them under lib? Although they are not scoped (presumed) and not actually a lib for the storybook app. Another option would be to create a new directory, templates, to place them.

    Being more aggressive, we could move all of our packages inside packages directory to match the community standard of mono repo directory structure. However, I think it's more unlikely.

  3. Template The basic idea is to just copy paste the default template created by sb init and merge them into the official default templates (cra-template, cra-template-typscript). We might want to customize Welcome component to match the template structure for CRA.
  4. Versioning Declarative Storybook configuration is on the horizon, should we just target 5.3 for these templates? So that we can just make our templates to follow the latest recommended way to configure storybook.
  5. Future We could make our official templates to include storybook doc by default, or we could create yet another templates for doc. Like cra-template-storybook-doc.

If these all sounds good, I can go ahead and create a PR for this, then we can gather further feedbacks from there.

mrmckeb commented 4 years ago

But this is definitely a good idea - before we introduced it, I flagged this idea with @shilman and @ndelangen too.

My only question is, what does the template do?

What we want to discourage is a template for every tool in the ecosystem, and instead we want to encourage templates that are great starting points for new apps. So I'd ask that we try to ship something that's more than CRA with Storybook preconfigured (as we can do that already). Perhaps it could also include Redux and Styled-Components - or something along those lines?

In short, if it's just SB init, I think it's hardly worth shipping... as we'll also then be responsible for the HTML template and some other core files that would regularly be updated on the CRA-side.

On point 1, I think I'd prefix it with Storybook so people know it's official.

On point 5, I think it should include docs on day one. What do you think @shilman?

kevin940726 commented 4 years ago

@mrmckeb For point 1, I think that requiring users to type --template @storybook/storybook is a bit odd. If we want it to seems official, I'd suggest --template @storybook (to install @storybook/cra-template). However, it's not currently supported by CRA, now running --template @storybook would instead install cra-template-@storybook.

@ is not a valid character in non-scoped package name. So that I think we can safely map @* to @*/cra-template in CRA. Should I make an issue/PR for this in CRA repo?

kevin940726 commented 4 years ago

IMHO an advantage for storybook is that it's purely a development tool, we don't have to touch any of the files in the default template. Instead we just have to add those deps and some files to src, and that's it. Perhaps we could automate this task with a script? So that we don't have to update our template every time CRA default template updates.

But yah it might seems like an overkill for creating a CRA template while we already can do those things today with sb init.

mrmckeb commented 4 years ago

I should double check, but I think you could just use @storybook. This PR added support for scoped templates, but I don't see a test for only providing a scope. https://github.com/facebook/create-react-app/pull/7991

Otherwise, I'm OK with just cra-template-storybook, but the scope does make it more official.

mrmckeb commented 4 years ago

But yah it might seems like an overkill for creating a CRA template while we already can do those things today with sb init.

I'm not sure... We could use this to produce a best-practice template (the Storybook way)?

kevin940726 commented 4 years ago

I accidentally pressed comment, oops.

My recommendation would be to make this template more tightly coupled with CRA, like integrating it with jest, testing-library/react, etc. To make a difference from the sb init. Include more things that we cannot do with sb init. Maybe even a larger list of predefined addons?

kevin940726 commented 4 years ago

@mrmckeb I just ran it, and here's the output.

$ npx create-react-app@latest --template @storybook my-app
npx: installed 91 in 4.455s

Creating a new React app in /Users/khao/work/repo/my-app.

Installing packages. This might take a couple of minutes.
Installing react, react-dom, and react-scripts with cra-template-...

yarn add v1.19.1
[1/4] 🔍  Resolving packages...
error An unexpected error occurred: "https://registry.npmjs.org/cra-template-: Not found".
info If you think this is a bug, please open a bug report with the information provided in "/Users/khao/work/repo/my-app/yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/add for documentation about this command.

Aborting installation.
  yarnpkg add --exact react react-dom react-scripts cra-template-@storybook --cwd /Users/khao/work/repo/my-app has failed.

Deleting generated file... package.json
Deleting generated file... yarn.lock
Done.

Looks like it's trying to install cra-template-@storybook.

I'm aware of facebook/create-react-app#7991, judging from the changes, looks like this case is not handled.

stale[bot] commented 4 years ago

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

mrmckeb commented 4 years ago

Hi @kevin940726, that PR was merged and you should be able to continue this work now. Let me know if you hit any other template issues - as I was the one that built that feature, I can hopefully help!

kevin940726 commented 4 years ago

@mrmckeb Great! I'm wondering should we target this for 6.0 release though, as part of the features we can announce?

mrmckeb commented 4 years ago

I'd be OK with that.

I'd like that we encourage stories alongside components in this template too.

What do you think?

kevin940726 commented 4 years ago

@mrmckeb You mean putting Button.js and Button.stories.js in the same directory? I'd love that, personally I've never used an universal stories folder ever. Either putting them inside the same directory, or following the convention in Jest and put stories inside a nearest __stories__ folder along side the component. It could be too complicated for a template to do the latter though, in practice, I'd expect to put stories at the same place as tests.

mrmckeb commented 4 years ago

Exactly @kevin940726 :) I think just alongside the component (Jest also allows you to put the test files alongside and use *.test.js).

The other question would be - do we have a TypeScript template or JavaScript template - or both?

kevin940726 commented 4 years ago

@mrmckeb Agree!

I would say both, @storybook/cra-template and @storybook/cra-template-typescript.