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

change init script of storybook #7695

Merged
merged 10 commits into from Apr 14, 2024
Merged

change init script of storybook #7695

merged 10 commits into from Apr 14, 2024

Conversation

zsh77
Copy link
Contributor

@zsh77 zsh77 commented Mar 11, 2024

Description

By running pnpm dlx sb init --skip-install and pnpm install --save-dev @storybook/cli storybook doesn't get added to the vite app properly and run into esbuild postinstall error which fails to start storybook. Although by running pnpm dlx sb init everything works and storybook and the dependencies get installed properly.

Testing Instructions

Going through with the process of adding storybook to turborepo should work fine which is:

  1. cd apps
  2. pnpm create vite
  3. add this to .npmrc :
auto-install-peers=true
legacy-peer-deps=true
node-linker=hoisted
  1. cd workshop
  2. pnpm dlx sb init

there is no package named 'tsconfig'. Replaced it with the correct name which is '@repo/typescript-config'
It wasn't being used at all.
By running `pnpm dlx sb init --skip-install` and `pnpm install --save-dev @storybook/cli` storybook doesn't get added to the vite app properly and run into `esbuild postinstall error` which fails to start storybook. Although by running  `pnpm dlx sb init` everything works and storybook  and the dependencies get installed properly.
@zsh77 zsh77 requested review from anthonyshew and a team as code owners March 11, 2024 06:42
@turbo-orchestrator turbo-orchestrator bot added area: docs Improvements or additions to documentation owned-by: turborepo labels Mar 11, 2024
Copy link

vercel bot commented Mar 11, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

7 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview Apr 13, 2024 8:15am
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview Apr 13, 2024 8:15am
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview Apr 13, 2024 8:15am
examples-native-web ⬜️ Ignored (Inspect) Visit Preview Apr 13, 2024 8:15am
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview Apr 13, 2024 8:15am
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview Apr 13, 2024 8:15am
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview Apr 13, 2024 8:15am

Copy link

vercel bot commented Mar 11, 2024

@zsh77 is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

@ijjk
Copy link
Member

ijjk commented Mar 11, 2024

Allow CI Workflow Run

  • approve CI run for commit: 5caaeec

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

@zsh77 zsh77 changed the title change init acript of storybook change init script of storybook Mar 11, 2024
@@ -78,8 +78,7 @@ Next, we need to scaffold Storybook:

```shell
cd workshop
pnpm dlx sb init --skip-install
pnpm install --save-dev @storybook/cli # Manually install deps and CLI
Copy link
Contributor

Choose a reason for hiding this comment

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

I specifically remember having to go through these steps in this order as the only way I could get it to work.

I think the Storybook team has released two majors since I wrote this doc, though, some some things may have changed. Can you confirm that you tested this manually?

Copy link
Contributor Author

@zsh77 zsh77 Mar 15, 2024

Choose a reason for hiding this comment

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

@anthonyshew yes I've got some screen shots just now to make sure once again that this works. Let me show you.

image

image

image

image

...

image

Screenshot 1402-12-25 at 14 10 15

Although this was just now with storybook version 8.0.0 .When I openeed this PR a couple of days ago the stable version of storybook was 7.6 and this also worked fine and adding storybook without installing dependencies was running into errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know how you feel about it, but I thought maybe including this link :
https://github.com/storybookjs/addon-onboarding/blob/main/README.md
in the dos would be beneficial and time saving in order to follow the instructions to uninstall @storybook/addon-onbording.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for going the extra mile with the screenshots! 🙏

I'm okay with that link! Feel free to add that and then I'll approve.

Copy link
Contributor Author

@zsh77 zsh77 Apr 2, 2024

Choose a reason for hiding this comment

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

Thank you 🙏

It's all done. Please let me know if you know a better way to document it.

@tounsoo
Copy link

tounsoo commented Mar 28, 2024

With SB8 and with turbo@1.13.0, I'm unable to make the SB work properly with the current guide + the guide defined in this PR. Might require additional configuration to get it work with SB8.

I'm getting this type error:
image

Note: I have added the dependency of UI as so "@repo/ui": "workspace:*", because pnpm dlx create-turbo@latest names the UI package as @repo/ui now.

I have also tried methods in this thread with no luck: #620

@zsh77
Copy link
Contributor Author

zsh77 commented Mar 30, 2024

With SB8 and with turbo@1.13.0, I'm unable to make the SB work properly with the current guide + the guide defined in this PR. Might require additional configuration to get it work with SB8.

I'm getting this type error: image

Note: I have added the dependency of UI as so "@repo/ui": "workspace:*", because pnpm dlx create-turbo@latest names the UI package as @repo/ui now.

I have also tried methods in this thread with no luck: #620

@tounsoo the error also happens if there are no exports like "." in the ui's package.json while you're using import {Button} from "@repo/ui". You could follow these steps while using pnpm:

  1. npx create-turbo@latest
  2. cd apps
  3. pnpm create vite (name the new package workshop)
  4. add this to .npmrc
auto-install-peers=true
legacy-peer-deps=true
node-linker=hoisted
  1. cd workshop
  2. pnpm dlx sb init (at this point you will get the storybook running)
  3. add @repo/ui to the dependencies of workshop like this from the root of repo:
pnpm add @repo/ui --filter=workshop
  1. To use a button from the ui package, you could change the import of button in Button.stories.tsx replace it with:
import { Button } from "@repo/ui/button";

because the package.json of ui package exports button.ts file as ./button.
10. Change Button.stories.tsx so that it will fullfill the new props of the button from ui package.

@tounsoo
Copy link

tounsoo commented Apr 1, 2024

@zsh77 Thank you!

Copy link
Contributor

@anthonyshew anthonyshew left a comment

Choose a reason for hiding this comment

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

Great! Thank you!

@anthonyshew anthonyshew merged commit 64f77a4 into vercel:main Apr 14, 2024
42 of 50 checks passed
@zsh77 zsh77 mentioned this pull request Apr 14, 2024
@zsh77 zsh77 deleted the patch-3 branch April 19, 2024 15:51
anthonyshew added a commit that referenced this pull request Apr 22, 2024
### Description

- fixed the init script of storybook on the `npm` tab, (the exact
changes we had in the PR: #7695
using npm instead of pnpm)
- replaced `ui` with `@repo/ui`
- bug fix of importing a button from an undefined export specifier

### Testing Instructions

Going through with the process of adding storybook to turborepo should
work fine which is:

- cd apps
- npm create vite
- cd workshop
- npx sb init

Co-authored-by: Anthony Shew <anthony.shew@vercel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: docs Improvements or additions to documentation owned-by: turborepo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants