Storybook: addon-storysource - Prettier transitive dependency collision

[sergiohgz]

Describe the bug
@storybook/[email protected] installs [email protected] because it requires to install ^1.16.4.
After recreating lockfile (using npm), as @storybook/addon-storysource installs its own version of Prettier, users could not update to v2.

To Reproduce
Steps to reproduce the behavior:

1. Add "prettier": "^2.0.0" in package.json
2. Remove package-lock.json and node_modules/
3. Perform npm install
4. Execute npx prettier -v

Expected behavior
After step 4, prettier version should be 2.0.1, but it's 1.19.1

System:

Environment Info:

  System:
    OS: Linux 5.5 Fedora 31 (Workstation Edition) 31 (Workstation Edition)
    CPU: (8) x64 Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
  Binaries:
    Node: 12.16.1 - /usr/local/bin/node
    Yarn: 1.22.0 - /opt/npm-global/bin/yarn
    npm: 6.14.2 - /opt/npm-global/bin/npm
  Browsers:
    Chrome: 80.0.3987.149
    Firefox: 74.0
  npmPackages:
    @storybook/addon-actions: 5.3.17 => 5.3.17 
    @storybook/addon-knobs: 5.3.17 => 5.3.17 
    @storybook/addon-storysource: 5.3.17 => 5.3.17 
    @storybook/addons: 5.3.17 => 5.3.17 
    @storybook/preset-create-react-app: 2.1.0 => 2.1.0 
    @storybook/react: 5.3.17 => 5.3.17 

Additional context
IMO, code should be prettified before running Storybook, but some users could not want this. As prettierConfig is optional, addon could Prettier could ask for Prettier to be installed on-demand (using peerDependencies).

[EvHaus]

Same problem here. Can you please move it to devDependencies instead? This is the offending line: https://github.com/storybookjs/storybook/blob/master/lib/source-loader/package.json#L39

[sergiohgz]

@EvHaus the problem is not having it as devDependency, because it will be used for code formatting in production. This way, should be user decision to install it, so it should be put in peerDependencies block

[shilman]

