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

Suggestion on slimming down dependencies #2151

Open
benmccann opened this issue Jun 9, 2023 · 8 comments
Open

Suggestion on slimming down dependencies #2151

benmccann opened this issue Jun 9, 2023 · 8 comments

Comments

@benmccann
Copy link
Contributor

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.

@fisker
Copy link
Collaborator

fisker commented Jun 9, 2023

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

@benmccann
Copy link
Contributor Author

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

@sindresorhus
Copy link
Owner

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

@sindresorhus
Copy link
Owner

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.

@silverwind
Copy link
Contributor

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);
  }
}

@pndewit
Copy link

pndewit commented Nov 14, 2023

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.

@sindresorhus
Copy link
Owner

@pndewit We are waiting for ESLint to support ESM.

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

@sindresorhus
Copy link
Owner

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

Repository owner locked and limited conversation to collaborators Nov 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants