azz / get-monorepo-packages

Get a list of packages from a monorepo
MIT License
19 stars 7 forks source link

Remove globby as dependency #4

Closed tunnckoCore closed 5 years ago

tunnckoCore commented 5 years ago

I don't think it is needed, at all. Also, it's slow no matter it uses fast-glob under the hood.

I recently needed and implemented such thing. I remove the /* from given workspaces, then just readdir.

pkg.workspaces
    .map((x) => x.slice(0, -2))
    .filter(Boolean)
    .reduce((acc, ws) => {
      const workspace = path.join(cwd, ws);
      const packages = fs
        .readdirSync(workspace)
        .map((dir) => {
          const pkgDir = path.join(workspace, dir);
          const pkgJsonPath = path.join(pkgDir, 'package.json');

          return { pkgDir, pkgJsonPath };
        })
        .map(({ pkgDir, pkgJsonPath }) => {
          // eslint-disable-next-line global-require, import/no-dynamic-require
          const pkgJson = require(pkgJsonPath);
          return { pkgDir, pkgJson };
        });

      return acc.concat(packages);
    }, [])
azz commented 5 years ago

Do you have any performance benchmarks? Doing a x/* glob should be very quick.

tunnckoCore commented 5 years ago

Benchmarks are on the fast-glob repo. And as I understand they are as of globby@8, recently v9 was released but I think nothing important that impacts benchmarks, so I don't think it will be faster than v8.

tunnckoCore commented 5 years ago

Also, generally speaking, it's not needed. It may be quick, but the most common case is /* and not more complex glob anyway.

azz commented 5 years ago

Maybe the most common, but not the only possible value. I'd rather this library be correct than fast, as it is generally used for dev tooling at process boot, and is not in any critical code paths.

tunnckoCore commented 5 years ago

No matter what, things should always be optimized for at least the 90% of the cases ("most common").

Anyway. Such mistakes are the reason why tooling is in that state currently.