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

prettier should be a dev dep #14714

Closed
michaelfarrell76 opened this issue Apr 26, 2021 · 6 comments
Closed

prettier should be a dev dep #14714

michaelfarrell76 opened this issue Apr 26, 2021 · 6 comments

Comments

@michaelfarrell76
Copy link

michaelfarrell76 commented Apr 26, 2021

Describe the bug
In the following: prettier is a regular dependency when it should be a devDependency

  • "@storybook/addon-docs@npm:6.2.9":
  • "@storybook/codemod@npm:6.2.9":
  • "@storybook/source-loader@npm:6.2.9":
@shilman
Copy link
Member

shilman commented Apr 26, 2021

Why should prettier be a dev dep? Storybook directly depends on it to function.

@JanJakes
Copy link
Contributor

Having prettier as a dependency indeed can cause problems when your own projects wants to use prettier as well (which I guess is often the case). We use prettier 2.3.1 in our monorepo in the workspace root but we've been seeing random small differences in formatting when used by different devs or in the CI pipeline.

After some investigation we realized this was caused by the Storybook's prettier sometimes being loaded instead of "our" prettier (which can indeed happen with NPM and Yarn v1). We had to use the resolutions field to enforce only one version of prettier but now we're getting warning Resolution field "prettier@2.3.1" is incompatible with requested version "prettier@~2.2.1".

I think there are two possible ways to solve this:

  1. The very quick & easy one is to use ^2.2.1 instead of ~2.2.1 in Storybook packages. I think this should cause no problems as prettier almost never changes its config and API.
  2. Make prettier a peer dependency (but then the question is how to handle a missing install plus point 1) still stands here, especially with NPM 7 that installs peer deps automatically).

TL;DR: Could Storybook require prettier ^2.2.1 instead of ~2.2.1?

@shilman
Copy link
Member

shilman commented Jun 16, 2021

@ndelangen do you know why we're using "~2.2.1" instead of "^2.2.1"? you changed it last.

@ndelangen
Copy link
Member

I do not see a reason for this at all. Let's upgrade!
@JanJakes would you be able to open a PR upgrading us to the more lenient version-range?

@michaelfarrell76
Copy link
Author

michaelfarrell76 commented Jul 31, 2021

ah i didnt realize storybook depends on prettier to function. i thought prettier was just a dev dependency used to format the code before it's pushed to git, not something that is required to actually run the storybook dependencies

@shilman
Copy link
Member

shilman commented Aug 3, 2021

Jiminy cricket!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.4.0-alpha.23 containing PR #15298 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

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

No branches or pull requests

4 participants