Skip to content
New issue

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

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

Already on GitHub? # to your account

chore(deps): Remove unused dependencies #415

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

WillGibson
Copy link
Collaborator

It occurred to me that we could reduced security risk by removing unused dependencies...

➜ npx depcheck       
Unused dependencies
* chalk
* commander
* needle
* proxy-agent
* xmlbuilder2

Some of those were false positives because, like Stryker, depcheck does not check markdown-link-check because it has no file extension.

@@ -38,17 +38,12 @@
"commander": "^13.1.0",
"link-check": "^5.4.0",
"markdown-link-extractor": "^4.0.2",
"needle": "^3.3.1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I only found that xmlbuilder2 is not used in this project. The others are used https://github.com/tcort/markdown-link-check/blob/master/markdown-link-check

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will double check later.

I was running the following after removing each to check for no apparent change in behaviour...

rm -rf package-lock.json node_modules && npm install && npm run test && ./markdown-link-check --verbose --progress test

I think this is perhaps adding to the case for more comprehensive test coverage.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just had a look to the package-lock.json and for example the removal of needle works because this package is also needed by the link-check dependency

"needle": "^3.3.1",
so it's installed and available for the markdown-link-check but if we stop using needle for link-check we'll don't have that package available anymore in the current project

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I forgot about the extensionless markdown-link-check file and filtered by .js extension when searching the code 🙄

Probably should explicitly include needle as we use it directly. Same for proxy-agent.

I have restored them.

"eslint": "^9.14.0",
"expect.js": "^0.3.1",
"express": "^4.21.1",
"globals": "^15.12.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

used in

export default [
but maybe it comes with the eslint import

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that comes with eslint/eslintrc.

Copy link
Contributor

Choose a reason for hiding this comment

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

import globals from "globals";

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

},
"devDependencies": {
"@eslint/js": "^9.14.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Used

import js from "@eslint/js";

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@nschonni nschonni Feb 28, 2025

Choose a reason for hiding this comment

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

For now it is, but it's also recommended to separately install it as part of the docs https://eslint.org/docs/latest/use/getting-started#manual-set-up

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, I've added that back.

"eslint": "^9.14.0",
"expect.js": "^0.3.1",
"express": "^4.21.1",
"globals": "^15.12.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

import globals from "globals";

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants