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 unused deps #1267

Merged
merged 2 commits into from
Jun 28, 2024
Merged

Remove unused deps #1267

merged 2 commits into from
Jun 28, 2024

Conversation

axelboc
Copy link
Contributor

@axelboc axelboc commented Jun 28, 2024

Fix #1242

All in all, depcheck did a good job 😊 β€” I did go over all the deps myself just in case it missed one (it didn't). Do please double check thoroughly.

Unused dependencies

  • dustjs-helpers: used but imported with require; kept 🟒
  • dustjs-linkedin: used but imported with require; kept 🟒
  • everpolate: unused; removed πŸ—‘οΈ
  • history: unused; removed πŸ—‘οΈ
  • jsmpeg: used via ui/src/components/SampleView/jsmpeg.min.js, not via NPM package, which is obsolete compared to jsmpeg.min.js; removed πŸ—‘οΈ
  • liform-react: unused; removed πŸ—‘οΈ
  • plotly.js: peer of react-plotly.js; unused; removed πŸ—‘οΈ
  • popper.js: unused; removed πŸ—‘οΈ
  • react-clipboard.js: unused; removed πŸ—‘οΈ
  • react-plotly.js: unused; removed πŸ—‘οΈ
  • react-router: peer of react-router-dom; used; kept 🟒
  • react-sticky: unused; removed πŸ—‘οΈ
  • shortid: unused; removed πŸ—‘οΈ
  • underscore: unused; removed πŸ—‘οΈ
  • ws: unused; removed πŸ—‘οΈ

Unused devDependencies

  • @testing-library/react: unused but should be πŸ˜‚ ; kept for now 🟒
  • @testing-library/user-event: unused but should be πŸ˜‚ ; kept for now 🟒
  • babel-preset-react-app: direct dep of create-react-app; no need to install ourselves; removed πŸ—‘οΈ
  • concurrently: unused; removed πŸ—‘οΈ
  • less: only one .less file ui/src/components/PeriodicTable/style.less with only a few mixins for CSS browser prefixing; styles should be written in pure CSS; browser prefixing is done automatically by autoprefixer based on browserslists config; removed πŸ—‘οΈ
  • less-watch-compiler: removed πŸ—‘οΈ
  • sass: no SASS/SCSS files found; removed πŸ—‘οΈ
  • wait-on: used in CI to wait for server to start before testing with Cypress; kept 🟒
  • webpack: dependency of webpack-dev-server; safer to keep in case specified version range matters 🟒
  • webpack-dev-server: dependency of react-scripts; safer to keep in case specified version range matters 🟒

Missing dependencies

All of these are direct dependencies of eslint-config-galex, so no need to install them. 🟒

@fabcor-maxiv
Copy link
Contributor

Nice job with the investigation! I will try to give it a check from our side, but there is not much doubt you know this stuff better than we do. : )

@fabcor-maxiv
Copy link
Contributor

Could we get a pre-commit hook or maybe a GitHub Action to make sure it stays clean? Would that make sense?

@fabcor-maxiv fabcor-maxiv linked an issue Jun 28, 2024 that may be closed by this pull request
Base automatically changed from unused-fontawesome to develop June 28, 2024 12:53
@marcus-oscarsson
Copy link
Member

Nice, well done and big thanks !

@axelboc
Copy link
Contributor Author

axelboc commented Jun 28, 2024

Could we get a pre-commit hook or maybe a GitHub Action to make sure it stays clean? Would that make sense?

Hmm, I feel like it would be a bit brittle and not really worth the effort. An unused dependency just adds a few ms to the installation time; nothing worth worrying about too much in my opinion, especially since it doesn't take long to do a bit of a clean-up like this from time to time.

Copy link
Contributor

@fabcor-maxiv fabcor-maxiv left a comment

Choose a reason for hiding this comment

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

I did some sanity check (including build JS UI from scratch), and did not encounter any issue.

@marcus-oscarsson marcus-oscarsson merged commit 8cf30df into develop Jun 28, 2024
13 checks passed
@marcus-oscarsson marcus-oscarsson deleted the usued-deps branch June 28, 2024 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some unused and some missing Javascript dependencies?
3 participants