Skip to content
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

Remove several unused JS dependencies #7521

Merged
merged 5 commits into from
Jan 9, 2024
Merged

Conversation

hotzenklotz
Copy link
Member

@hotzenklotz hotzenklotz commented Jan 3, 2024

While debugging the library version conflicts for PR #7505 I ran a check for unused libs with depcheck. While it flagged many false positive libs I think it correctly identified these 5 libs as unused:

[Name]: [PR where the lib was introduced]

Hopefully this decreases our bundle size a little bit and helps to avoid version conflicts in the future.

Command to detect unused libs.

npx depcheck . --specials=eslint,webpack,prettier,typescript,less,esbuild

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • Delete node_modules and yarn install
  • I did tests to verify that:
    • That throwing an Error will trigger a Airbrake notifications
    • That quick select works in AI and non-AI mode
    • That VX logs can still be viewed
    • That skeletons & segment tree components looked "normal"

Issues:

  • None

(Please delete unneeded items, merge only when none are left open)

@@ -166,7 +165,6 @@
"classnames": "^2.2.5",
"color-hash": "^2.0.1",
"comlink": "^4.3.0",
"cross-fetch": "^3.1.5",
Copy link
Member Author

Choose a reason for hiding this comment

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

This lib was introduced to added a polyfill for airbrake-js. We have switched to @airbrake/browser in the meantime, which seems to work without polyfills.

@hotzenklotz hotzenklotz marked this pull request as ready for review January 3, 2024 14:38
@hotzenklotz
Copy link
Member Author

@philippotto @daniel-wer I already tried to verify as much as possible that we really do not use these libs anywhere in our code or tooling. Yet, I didn't remember why we added some of this in the first-place or why it has become unused. Maybe some of the lib names mean more to you and you can verify that this is safe to remove. Thanks.

@hotzenklotz hotzenklotz self-assigned this Jan 3, 2024
@@ -154,7 +154,6 @@
"@types/pixelmatch": "^5.2.4",
"@types/pngjs": "^6.0.1",
"@types/three": "^0.142.0",
"@use-it/interval": "^1.0.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

We have implemented this ourselves in libs/react_helpers

@philippotto
Copy link
Member

I didn't test this PR (your performed testing looks very good 👍), but your explanations sound very reasonable and I agree that these packages seem to be unused.

Hopefully this decreases our bundle size a little bit [...]

I would have hoped that webpack wouldn't include unused bits, but removing unused packages is still a good idea, obviously.

@hotzenklotz
Copy link
Member Author

@daniel-wer Do these libs mean anything to you? Do you see any red flags?

@daniel-wer
Copy link
Member

@daniel-wer Do these libs mean anything to you? Do you see any red flags?

I don't see any red flags. Thanks for the detailed description and researching where these libs were introduced!

Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

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

cool, then, let's do it 🎉

@bulldozer-boy bulldozer-boy bot merged commit 82edc9c into master Jan 9, 2024
2 checks passed
@bulldozer-boy bulldozer-boy bot deleted the remove-unused-deps branch January 9, 2024 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants