Deprecate helper-npm-install-peer-deps

[simonplend]

Projects should have a dependency on packages in their package.json that are peerDependencies of other packages that the project has a dependency on. By auto-magically installing packages that npm ls flags as missing peer dependencies, we're masking the problem of incomplete package.json files and discouraging developers from improving their understanding of peer dependencies.

To remedy this, I propose that we:

• Update n-gage dependencies for eslint and stylelint
• Generate build state artifact that lists missing peer dependencies (helper-generate-build-state-artifacts)
• Update helper-npm-install-peer-deps so it no longer installs missing peer dependencies, but refers to the new build state artifact
• Update config templates in n-circle2-cli so they no longer call the helper-npm-install-peer-deps

[adgad]

This sounds good to me.

Am I right in thinking that most projects currently aren't installing peer deps from this hack (i.e. is ajv actually still a thing?). If it is, perhaps we should fix the common ones in n-ui or something, so for most apps it's seamless?

[simonplend]

The peer deps helper was installing any missing peer deps and then npm update was run in the next step, which was removing the peer deps that were just installed as they were extraneous. I fixed that in our CircleCI config templates here so that these extraneous packages are left alone: https://github.com/Financial-Times/n-circle2-cli/pull/11. The work I'm proposing above is a follow on from this.

Regarding ajv I saw the conflict yesterday on next-article because it was depending on the latest major version of ajv (presumably as an attempt to fix a peer dep issue in the past), but n-gage depends on old major versions of eslintrc and stylelint, which creates the following issue...

`n-gage` deps are quite out-of-date, at least for `stylelint` and `eslint`

and the old versions of those deps rely on old versions of other packages, and are sometimes pinned to a specific version e.g. https://github.com/eslint/eslint/blob/v4.19.1/package.json#L73

table": "4.0.2"

and that version of `table` depends on older versions of `ajv-keywords` and `ajv`

specifically `ajv@^5.2.3`

and `next-article` is specifying `ajv@^6.5.2` in its dependencies

so when the old version of `ajv-keywords` that `table@4.0.2` gets hoisted to the top of `node_modules` it doesn't have `ajv@^5.x.x` available as a peer dependency, because `ajv@^6.5.2` is what's installed there

(from a Slack chat)

Fixed by removing ajv as a dep for next-article: https://github.com/Financial-Times/next-article/pull/2927/commits/f740cbbfac84c20e64a3761f986b832f6c52fca8

[adgad]

Okay that sounds good. I guess I was mainly checking that we weren't going to suddenly break hundreds of builds. If it's just the odd couple that have to fix their deps that's fine.

[simonplend]

I think if I upgrade eslint and stylelint for n-gage then we hopefully shouldn't run into too many ajv dependency version conflicts on apps 🤞

After running npm install for n-ui:

npm WARN ajv-keywords@3.2.0 requires a peer of ajv@^6.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN karma-sinon-chai@1.3.4 requires a peer of sinon@>=2.1.0 <5 but none is installed. You must install peer dependencies yourself. << IGNORING THIS ONE FOR NOW

$ npm ls ajv-keywords
@financial-times/n-ui@0.0.0 /home/simonplend/dev/work/clients/financial-times/n-ui
├─┬ @financial-times/n-gage@2.0.4
│ └─┬ eslint@4.19.1
│   └─┬ table@4.0.2
│     └── ajv-keywords@2.1.1 
└─┬ webpack@4.16.5
  ├── ajv-keywords@3.2.0 
  └─┬ schema-utils@0.4.7
    └── ajv-keywords@3.2.0  deduped

$ npm ls ajv
@financial-times/n-ui@0.0.0 /home/simonplend/dev/work/clients/financial-times/n-ui
├─┬ @financial-times/n-gage@2.0.4
│ └─┬ eslint@4.19.1
│   ├── ajv@5.5.2 
│   └─┬ table@4.0.2
│     └── ajv@5.5.2  deduped
├─┬ @financial-times/n-test@1.9.0
│ └─┬ wdio-sauce-service@0.4.10
│   └─┬ request@2.83.0
│     └─┬ har-validator@5.0.3
│       └── ajv@5.5.2  deduped
├─┬ coveralls@3.0.2
│ └─┬ request@2.88.0
│   └─┬ har-validator@5.1.0
│     └── ajv@5.5.2  deduped
└─┬ node-sass@4.9.3
  └─┬ request@2.87.0
    └─┬ har-validator@5.0.3
      └── ajv@5.5.2  deduped

$ grep 'version' node_modules/ajv/package.json 
  "version": "5.5.2"

$ grep 'version' node_modules/ajv-keywords/package.json 
  "version": "3.2.0"

[simonplend]

Yeah, I'm pretty sure this won't break hundreds of builds, but that's why I want to take it one step at a time to see how things look before fully deprecating the helper-npm-install-peer-deps 😄