Skip to content

refactor: remove unused dependencies #1905

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

refactor: remove unused dependencies #1905

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bee024f
refactor: remove unused dependencies
visualfanatic Oct 31, 2021
258c563
Merge branch 'alpha' into remove-unused-deps
mtrezza Oct 31, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 0 additions & 45 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 1 addition & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@
"react-router": "5.2.1",
"react-router-dom": "5.3.0",
"regenerator-runtime": "0.13.8",
"semver": "7.3.4",
"typescript": "4.4.3"
Copy link
Member

Choose a reason for hiding this comment

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

typescript is needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mtrezza could you specify what is using typescript? I checked the repo and there is no configuration for TS, there are no .tsx? files and no build set up for TS 🤔

Copy link
Member

@mtrezza mtrezza Oct 31, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

madge uses it; even if you don't see any reference for a dependency in the code base, another dependency may require it based on its configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mtrezza I checked the report madge generates and everything works.

Screen Shot 2021-11-01 at 16 28 49

even if you don't see any reference for a dependency in the code base, another dependency may require it based on its configuration.

Yeah, I get that, I wouldn't try to send the PR otherwise. I checked peer deps of other packages before to be sure if TS is really not required.

Copy link
Member

@mtrezza mtrezza Nov 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

image

It's a dependency N levels deep, you'd have look at package-lock.json from madge to see that, you won't see that in its package.json. I'm assuming it's configuration-dependent in madge, so it's probably not used in all cases, that's why it may be not a fixed dependency of the madge package.

"semver": "7.3.4"
},
"devDependencies": {
"@actions/core": "1.2.6",
Expand All @@ -91,7 +90,6 @@
"all-node-versions": "8.0.0",
"babel-eslint": "10.1.0",
"babel-loader": "8.1.0",
"babel-plugin-transform-object-rest-spread": "6.26.0",
"core-js": "3.6.5",
"cross-env": "7.0.2",
"css-loader": "3.5.2",
Expand Down