Added support for finding pnpm workspace packages

[Andarist]

No description provided.

[Andarist]

I'm quite unsure why this is needed (but there is an explicit test for it). manypkg is by definition supposed to help with validating monorepo setups, so i dont understand why a single package mode is supported?

[emmatown]

It exists so that other tools can use this even though it's not Manypkg's use case. (I have some ideas in mind where this will be necessary)

[Andarist]

I have some ideas in mind where this will be necessary

Could you share them? I'm trying to understand the whole context, so that would be helpful to me

[emmatown]

The main one I have right now is a minimal wrapper around npm, pnpm, Yarn and possibly Bolt that chooses the right tool given a repo and lets you do installs and run scripts/binaries and I want it to work on single package repos as well.

[emmatown]

Could you replace this file with what's below? (I started writing this a little bit ago but hadn't written tests yet and I think it'll be a little faster than yours since it won't do the globbing and the pnpm file checking will happen in parallel with the package.json reading)

import findUp from "find-up";
import path from "path";
import fs from "fs-extra";

export class NoPkgJsonFound extends Error {
  directory: string;
  constructor(directory: string) {
    super(
      `No package.json could be found upwards from the directory ${directory}`
    );
    this.directory = directory;
  }
}

async function hasWorkspacesConfiguredViaPkgJson(
  directory: string,
  firstPkgJsonDirRef: { current: string | undefined }
) {
  try {
    let pkgJson = await fs.readJson(path.join(directory, "package.json"));
    if (firstPkgJsonDirRef.current === undefined) {
      firstPkgJsonDirRef.current = directory;
    }
    if (pkgJson.workspaces || pkgJson.bolt) {
      return directory;
    }
  } catch (err) {
    if (err.code !== "ENOENT") {
      throw err;
    }
  }
}

async function hasWorkspacesConfiguredViaPnpm(directory: string) {
  // @ts-ignore
  let pnpmWorkspacesFileExists = await fs.exists(
    path.join(directory, "pnpm-workspace.yaml")
  );
  if (pnpmWorkspacesFileExists) {
    return directory;
  }
}

export default async function findWorkspacesRoot(cwd: string): Promise<string> {
  let firstPkgJsonDirRef: { current: string | undefined } = {
    current: undefined
  };
  let dir = await findUp(
    directory => {
      return Promise.all([
        hasWorkspacesConfiguredViaPkgJson(directory, firstPkgJsonDirRef),
        hasWorkspacesConfiguredViaPnpm(directory)
      ]).then(x => x.find(dir => dir));
    },
    { cwd, type: "directory" }
  );
  if (firstPkgJsonDirRef.current === undefined) {
    throw new NoPkgJsonFound(cwd);
  }
  if (dir === undefined) {
    return firstPkgJsonDirRef.current;
  }
  return dir;
}

[Andarist]

Well, my main motivation for this was to just reuse get-workspaces logic, because on the high level we want both of those (get-workspaces and find-workspaces-root) to match. There are other types of workspaces config (for example lerna) and when you add support for those you need to always keep those two in sync (if you implement each of them separately).

This check is not really happening that often, so I wasn't really concerned with perf as it should be rather sufficient anyway - especially given that manypkg is primarily for monorepo usage, so doing a second fs traversal for single package roots should happen really rarely.

[emmatown]

There are other types of workspaces config (for example lerna) and when you add support for those you need to always keep those two in sync (if you implement each of them separately).

Yeah, that's certainly a potential concern. I think it's acceptable for this because there won't be that many types of workspaces config.

This check is not really happening that often, so I wasn't really concerned with perf as it should be rather sufficient anyway - especially given that manypkg is primarily for monorepo usage, so doing a second fs traversal for single package roots should happen really rarely.

That's certainly true for Manypkg but for the usecase I mentioned in #39 (comment) it will be a lot more important (e.g. importing globby and its deps takes ~500ms iirc which is likely too slow for that use case)

[Andarist]

A potential idea that could alleviate this concern - splitting the logic of get-workspaces into two parts: determining the workspace type (yarn, bolt, pnpm, others) & resolving the workspace packages. That way we could just reuse the first one here and as to the concern of importing globby being slow:

• either split get-workspaces into two packages
• or use "lazy" mode of Babel's commonjs transform

[emmatown]

I guess splitting the logic of get-workspaces would be fine but I feel like it's not enough code for it to matter much so IMO the best thing to do would be to move get-workspaces into this repo (@Noviny do you have any problems with this?) so that we can easily keep them in sync.

[emmatown]

Update on this, I'm happy to use the implementation here because I've decided to write the previously mentioned tool in Rust.

[Andarist]

huh, which one do you want to write in Rust?

[emmatown]

The wrapper around the various package managers to install packages and run scripts and binaries

[emmatown]

Thanks!!