Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Remove unused dependencies #339

Merged
merged 3 commits into from
Oct 3, 2017
Merged

Remove unused dependencies #339

merged 3 commits into from
Oct 3, 2017

Conversation

edmorley
Copy link
Member

@edmorley edmorley commented Oct 2, 2017

1) Moves preset-airbnb-base dependencies out of middleware-eslint

The eslint-config-airbnb-base and eslint-plugin-import packages are not used by neutrino-middleware-eslint itself, but instead by neutrino-preset-airbnb-base.

2) Removes unused dependencies

  • webpack-dev-middleware can be removed from the karma preset, since it's only used by karma-webpack which already has it as a dependency.
  • webpack-dev-server can be removed from the web preset since it's a leftover from when neutrino-middleware-dev-server was created.
  • the web preset's core-js and babel-polyfill dependencies are leftover from Upgrading Webpack to v3, merge config as middleware #315.
  • everything else was manually determined to be unused via correlating the package list against greps of the source, plus checking that the package wasn't a peer dependency of another listed package.

The yarn.lock comment changes are because I'm using an up to date version of yarn that no longer adds the version numbers to the lockfile (which aims to reduce lock file churn due to version differences). Hopefully shortly we'll be on a newer yarn everywhere, thereby avoiding this.

The `eslint-config-airbnb-base` and `eslint-plugin-import` packages
are not used by `neutrino-middleware-eslint` itself, but instead by
`neutrino-preset-airbnb-base`.
Of note:
* `webpack-dev-middleware` can be removed from the karma preset,
  since it's only used by `karma-webpack` which already has it as a
  dependency.
* `webpack-dev-server` can be removed from the web preset since it's
  a leftover from when `neutrino-middleware-dev-server` was created.
* the web preset's `core-js` and `babel-polyfill` dependencies are
  leftover from #315.

Everything else was manually determined to be unused via correlating
the package list against greps of the source, plus checking that the
package wasn't a peer dependency of another listed package.
"fluture": "^7.1.3",
"lodash.clonedeep": "^4.5.0",
"ramda": "^0.24.1"
},
"devDependencies": {
"eslint-config-airbnb-base": "^12.0.1"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the past the reason this devDependency existed had to do with local development and linking. ESLint didn't like to resolve where this package existed for the Airbnb preset when you linked it, probably due to the nested node_modules structure. My guess is that problem still exists, but can probably be fixed if we move to yarn workspaces and hoisted deps.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree there is a bug in yarn where it's over-eager to complain about "missing" peer deps (I'm going to file that separately). However in this case, the devDependency was actually in the wrong package - it's airbnb base that's using it :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's what I mean. When you link neutrino-preset-airbnb-base, ESLint couldn't resolve eslint-config-airbnb-base because the eslint binary lives in neutrino-middleware-eslint but it was trying to use a working directory relative to neutrino-preset-airbnb-base.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah that makes sense. I thought you meant a related bug that I ran into, where yarn outputs "missing peer" for a case that npm doesn't.

I'm looking forward to yarn workspaces working well and all of this going away! :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me too Ed, me too.

Copy link
Member

@eliperelman eliperelman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another excellent PR. Thanks!

@eliperelman eliperelman merged commit af4c8ab into neutrinojs:master Oct 3, 2017
@eliperelman
Copy link
Member

Released in v7.0.2.

@edmorley edmorley deleted the rm-unused-dependencies branch October 3, 2017 14:14
@eliperelman
Copy link
Member

For major versions it would definitely matter, for patches and minors probably not, but this is definitely wrong. Maybe it's a race condition between the publish of dependencies and the updating of them in package.json.

@edmorley
Copy link
Member Author

I thought you meant a related bug that I ran into, where yarn outputs "missing peer" for a case that npm doesn't.

I've filed yarnpkg/yarn#4675 for this other case.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants