ipfs-inactive / js-ipfs-unixfs-importer

[ARCHIVED] JavaScript implementation of the UnixFs importer used by IPFS
MIT License
5 stars 4 forks source link

Remove no longer needed dependencies #8

Closed vmx closed 5 years ago

vmx commented 5 years ago

I use dependency-check to find dependencies that are no longer needed.

I open an issue instead of an PR to pass on the knowledge of that useful too. I normally run those for commands:

dependency-check ./package.json --missing --no-dev
dependency-check ./package.json --unused --no-dev
dependency-check ./package.json --missing ./test/*
dependency-check ./package.json --unused ./test/*
achingbrain commented 5 years ago

I think this should be added to the lint step in https://github.com/ipfs/aegir instead of piecemeal to individual repos as it's very useful - do you fancy putting a PR together there?

vmx commented 5 years ago

Sadly you're so right. I'll do that this week.

vmx commented 5 years ago

As promised: https://github.com/ipfs/aegir/pull/309

achingbrain commented 5 years ago

I'm closing this because those commands all give Success! messages on master. Hopefully your PR to aegir will be able to pick up any newly introduced breakage.

vmx commented 5 years ago

I get an error

$ dependency-check ./package.json --unused --no-dev
Fail! Modules in package.json not used in code: cids

Let's grep if it really isn't used:

$ grep --recursive "require('cids')" src
src/importer/flush-tree.js:const CID = require('cids')

Oh, it is used, why does dependency-check then report it as not being used? Let's see who is using flush-tree.

$ grep --recursive "flush-tree" .
Binary file ./.git/index matches

flush-tree is not included anywhere in the source code. After removing src/importer/flush-tree.js the tests still pass.

achingbrain commented 5 years ago

Weird. I ran:

$ dependency-check ./package.json ./src/**/*.js --unused --no-dev
Success! All dependencies in package.json are used in the code

as it's similar to the command @hugomrdias wants to default the check to in aegir.

But you are quite right:

$ dependency-check ./package.json --unused --no-dev
Fail! Modules in package.json not used in code: cids

Indeed, if I remove the glob:

$ dependency-check ./package.json src --unused --no-dev
Fail! Modules in package.json not used in code: cids
hugomrdias commented 5 years ago

watch out for only using package.json for input! the reason i want to default to ./package.json ./src/**/*.js is to include browser files, dependency-check uses node resolve to walk the deps if you only use package.json to indicate entrypoint and this way browser files will get ignored.