Pnpm: Missing typescript peer dependency in a Vite/JS project

[shilman]

Running

pnpm create vite@latest
pnpx sb@next init --builder=vite
pnpm run storybook

Yields

 WARN  Issues with peer dependencies found
.
├─┬ @storybook/addon-essentials 7.0.0-beta.2
│ └─┬ @storybook/addon-controls 7.0.0-beta.2
│   └─┬ @storybook/blocks 7.0.0-beta.2
│     └─┬ @storybook/docs-tools 7.0.0-beta.2
│       └─┬ @storybook/core-common 7.0.0-beta.2
│         └── ✕ missing peer typescript@"*"
└─┬ @storybook/vue3-vite 7.0.0-beta.2
  ├─┬ @storybook/builder-vite 7.0.0-beta.2
  │ └─┬ @joshwooding/vite-plugin-react-docgen-typescript 0.0.5
  │   ├── ✕ missing peer typescript@">= 4.3.x"
  │   └─┬ react-docgen-typescript 2.2.2
  │     └── ✕ missing peer typescript@">= 4.3.x"
  └─┬ @storybook/core-server 7.0.0-beta.2
    └── ✕ missing peer typescript@"*"
Peer dependencies that should be installed:
  typescript@>=4.3.0

Originally posted by @shilman in #19821 (comment)

[shilman]

Make this optional!

[IanVS]

