search

mozilla / pdf.js

PDF Reader in JavaScript
https://mozilla.github.io/pdf.js/
Apache License 2.0
47.17k stars 9.82k forks source link

Extra dependencies? #18354

Open wojtekmaj opened 4 days ago

wojtekmaj commented 4 days ago

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

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

Snuffleupagus commented 4 days ago

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 commented 4 days ago

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 commented 3 days ago

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 https://github.com/mozilla/pdf.js/pull/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.