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

unable to get storybook working #10

Closed
JacobInCode opened this issue Sep 16, 2021 · 16 comments
Closed

unable to get storybook working #10

JacobInCode opened this issue Sep 16, 2021 · 16 comments
Labels
enhancement New feature or request

Comments

@JacobInCode
Copy link

Hi, thanks for the awesome template. Unfortunately, running storybook init doesn't add any of the correct directories. Any ideas?

@markatli
Copy link

markatli commented Sep 16, 2021

Would also love to see this boilerplate working out-of-the-box with Storybook.

@ixartz
Copy link
Owner

ixartz commented Sep 16, 2021

@jsmellz I've never tried storybook for this project. So, it can definitely be hard to integrate the boilerplate with Storybook.

Totally Ok to add Storybook to this project. Open to suggestion and feedback.

@ixartz ixartz added the enhancement New feature or request label Nov 28, 2021
@riolly
Copy link
Contributor

riolly commented Jul 4, 2022

@0xJacobean
The import alias like '@' is causing the problem.
You have to resolve it on the storybook main config file like this
Here is the official docs for the config.
Here is great resource to setup the storybook on nextjs from the creator.

@markatli @ixartz
Making a specific branch with storybook integrated as default will be awesome.

@ixartz
Copy link
Owner

ixartz commented Jul 4, 2022

Someone has already open a pull request: #35

But, he didn't responded back

@CO0Ki3
Copy link
Contributor

CO0Ki3 commented Dec 7, 2022

@ixartz ok, so I prepared to pr, but wanna get your advice.

  1. bugs in npm v8
    check this link: "storybook init" builds wonky package-lock.json using npm v8 storybookjs/storybook#18298
    I'm using node v18 and npm v8. I try setting storybook and faced this issue.
    화면 캡처 2022-12-07 092022
    Do migrate npm7, storybook makes .npmrc.
legacy-peer-deps=true

This is necessary to maintain package-lock, and there is no problem when someone else clones the project.
So is it okay to make pr with this?

  1. storybook default components
    I don't know if you've ever used storybook.
    When you first set this up, it create example components: a button, a header, and a page.
    Is it okay to make a pr with these included? If not, I'll remove components.
    image

@ixartz
Copy link
Owner

ixartz commented Dec 7, 2022

@CO0Ki3 Thank you for your help. But here is my experience:

  1. I'm not against legacy-peer-deps but the CI will be failing, at least last time I'm using legacy-peer-deps (not with Storybook) and it fails the CI. Is it any we can avoid legacy-peer-deps?
    Does it work with npm9 without legacy-peer-deps?

  2. I have never used Storybook before, my experience is limited. But, I think we should remove the default component that are not used in the boilerplate.

So, instead of a storybook of Logged Out, we can create a storybook of https://github.com/ixartz/Next-js-Boilerplate/blob/main/src/pages/index.tsx or https://github.com/ixartz/Next-js-Boilerplate/blob/main/src/pages/about.tsx

Currently, the boilerplate doesn't have a small component, I think it make sense to add a button component and create a new directory named components. Then, we can create a new storybook for the new button component.

What do you think?

@CO0Ki3
Copy link
Contributor

CO0Ki3 commented Dec 7, 2022

@ixartz Thank you for advice!

  1. I haven't seen legacy-peer-deps cause problems when project hasn't storybook.
    I'll check to see if there's a way to apply it only to storybook.
    I have never used npm9. However, since it is a higher version, it is expected that there will be problems.
    I'll check this one more time.

  2. I also agree with you.

In my opinion, the first problem should be solved first.
When it's solved, I will create an example with the page of this project and a simple button component.

@ixartz
Copy link
Owner

ixartz commented Dec 7, 2022

@CO0Ki3 Thank you for your response. We are totally aligned, definitively we should solve the first one. We can even merge the first one and then, tackling the second. It should make a smaller PR and make things easier to manage.

@CO0Ki3
Copy link
Contributor

CO0Ki3 commented Dec 8, 2022

@ixartz Cool:)

@CO0Ki3
Copy link
Contributor

CO0Ki3 commented Dec 8, 2022

@ixartz well.. npm v19 also make problem. So I'm finding how can use legacy-peer-deps only storybook:)
image

@CO0Ki3
Copy link
Contributor

CO0Ki3 commented Dec 8, 2022

@ixartz It just occurred to me that legacy-peer-deps ignores peer dependency conflicts.
As of now, both dev and prod seem to be working in local without any problems.
But what happens when CI fails?

@ixartz
Copy link
Owner

ixartz commented Dec 8, 2022

@CO0Ki3 You can open a new pull request and you can directly check on CI if it works.

@vcctm
Copy link

vcctm commented Dec 15, 2022

Just to be aware, did you guys make it? I will use this boilerplate and thinking to integrate storybook!

@ixartz
Copy link
Owner

ixartz commented Dec 15, 2022

Storybook 7 will automatically support Next.js out of the box: https://storybook.js.org/blog/integrate-nextjs-and-storybook-automatically/
I think it will make sense to add Storybook 7 directly, it's expected to be released in January.

@ixartz
Copy link
Owner

ixartz commented Dec 15, 2022

But, I'm also open to merge the PR if someone make it work with Storybook 6

@ixartz
Copy link
Owner

ixartz commented Apr 5, 2023

Good news! The boilerplate now supports Storybook. I've just added Storybook v7, they release this new version 5 days ago.

The PR: #112

@ixartz ixartz closed this as completed Apr 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants