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

upgrade storybook to 5.1.11 #1132

Merged
merged 4 commits into from
Jun 15, 2020
Merged

upgrade storybook to 5.1.11 #1132

merged 4 commits into from
Jun 15, 2020

Conversation

chynh
Copy link
Contributor

@chynh chynh commented Jun 8, 2020

Description of changes

Migrate to Storybook 5.1.11
Minor changes to jest tests to remove console warnings

Issue Resolved

Fixes #397

@vercel
Copy link

vercel bot commented Jun 8, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/operation-code/operation-code/jp4s50zcz
✅ Preview: https://operation-code-git-fork-chynh-storybook.operation-code.vercel.app

.storybook/webpack.config.js Outdated Show resolved Hide resolved
@kylemh
Copy link
Member

kylemh commented Jun 8, 2020

Just a reminder @chynh - you're a member so you don't need to fork this repo. You can just make branches directly off this repo. It'll be beneficial for me so I don't have to use https://hub.github.com/ to checkout your PR

Anyways, the Netlify build seems to be failing and not because of caching issues.

@chynh
Copy link
Contributor Author

chynh commented Jun 8, 2020

Just a reminder @chynh - you're a member so you don't need to fork this repo. You can just make branches directly off this repo. It'll be beneficial for me so I don't have to use https://hub.github.com/ to checkout your PR

Oh yeah I did not know this. Thanks for letting me know.

Anyways, the Netlify build seems to be failing and not because of caching issues.

Seems like you guys had the same issue before. Were you able to resolve it?
Maybe it has something to do with this message. Module not found: Error: Can't resolve 'fs' in '/opt/build/repo/node_modules/require-context.macro'

@kylemh
Copy link
Member

kylemh commented Jun 9, 2020

😅 encountering my own demons

storybookjs/require-context.macro#25

A few ideas:

  1. Add fs as a devDependency. I don't think this will work, but it'd be the quickest fix.
  2. Go to Storybook 5.3.x and use .storybook/main.js instead of .storybook/{addons|config|backgrounds}.js since this fs/webpack interop doesn't seem to be required anymore.
  3. Use the hook instead of the macro https://github.com/storybookjs/storybook/tree/v5.1.11/addons/storyshots/storyshots-core#option-1-plugin
  4. I want to move away from Netlify cuz it's costing me $5/mo. Perhaps Vercel has a better container environment that "just works" ™️. You could add a 2nd build to now.json (I'm just worried about what this would mean for our GitHub PR integration tho)

@kylemh
Copy link
Member

kylemh commented Jun 10, 2020

🤖 This is a bot 🤖

🎉 Deployed Storybook preview! 🎉

Click the link at the bottom of this comment to see it live.

Built with commit fda348d

https://deploy-preview-1132--operation-code-storybook.netlify.app

@kylemh
Copy link
Member

kylemh commented Jun 10, 2020

Getting TypeError: fs.readdirSync is not a function in the runtime on the Netlify deploy

@chynh
Copy link
Contributor Author

chynh commented Jun 11, 2020

Getting TypeError: fs.readdirSync is not a function in the runtime on the Netlify deploy

hmm yeah I saw that. I guess I'll have to spend some time looking into this issue this weekend. Is it possible for me to trigger netlify deploy manually?

@kylemh
Copy link
Member

kylemh commented Jun 11, 2020

You're more than welcome to make your own Netlify account and deploy your Storybook builds willy nilly!

@chynh
Copy link
Contributor Author

chynh commented Jun 14, 2020

I was able to deploy without the weird 'fs' issue on my netlify account. Not sure what is causing issue on your account. Maybe the build setting?

Below is my setting:
Screen Shot 2020-06-14 at 2 03 56 PM

Ideally, it would be great to add 2nd build to Vercel but I am not sure how to do that. Been looking into document but not have been successful so far. ¯\_(ツ)_/¯

@codeclimate
Copy link

codeclimate bot commented Jun 15, 2020

Code Climate has analyzed commit fda348d and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (90% is the threshold).

This pull request will bring the total coverage in the repository to 94.5% (0.0% change).

View more on Code Climate.

@kylemh
Copy link
Member

kylemh commented Jun 15, 2020

That was super helpful thanks! I had NODE_ENV and BABEL_ENV set to test in the environment variables. I got rid of those and we're flying! Woohoo!

@kylemh kylemh merged commit d441493 into OperationCode:master Jun 15, 2020
@kylemh
Copy link
Member

kylemh commented Jun 15, 2020

What was the reasoning for going specifically to version 5.1.11 as opposed to something more recent?

@chynh
Copy link
Contributor Author

chynh commented Jun 16, 2020

What was the reasoning for going specifically to version 5.1.11 as opposed to something more recent?

I initially tried version 5.2.x but had issue with Accordion component. I don’t know what changed but Storybook seemed to have issue with passing JSX as prop. Might be something to keep in mind when upgrading in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate to newest version of Storybook
2 participants