Skip to content

chore: remove unused dependencies #3729

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 7, 2021
Merged

Conversation

jsjoeio
Copy link
Contributor

@jsjoeio jsjoeio commented Jul 6, 2021

This PR removes unused dependencies.

Process

To do this, we use a tool called depcheck. Locally, I ran npx depcheck --ignore-patterns=lib,dist,coverage,contrib which gave the following output:

Unused dependencies
* argon2
* env-paths
* node-fetch
* pem
* safe-buffer
* safe-compare
* tar-stream
* xdg-basedir
Unused devDependencies
* @types/pem
* @types/safe-compare
* @types/tar-stream
* @types/wtfnode
* audit-ci
* doctoc
* eslint-import-resolver-alias
* leaked-handles
* prettier-plugin-sh
* shellcheck
* stylelint
* stylelint-config-recommended
* wtfnode
Missing dependencies
* express-serve-static-core: ./typings/pluginapi.d.ts

Most of these are false-positives. I've identified the ones we no longer used and removed them as part of this PR. I've corrected this list and added more about how we use each dependency:

False positives
* @types/pem
* @types/safe-compare
* @types/tar-stream
* argon2 - used to hash passwords
* doctoc - used to generate table of contents in .md files
* audit-ci - used to identify vulnerabilities as part of CI
* env-paths - used to get environment paths
* pem - used to generate certificates 
* safe-compare - used to check if legacy hash & password are a match
* xdg-basedir - used when determining environment paths
* prettier-plugin-sh - formats shell files with prettier
* shellcheck - used to lint shell files
* stylelint - used for linting
* stylelint-config-recommended - used for linting

Moved Dependencies
* @types/wtfnode
* node-fetch - used as a testing dependency (moved out of root `package.json`) 
* wtfnode - used as a testing dependency (moved out of root `package.json`) 

Removed
* safe-buffer
* tar-stream
* leaked-handles
* eslint-import-resolver-alias

Other
* express-serve-static-core: ./typings/pluginapi.d.ts - found a comment [here](https://github.com/cdr/code-server/blob/main/.eslintrc.yaml#L39) which says "This isn't a real module, just types, which apparently doesn't resolve"

Fixes #3423

@codecov
Copy link

codecov bot commented Jul 6, 2021

Codecov Report

Merging #3729 (0088dd6) into main (6f7ae29) will not change coverage.
The diff coverage is n/a.

❗ Current head 0088dd6 differs from pull request most recent head 0b072e7. Consider uploading reports for the commit 0b072e7 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3729   +/-   ##
=======================================
  Coverage   61.55%   61.55%           
=======================================
  Files          35       35           
  Lines        1813     1813           
  Branches      365      365           
=======================================
  Hits         1116     1116           
  Misses        588      588           
  Partials      109      109           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6f7ae29...0b072e7. Read the comment docs.

@jsjoeio jsjoeio force-pushed the jsjoeio-remove-unused-deps branch from d0f558d to 4e94006 Compare July 6, 2021 22:02
@jsjoeio jsjoeio self-assigned this Jul 6, 2021
@jsjoeio jsjoeio added chore Related to maintenance or clean up dependencies Pull requests that update a dependency file labels Jul 6, 2021
@jsjoeio jsjoeio marked this pull request as ready for review July 6, 2021 23:38
@jsjoeio jsjoeio requested a review from a team as a code owner July 6, 2021 23:38
@jsjoeio jsjoeio force-pushed the jsjoeio-remove-unused-deps branch from 4e94006 to 0b072e7 Compare July 6, 2021 23:38
Copy link
Contributor

@jawnsy jawnsy left a comment

Choose a reason for hiding this comment

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

I always ❤️ this kind of spring cleaning, thanks for doing it! 🧹 And thanks also for making sure our test suite covers enough that we can feel confident doing this, Joe!

@jsjoeio jsjoeio merged commit 7224268 into main Jul 7, 2021
@jsjoeio jsjoeio deleted the jsjoeio-remove-unused-deps branch July 7, 2021 17:20
@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jul 7, 2021

Oh no...I think i broke something by removing eslint-import-resolver-alias

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Related to maintenance or clean up dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check for unused dependencies
2 participants