Why does it matter what version of prettier Storybook chooses to use? NPM supports multiple versions of libraries (that's why your node_modules folder is so huge). I don't understand why a dependency of a Storybook library would restrict you from using a different version in your project.

We're not going to move prettier to a peerDependency because it needlessly complicates setup -- unless I'm missing something about how NPM or prettier works.

[EvHaus]

For us it's because of our monorepo structure. Consider:

• MyProject/
  
  
  
  • package.json (this has prettier)
  
  
  • packages/
    
    
    
    
    
    • SubPackageA/
      
      
      
      
      
      
      
      • package.json (this has storybook)
      
      
      
      
    
    
    
    • SubPackageB/
      
      
      
      
      
      
      
      • package.json
      
      
      
      
    
    
    
  
  

I want both SubPackageA and SubPackageB to use the same shared prettier version and configuration, so it's installed at the root level. However, SubPackageA installs Storybook and it brings prettier along for the ride. Now anything in SubPackageA uses that version of prettier instead of the shared global one that I defined a few directories above.

[shilman]

@EvHaus can you make SubPackageA depend explicitly on prettier v2 (as a devDependency)? That way Storybook's version of prettier will not be hoisted AFAIK.

[sergiohgz]

Hi @shilman,

First of all, sorry if I could sound rude before, the global problem we are living is elevating our anxiety levels and we should not explode with software :wink:. Also, I want to say that Storybook is an awesome project and you (the team) is doing a great job managing it. Now, I'll explain better and change the title to better understanding.

TL;DR jump to third paragraph

I have upgraded to Prettier v2 last monday and I saw that, when recreating lockfiles, v1 binary is installed by some package, in this case, @storybook/addon-storysource. As you said before, there is no need to upgrade to v2 in the addon because it could stay in v1 and the capabilities about how node_modules are managed depends more on the package manager you prefer to use.

The problem I'm dealing with is that I don't install Prettier per project because I have a package that manages linting and code prettifying, providing their own packages, an eslint-config-xxxxx. As Prettier is a transitive dependency installed by both packages, @storybook/addon-storysource and eslint-config-xxxxx, when installing with npm CLI, addon transitive dependency takes precedence over the one installed by ESLint config (this is based on naming). If we install Prettier manually (as dev, of course), the problem disappears, but it's a dependency to maintain and can cause extraneous behaviours with ESLint configuration (we are humans and we forget to align dependencies across pacakges :sweat_smile:). I think that migrating to Yarn could resolve this problem because it have resolutions to use the version we want, but changing prefered dependency manager is not a solution for everyone.

Looking for the features of this addon, I like the ability to see code formatted in a different way on the Storybook than in the source code, but this is a feature that not every user of the addon would use. As this is an optional feature (let me know if I'm not correct or I misunderstood something), my suggestion is moving Prettier as devDependency (for internal use) and peerDependency/optionalDependency for addon users. This way, it can open the door for more:

• Users who don't need/want to prettify for Storybook visualization, don't need to install it, making their node_modules lighter.
• Users who prettify code but don't use prettierConfig option can install the version they need and Storybook won't enable this feature.
• Users who want to prettify code for Storybook must use prettierConfig option and here Storybook can make verification for these cases:
  
  
  
  • If Prettier version doesn't matter, addon can set * for version in peer or optional and provide a message. If there is no Prettier installed, Storybook can ignore prettierConfig or fail fast
  
  
  • If Storybook requires a specific version (v1 by example), addon can set that version as peer or optional. Storybook can show a message when loading addon saying the version required and that it don't support other versions officially, so unexpected behaviours should not be reported. In this case, ignoring is also available, but fail fast is forcing again in a worst way.
    
    I don't know if I'm missing any other case, but we can work in this features, not only for this addon, because there are more that has this same feature (e.g, @storybook/addon-docs)
  
  

Thanks for the great job and the great tool

[shilman]

@sergiohgz no you weren't rude at all. Apologies if I sounded rude or annoyed -- I was just trying to be direct since I feel confident about my response.

Let me try again:

I think prettier can be removed completely as a dependency from storysource. It's dead code from an older version of storysource.

Unfortunately, that doesn't solve the problem. prettier is a dependency of source-loader, a library that storysource uses. NOT a devDependency.

If a package uses another package, it should either be a dependency or a peer dependency. If it's a peer dependency, it places the burden on the user of installing that package. I don't want to put that burden on our users. Storybook is already complex enough to set up.

If your monorepo setup is hoisting things incorrectly that's up to you to solve. It's really not a Storybook issue.

I don't want to specify * because what happens if they upgrade to 3 and break the API? However, if we can verify that prettier 2 works, I'm OK specifying ^1 || ^2 as a compromise.

What do you think? cc @ndelangen

[EvHaus]

@EvHaus can you make SubPackageA depend explicitly on prettier v2 (as a devDependency)? That way Storybook's version of prettier will not be hoisted AFAIK.

Yeah, this was my solution as well. Just sucks to have to now keep the two dependencies in sync (the one in our root and the one in this sub package).

It's really not a Storybook issue.

You're right. I incorrectly assumed you were using prettier as a dev dependency. If it's a true dependency (via source-loader) then... it's unfortunate but I agree that you should just keep things as they are.

[ndelangen]

I don't want to specify * because what happens if they upgrade to 3 and break the API? However, if we can verify that prettier 2 works, I'm OK specifying ^1 || ^2 as a compromise.

I agree

[shilman]

FYI, I believe we've upgraded to prettier 2 in 6.0.

[sergiohgz]

That would be really nice @shilman

[sebarthel89]

I have a similar issue described in #10339. I would love to understand good solutions for it and go forward in this direction with storybook.

[stale[bot]]

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

[stale[bot]]

Hey there, it's me again! I am going close this issue to help our maintainers focus on the current development roadmap instead. If the issue mentioned is still a concern, please open a new ticket and mention this old one. Cheers and thanks for using Storybook!