The typescript issue is tricky. One reason we're seeing it is that the typescript docgen plugin is used by default, so it's always installed, and just not used in some cases (e.g. when the project isn't using typescript). But it has a peer dependency on typescript, causing this warning. I've talked with @joshwooding about it in the past, and we're not sure how we can solve this.

[ndelangen]

@IanVS if builder-vite, had a peerDependency on typescript, AND that peerDependency was optional.. would that fix the issue?

[IanVS]

I'm not sure, and I don't really know how to test the theory... I'd be surprised if it works, but stranger things have happened.

[shilman]

Yo-ho-ho!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-beta.7 containing PR #20231 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Closing this issue. Please re-open if you think there's still more to do.

[IanVS]

No dice.

 WARN  Issues with peer dependencies found
.
├─┬ @storybook/react 7.0.0-beta.7
│ └── ✕ missing peer typescript@"*"
└─┬ @storybook/react-vite 7.0.0-beta.7
  ├─┬ @joshwooding/vite-plugin-react-docgen-typescript 0.0.5
  │ ├── ✕ missing peer typescript@">= 4.3.x"
  │ └─┬ react-docgen-typescript 2.2.2
  │   └── ✕ missing peer typescript@">= 4.3.x"
  └─┬ @storybook/builder-vite 7.0.0-beta.7
    ├── ✕ missing peer typescript@">= 4.3.x"
    └─┬ @joshwooding/vite-plugin-react-docgen-typescript 0.0.5
      └── ✕ missing peer typescript@">= 4.3.x"
Peer dependencies that should be installed:
  typescript@>=4.3.0

[ndelangen]

ok, so

• @storybook/react should NOT have a peerDep on typescript
• @joshwooding/vite-plugin-react-docgen-typescript should have it's peerDep to typescript become optional? @joshwooding, do you recon that's possible? I could possibly open a PR, if that help? 🙏

[IanVS]

The tricky thing about that is that the plugin won't work if typescript isn't installed (in theory it's useful for more than just storybook), so it's not really optional as far as that package is concerned...

Maybe it needs to be a dependency instead? Even though it sucks for non-typescript storybook projects to have to include typescript in their node_modules.

But maybe it's fine to make optional, and maybe add an error message if it's not installed when it tries to run?

[ndelangen]

It could do a runtime check? And do a no-op if it's not there?

I understand that that's not great from a single unit perspective, but would make it usable in the larger ecosystem

I could make a fork if that's what it takes?

We need A strategy, right now we have none.

[ndelangen]

I opened a PR joshwooding/vite-plugin-react-docgen-typescript#8

I'm not sure the code is good, I'm not sure anyone will like it, but it does proof it's technically possible, to make the plugin depend on typescript optionally and not fail hard when it's missing.

[ndelangen]

I'll re-open this issue, to track the progress on the vite-builder > typescript peer dep problem

[shilman]

Ooh-la-la!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-beta.13 containing PR #20328 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Closing this issue. Please re-open if you think there's still more to do.

[robin-thomas]

Getting the below warning with v7.0.0-beta.13 release, when I do npm install.

npm WARN ERESOLVE overriding peer dependency
npm WARN While resolving: @joshwooding/vite-plugin-react-docgen-typescript@0.0.0-20221218231544
npm WARN Found: vite@4.0.2
npm WARN node_modules/vite
npm WARN   dev vite@"^4.0.1" from the root project
npm WARN   5 more (@storybook/builder-vite, @storybook/react-vite, ...)
npm WARN 
npm WARN Could not resolve dependency:
npm WARN peer vite@"^3.0.0" from @joshwooding/vite-plugin-react-docgen-typescript@0.0.0-20221218231544
npm WARN node_modules/@joshwooding/vite-plugin-react-docgen-typescript
npm WARN   @joshwooding/vite-plugin-react-docgen-typescript@"0.0.0-20221218231544" from @storybook/react-vite@7.0.0-beta.13
npm WARN   node_modules/@storybook/react-vite
npm WARN 
npm WARN Conflicting peer dependency: vite@3.2.5
npm WARN node_modules/vite
npm WARN   peer vite@"^3.0.0" from @joshwooding/vite-plugin-react-docgen-typescript@0.0.0-20221218231544
npm WARN   node_modules/@joshwooding/vite-plugin-react-docgen-typescript
npm WARN     @joshwooding/vite-plugin-react-docgen-typescript@"0.0.0-20221218231544" from @storybook/react-vite@7.0.0-beta.13
npm WARN     node_modules/@storybook/react-vite

Didn't have this error in v7.0.0-beta.12 release

[ndelangen]

@robin-thomas you're right, I'll open a PR today that should fix that. All that should be required is upgrade storybook to the latest release of @joshwooding/vite-plugin-react-docgen-typescript.

[robin-thomas]

@ndelangen Any idea when the fix will be released?

[IanVS]

@robin-thomas if this is causing you issues, you should be able to use an override to control the version of @joshwooding/vite-plugin-react-docgen-typescript in your package.json, until this is released. Betas are normally released every few days, but with the holidays it might take a bit longer.

[shilman]

I didn't do a release yesterday due to a bad release of babel per #20382. hopefully we can get that resolved today

[ndelangen]

Babel issue is resolved! Go go go @shilman

[shilman]

ZOMG!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-beta.14 containing PR #20359 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Closing this issue. Please re-open if you think there's still more to do.

[ermik]

@shilman while this and even this is hot — here's another concerning peer:

WARN  Issues with peer dependencies found
projects/extension
└─┬ @storybook/preact-webpack5 7.0.0-beta.14
  ├── ✕ missing peer @babel/core@"*"
  ├─┬ @storybook/builder-webpack5 7.0.0-beta.14
  │ ├── ✕ missing peer typescript@"*"
  │ └─┬ fork-ts-checker-webpack-plugin 7.2.14
  │   └── ✕ missing peer typescript@>3.6.0
  └─┬ @storybook/preset-preact-webpack 7.0.0-beta.14
    ├── ✕ missing peer @babel/core@"*"
    └─┬ @babel/plugin-transform-react-jsx 7.20.7
      ├── ✕ missing peer @babel/core@^7.0.0-0
      └─┬ @babel/plugin-syntax-jsx 7.18.6
        └── ✕ missing peer @babel/core@^7.0.0-0
Peer dependencies that should be installed:
  @babel/core@">=7.0.0 <8.0.0"  typescript@>3.6.0

I might need Babel at some point (currently just using esbuild tsx loader, works great), but I definitely don't need fork because I am not typechecking in SB compiles.

[ndelangen]

Looks like we need to add
@babel/core@"*" to @storybook/preset-preact-webpack

Do a similar change to make fork-ts-checker-webpack-plugin have a optional dependency on typescript.

[ermik]

FWIW at the time of testing beta.14 I inherited Typescript from my monorepo root, but that's not a conventional behavior and isn't actually supported in PNPM yet. I am changing the actual project's package.json to correctly list it as a direct dependency (of my code).

I do still agree with optionality of typescript in the case of fork-ts-checker-webpack-plugin, but It may need to be broader than that — if the the plugin is not used, neither the plugin itself nor any of its dependencies should be included. 👍🏻

[IanVS]

I think the challenge is that storybook wants to support TypeScript "out of the box", without requiring users to install something else for it to work. This means that it has to include packages like this.

[ndelangen]

@ermik I wish there was a way to determine:

if the the plugin is not used

But that's as @IanVS also explains... configurable.. we'll only know we need it at runtime, and therefore we must have a dependency on it.

[ermik]

Very valid points, I didn't think of that!

Storybook is very keen on providing a guided startup experience, and that's where a user can specify whether or not they would like to use fork-ts-checker as part of their storybook compilation ("Would you like?... Y/N"). As for typescript — if they are developing a project using Typescript — they will likely have a dependency on it, so it's not anything to worry about.

However I do understand if this is a subjective decision and these ideas do not fit within that.

[IanVS]

I think it would indeed be worth thinking through the overall installation process for storybook. For example, we could ask some questions about whether or not people want docs or tests or typescript, etc. And depending on the answers, we could potentially avoid installing a lot of dependencies. But that's something to think about after 7.0 is released. 😅

[IanVS]

Do a similar change to make fork-ts-checker-webpack-plugin have a optional dependency on typescript.

I'd be surprised if they're willing to make that kind of change, it's a much more established project, and it doesn't really make sense for it to make its dependency optional. Forking it just to make that change seems like the wrong approach too, from a maintenance burden perspective. I wonder if we just need to tell folks that this is a peer dep warning that they can safely ignore if they're not using typescript.

[ndelangen]

I'm wondering if there's some thing we could set up maybe similar to @storybook/expect that makes the dependency optional. I do very much get the fear of maintenance on this.

[ndelangen]

I agree with @IanVS forking that package is a no-go.

But that's a webpack-builder issue, so I'm guessing for vite, (what this specific issue is about) it's solved.