Should @storybook/react depend on `ajv`/`ajv-keywords`?

[majapw]

Issue details

Hi all,

I've been running through a pretty interesting dependency issue that I'm trying to figure out how best to mitigate. For context I am on:
npm@5.6.0
node@8.9.1

I have storybook as a dev dependency of my package as well as a whole lot of other things. Notably, storybook brings in webpack@^3.11.0 and postcss-loader@^2.1.0=>schema-utils@^0.4.4 ... both of which bring in ajv@6/ajv-keywords@3 as of a few days ago.

These changes down the tree made my app's npm ls (and therefore my tests) all start erroring out with no code changes.

I have a basic repro case here: https://github.com/majapw/ajv-deps-test

Basically if you install eslint at the latest (which pulls in ajv@5) and @storybook/react at the latest (which pulls in ajv@6) you get an error when you run npm ls due to peer dep incompatibility. I think that if storybook depended directly on ajv@6, this would mitigated. I could be wrong though and am open to other possibilities. Thoughts?

@backwardok @ljharb @gabergg

Steps to reproduce

git clone git@github.com:majapw/ajv-deps-test.git
npm install
npm ls

Please specify which version of Storybook and optionally any affected addons that you're running

@storybook/react@3.3.13

Affected platforms

react

[Hypnosphi]

This part looks really weird, given that 6.1.1 matches ^6.0.0 mask:

UNMET PEER DEPENDENCY ajv@6.1.1

Iooks like an npm bug to me

[backwardok]

@Hypnosphi it's unmet because eslint requires ^5.3.0, not because storybook requires ^6.0.0

[majapw]

@Hypnosphi I think it's mostly that the error messages are pretty... unclear. But it seems to be that the lack of an ajv declaration at the top of that tree means there's some confusion as to what ajv package to use... and in this case, there are two.

[ljharb]

Specifically, if any of the direct deps here specifies a peer dependency that storybook/react does not explicitly declare as a dep, then that peer dep is an implicit, but required part of storybook/react’s API.

What happens if you cd into that package, npm install, and then run npm ls?

[Hypnosphi]

@backwardok why does it write 6.1.1 not 5.3.0 then

@ljharb ajv-keywords isn't our direct dependency, so it should be responsibility of the packages that depend on it

But actually, as far as I see, all the packages in the tree that depend on ajv-keywords@3 also depend on its peer dep ajv@6. You can verify it by inspecting corresponding package.json files

[Hypnosphi]

Probably relevant: npm/npm#19483

[backwardok]

@Hypnosphi because it installed 5.3.0 and then didn't install 6.1.1, which results in the unmet dependency of ^6.0.0

[Hypnosphi]

didn't install 6.1.1

@backwardok in fact it did (at least for me with npm@5.5.1)

[Hypnosphi]

$ cat node_modules/webpack/node_modules/ajv/package.json
{
  ...
  "version": "6.1.1"
}

[majapw]

Yeah it's weird because i seem to have both in my tree.

➜  ajv-deps-test git:(master) cat node_modules/ajv/package.json| grep \"version\"
  "version": "5.5.2"
➜  ajv-deps-test git:(master) cat node_modules/webpack/node_modules/ajv/package.json | grep \"version\" 
  "version": "6.1.1"

[majapw]

My basic example has many copies of ajv:

node_modules/webpack/node_modules/ajv/package.json
  "version": "6.1.1"
node_modules/uglifyjs-webpack-plugin/node_modules/ajv/package.json
 "version": "6.1.1"
node_modules/postcss-loader/node_modules/ajv/package.json
  "version": "6.1.1"
node_modules/ajv/package.json
"version": "5.5.2"
node_modules/fsevents/node_modules/ajv/package.json
"version": "4.11.8"

[majapw]

The bug is an npm bug! Woooo!

eslint/eslint#10022 (comment)

Closing this. :/