Add acorn dependency to fix npm warning #666
Conversation
It seems there is a problem with npm installing 'peer dependencies' npm's warning says that we need to install this dependency ourselves. The bug is tracked here: eslint/eslint#11018
govuk-design-system-ci
temporarily deployed
to
govuk-prototype-kit-pr-666
|
Not sure this is the best way to solve this issue since we're not using this dependency at the top level in this project. Do we know that the warnings are causing issues for our users? |
|
yeh people are always worried about the warnings in training and asking if something went wrong. more importantly, this is a necessary dependency that isn't being installed. This is npm's own recommendation for how to fix that. They are apparently working on a fix themselves for npm 6 |
|
I don't really see a downside to this - I think the worst that can happen is in the future standard/eslint doesn't need acorn any more, and we have an extraneous package for a while until we remove it. |
|
I think it would be better to figure out which dependency uses acorn and acorn-jsx and try and fix this issue upstream? |
There was a problem hiding this comment.
I wasn't aware that this was an issue that came up in training, that's good to know.
Found this response from the npm team that suggests this problem wont be solved soon: eslint/eslint#10022 (comment)
necessary dependency that isn't being installed
I'd recommend we raise an issue to track removing this dependency in the future since we don't actually need this dependency to run standard (we don't lint JSX).
|
related to #660 |
joelanman
force-pushed
the
fix-acorn-warning
branch
from
d919d68
to
d04f6c6
Compare
In PR #667 we added `acorn` as a direct dependency to fix complaints from npm about peer dependencies, due to a bug in npm [[1]]. That bug now appears to have been fixed; removing `acorn` from our `package.json` does not result in any peer dependency warnings. Also, as we now ensure that users do not install dev dependencies, we can be confident that they will not need `acorn`, as it is only required by dev dependencies. [1]: #666
In PR #667 we added `acorn` as a direct dependency to fix complaints from npm about peer dependencies, due to a bug in npm [[1]]. That bug now appears to have been fixed; removing `acorn` from our `package.json` does not result in any peer dependency warnings. Also, as we now ensure that users do not install dev dependencies, we can be confident that they will not need `acorn`, as it is only required by dev dependencies. [1]: #666
In commit e6ed4ff we added `acorn` as a direct dependency to fix complaints from npm about peer dependencies, due to a bug in npm [[1]]. That bug now appears to have been fixed; removing `acorn` from our `package.json` does not result in any peer dependency warnings. [1]: alphagov/govuk-prototype-kit#666
It seems there is a problem with npm installing 'peer dependencies'
npm's warning says that we need to install this dependency ourselves.
We are getting this issue via
eslint, viastandardThe bug is tracked here:
eslint/eslint#11018