eslint-plugin-unicorn icon indicating copy to clipboard operation
eslint-plugin-unicorn copied to clipboard

Suggestion on slimming down dependencies

Open benmccann opened this issue 1 year ago • 8 comments

eslint-plugin-unicorn has 63 transitive dependencies: https://npm.anvaka.com/#/view/2d/eslint-plugin-unicorn

39 of them come from read-pkg-up: https://npm.anvaka.com/#/view/2d/read-pkg-up

This functionality isn't super complicated and could probably be done without any dependencies. E.g. here's Vite's implementation, which resides in a single file and is more complicated than than the current implementation in some ways since it has caching built-in: https://github.com/vitejs/vite/blob/ff3ce312a572ec126808afb97b5d3f0a6f9adcb1/packages/vite/src/node/packages.ts#L117

As an example of why we could do this with less code, read-pkg-up pulls in some dependencies to normalize the package data, but this plugin uses the {normalize: false} option, so that functionality would not be necessary.

benmccann avatar Jun 09 '23 16:06 benmccann

How many will left if we remove read-pkg-up?

fisker avatar Jun 09 '23 17:06 fisker

I believe there are 8 overlapping with other dependencies, so it would drop from 63 down to 32.

benmccann avatar Jun 09 '23 17:06 benmccann

We can switch to find-up and JSON.parse(fs.readFileSync(pkgPath, 'utf8')).

sindresorhus avatar Jun 09 '23 18:06 sindresorhus

This functionality isn't super complicated and could probably be done without any dependencies.

That's a common fallacy. If you reinvent the code, you'll likely also reinvent the same bugs and unhandled edge-cases as a package initially had and later resolved.

sindresorhus avatar Jun 09 '23 18:06 sindresorhus

We can switch to find-up and JSON.parse(fs.readFileSync(pkgPath, 'utf8')).

Don't even need find-up, this simple function will do:

function findUpSync(filename, dir) {
  const path = join(dir, filename);
  try {
    accessSync(path);
    return path;
  } catch {}

  const parent = dirname(dir);
  if (parent === dir) {
    return null;
  } else {
    return findUpSync(filename, parent);
  }
}

silverwind avatar Jun 16 '23 19:06 silverwind

Any update on this? Would you be open to accept a PR that replaces the package with either of the aforementioned suggestions? I noticed this package is using a 4 year old version of read-pkg-up, so I think it's a good idea to at least do something with it. read-pkg-up is now at v11 and even called differently: read-package-up.

pndewit avatar Nov 14 '23 07:11 pndewit

@pndewit We are waiting for ESLint to support ESM.

We'll probably switch to https://github.com/sindresorhus/find-up-simple

sindresorhus avatar Nov 14 '23 07:11 sindresorhus

Going to lock this for now until we can actually do something about it.

sindresorhus avatar Nov 14 '23 07:11 sindresorhus