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

Create an official template for CRA #9327

Open
kevin940726 opened this issue Jan 5, 2020 · 16 comments
Open

Create an official template for CRA #9327

kevin940726 opened this issue Jan 5, 2020 · 16 comments
Labels
cli cra Prioritize create-react-app compatibility feature request

Comments

@kevin940726
Copy link
Contributor

kevin940726 commented Jan 5, 2020

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
Copy link
Member

shilman commented Jan 5, 2020

@kevin940726 this sounds awesome! @mrmckeb WDYT?

@kevin940726
Copy link
Contributor Author

kevin940726 commented Jan 8, 2020

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
Copy link
Member

mrmckeb commented Jan 8, 2020

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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
Copy link
Member

mrmckeb commented Jan 8, 2020

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.
facebook/create-react-app#7991

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

@mrmckeb
Copy link
Member

mrmckeb commented Jan 8, 2020

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
Copy link
Contributor Author

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
Copy link
Contributor Author

kevin940726 commented Jan 8, 2020

@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
Copy link

stale bot commented Feb 6, 2020

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!

@stale stale bot added the inactive label Feb 6, 2020
@mrmckeb
Copy link
Member

mrmckeb commented Mar 1, 2020

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!

@stale stale bot removed the inactive label Mar 1, 2020
@kevin940726
Copy link
Contributor Author

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

@mrmckeb
Copy link
Member

mrmckeb commented Mar 2, 2020

I'd be OK with that.

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

What do you think?

@kevin940726
Copy link
Contributor Author

@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
Copy link
Member

mrmckeb commented Mar 3, 2020

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
Copy link
Contributor Author

@mrmckeb Agree!

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli cra Prioritize create-react-app compatibility feature request
Projects
None yet
Development

No branches or pull requests

4 participants