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

Extra dependencies? #18354

Closed
wojtekmaj opened this issue Jul 1, 2024 · 5 comments
Closed

Extra dependencies? #18354

wojtekmaj opened this issue Jul 1, 2024 · 5 comments

Comments

@wojtekmaj
Copy link
Contributor

Hey,
there are a couple of dependencies I can't really find use of in the codebase. These dependencies are:

  • caniuse-lite
  • eslint-plugin-html
  • eslint-plugin-fetch-options
  • globals
  • jstransformer-nunjucks

Do you think we could remove them? I can help you with that if you want.

@Snuffleupagus
Copy link
Collaborator

there are a couple of dependencies I can't really find use of in the codebase.

That doesn't necessarily mean that they are unused though, since those were (and possibly still are) necessary for other dependencies; hence any changes/removals would require very careful testing.

For example, the globals dependency was originally added (by you :-) in PR #10293.

@timvandermeij
Copy link
Contributor

The jstransformer-nunjucks one is necessary for Metalsmith to build the website (and was introduced quite recently). The globals one was at least needed for eslint-plugin-mozilla at the time, but I'm not sure if that is still the case, so this one could be checked. For the other ones I think they are still used, but that can be verified using Git blame to find out when they were introduced, for which purpose and if they are still in use. Indeed, this should be verified carefully, but if they are in fact unused we are of course open for PRs.

@wojtekmaj
Copy link
Contributor Author

those were (and possibly still are) necessary for other dependencies

If that's the case, then they should have specified them as peerDependencies. npm nowadays installs peerDependencies automatically, so this shouldn't lead to missing dependencies, as long as they are specified correctly.

For example, the globals dependency was originally added (by you :-) in PR #10293.

WOW it's been a while :D Can't believe I was a contributor for 6 years!

Anyways, tracked the origins of each dependency listed.

@timvandermeij
Copy link
Contributor

Thank you for investigating this. For the ESLint ones the new link is https://searchfox.org/mozilla-central/rev/master/tools/lint/eslint/eslint-plugin-mozilla/package.json#35-40 and I think those were added because they're peer dependencies and are probably not installed by default unless they are added explicitly. The globals one is interesting though and warrants more research to see if we can get rid of it. I think the others must stay.

@timvandermeij
Copy link
Contributor

Closing since globals is removed in #18509, jstransformer-nunjucks is used by Metalsmith to read Nunjucks template files (for example https://github.com/mozilla/pdf.js/blob/master/docs/templates/layout.njk) during building the website, caniuse-lite is used for Autoprefixer to always have the most recent database and eslint-plugin-{html,fetch-options} are useful peer dependencies of eslint-plugin-mozilla which we wish to use to the fullest.

Thank you for bringing this to our attention, since it did result in globals being removed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants