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

Make storybook configurable in a redwood app #1828

Merged
merged 6 commits into from
Mar 5, 2021
Merged

Conversation

jtoar
Copy link
Contributor

@jtoar jtoar commented Feb 21, 2021

This PR makes storybook's main.js file configurable from a Redwood App by copying the strategy we currently use with webpack. The webpack-merge docs say that the merge function works with more than just webpack configs, so I reused it here. Here's the quote from the docs:

This behavior is particularly useful in configuring webpack although it has uses beyond it. Whenever you need to merge configuration objects, webpack-merge can come in handy.

This PR doesn't do anything for storybook/preview.js yet—I know that's what #989 was mainly about. So a question while we're on that topic:

  • should we just have one storybook.config.js file in a redwood app, with a bunch of exports (one for main, one for preview, etc) or a storybook config dir, like we have in core?&
    • answer: two files, one suffixed with .config.js, the other with .preview.js

Finally, here's an unimpressive screenshot just to show that things are working:

image

The file in the redwood app looks like:

// web/config/storybook.config.js

module.exports = {
  addons: ['@storybook/addon-a11y'],
}

@jtoar jtoar added kind/improvement topic/config Babel, Webpack, ESLint, Prettier, etc. topic/storybook labels Feb 21, 2021
@jtoar jtoar requested a review from peterp February 21, 2021 20:45
@jtoar jtoar added this to In progress in Storybook via automation Feb 21, 2021
@jtoar jtoar added this to In progress in Accessibility via automation Feb 21, 2021
@jtoar jtoar self-assigned this Feb 21, 2021
@github-actions
Copy link

github-actions bot commented Feb 21, 2021

📦 PR Packages

Click to Show Package Download Links

https://rw-pr-redwoodjs-com.s3.amazonaws.com/1828/create-redwood-app-0.26.2-54406e6.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1828/redwoodjs-api-0.26.2-54406e6.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1828/redwoodjs-api-server-0.26.2-54406e6.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1828/redwoodjs-auth-0.26.2-54406e6.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1828/redwoodjs-cli-0.26.2-54406e6.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1828/redwoodjs-core-0.26.2-54406e6.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1828/redwoodjs-dev-server-0.26.2-54406e6.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1828/redwoodjs-eslint-config-0.26.2-54406e6.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1828/redwoodjs-eslint-plugin-redwood-0.26.2-54406e6.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1828/redwoodjs-forms-0.26.2-54406e6.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1828/redwoodjs-internal-0.26.2-54406e6.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1828/redwoodjs-prerender-0.26.2-54406e6.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1828/redwoodjs-router-0.26.2-54406e6.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1828/redwoodjs-structure-0.26.2-54406e6.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1828/redwoodjs-testing-0.26.2-54406e6.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1828/redwoodjs-web-0.26.2-54406e6.tgz

Install this PR by running yarn rw upgrade --pr 1828:0.26.2-54406e6

@peterp
Copy link
Contributor

peterp commented Feb 22, 2021

@jtoar I think it makes sense to have two separate configuration files, since the one has a Node.js context and the other has a browser context. I don't think they have to be in a directory, so maybe just config/storybook.preview.js and config/storybook.config.js

Copy link
Contributor

@peterp peterp left a comment

Choose a reason for hiding this comment

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

Here's my comment from a few days ago, I wasn't sure if you saw it.

I do not think that we need to do storybook.preview.js right now. If you agree then we can merge this and add storybook.preview.js seperately.

@jtoar I think it makes sense to have two separate configuration files, since the one has a Node.js context and the other has a browser context. I don't think they have to be in a directory, so maybe just config/storybook.preview.js and config/storybook.config.js

@jtoar
Copy link
Contributor Author

jtoar commented Mar 3, 2021

Dang, screwed up the rebasing again; hold!

@jtoar jtoar requested a review from peterp March 3, 2021 07:00
@jtoar
Copy link
Contributor Author

jtoar commented Mar 3, 2021

I do not think that we need to do storybook.preview.js right now. If you agree then we can merge this and add storybook.preview.js seperately.

@peterp totally agree, let's get this one in for the next release if you think it's looking good!

@dac09 dac09 added this to the next release milestone Mar 4, 2021
@cypress
Copy link

cypress bot commented Mar 4, 2021



Test summary

8 0 0 0Flakiness 1


Run details

Project RedwoodJS Framework
Status Passed
Commit 3d2bcb1 ℹ️
Started Mar 5, 2021 11:34 AM
Ended Mar 5, 2021 11:36 AM
Duration 01:38 💡
OS Linux Ubuntu - 16.04
Browser Chrome 88

View run in Cypress Dashboard ➡️


Flakiness

cypress/integration/tutorial/tutorial.js Flakiness
1 The Redwood Tutorial - Golden path edition > 6. Routing Params

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@peterp
Copy link
Contributor

peterp commented Mar 5, 2021

Ship it!

@peterp peterp merged commit 751d342 into main Mar 5, 2021
Accessibility automation moved this from In progress to Done Mar 5, 2021
Storybook automation moved this from In progress to Done Mar 5, 2021
@thedavidprice thedavidprice deleted the ds-make-sb-configurable branch March 5, 2021 16:53
@thedavidprice
Copy link
Contributor

@jtoar thoughts about where/how to document this?

@jtoar
Copy link
Contributor Author

jtoar commented Mar 5, 2021

@thedavidprice time for a Storybook doc I think; I'lll add it as a section there, then link to some of the other docs we have like Mocking GraphQL Requests.

dac09 added a commit to dac09/redwood that referenced this pull request Mar 7, 2021
…-e2e-tests

* 'main' of github.com:redwoodjs/redwood:
  Bump @apollo/client from 3.3.7 to 3.3.11 (redwoodjs#1923)
  Switch create-redwood-app template to TypeScript (redwoodjs#1810)
  Bump @graphql-tools/merge from 6.2.7 to 6.2.10 (redwoodjs#1910)
  Adds resolution to template, to prevent module duplication in web/node_modules (redwoodjs#1933)
  Update service generator to use Prisma namespace (redwoodjs#1853)
  Make storybook configurable in a redwood app  (redwoodjs#1828)
@jangxyz
Copy link
Contributor

jangxyz commented Mar 15, 2021

@peterp 's change of storybook to storybookConfig breaks loading user storybook config in the current release (0.27.1). Currently it's still reading from redwoodPaths.web.storybook. (core/config/storybook/main.js#L64)

Maybe you should apply the change to main.js as well?

@jtoar
Copy link
Contributor Author

jtoar commented Mar 15, 2021

That was my bad, and the fix is here: #2002. Looks ok to you @jangxyz?

@jangxyz
Copy link
Contributor

jangxyz commented Mar 16, 2021

Looks great! 😎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic/config Babel, Webpack, ESLint, Prettier, etc. topic/storybook
Projects
No open projects
Accessibility
  
Done
Storybook
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants