Fixes yarn why -R (#6251)

## What's the problem this PR addresses?

When using the `-R` flag on a flag with a large amount of workspaces,
`yarn why` may fail to return enough information to answer the question
"why is this package here". This is because of a bug in the
implementation where we decline to print a package in the tree view if:

- We saw it before, or
- It's a workspace (because we'll print it later anyway)

However we check the first condition first and we put the package in the
"seen" list before we check whether it's a workspace. As a result,
workspaces that are dependencies of other workspaces won't have their
dependencies shown.

## How did you fix it?

Flipped the two conditions.

## Checklist

<!--- Don't worry if you miss something, chores are automatically
tested. -->
<!--- This checklist exists to help you remember doing the chores when
you submit a PR. -->
<!--- Put an `x` in all the boxes that apply. -->
- [x] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).

<!-- See
https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released
for more details. -->
<!-- Check with `yarn version check` and fix with `yarn version check
-i` -->
- [x] I have set the packages that need to be released for my changes to
be effective.

<!-- The "Testing chores" workflow validates that your PR follows our
guidelines. -->
<!-- If it doesn't pass, click on it to see details as to what your PR
might be missing. -->
- [x] I will check that all automated PR checks pass before the PR gets
reviewed.

.yarn/versions/d6ab7cea.yml

@@ -0,0 +1,23 @@
+releases:
+  "@yarnpkg/cli": patch
+  "@yarnpkg/plugin-essentials": patch
+
+declined:
+  - "@yarnpkg/plugin-compat"
+  - "@yarnpkg/plugin-constraints"
+  - "@yarnpkg/plugin-dlx"
+  - "@yarnpkg/plugin-init"
+  - "@yarnpkg/plugin-interactive-tools"
+  - "@yarnpkg/plugin-nm"
+  - "@yarnpkg/plugin-npm-cli"
+  - "@yarnpkg/plugin-pack"
+  - "@yarnpkg/plugin-patch"
+  - "@yarnpkg/plugin-pnp"
+  - "@yarnpkg/plugin-pnpm"
+  - "@yarnpkg/plugin-stage"
+  - "@yarnpkg/plugin-typescript"
+  - "@yarnpkg/plugin-version"
+  - "@yarnpkg/plugin-workspace-tools"
+  - "@yarnpkg/builder"
+  - "@yarnpkg/core"
+  - "@yarnpkg/doctor"

packages/acceptance-tests/pkg-tests-specs/sources/commands/why.test.ts

@@ -0,0 +1,211 @@
+import {ppath}    from '@yarnpkg/fslib';
+import {fs, misc} from 'pkg-tests-core';
+
+describe(`Commands`, () => {
+  describe(`why`, () => {
+    test(
+      `it should list the workspaces using a specific dependency`,
+      makeTemporaryEnv({
+        workspaces: [`packages/*`],
+      }, async ({path, run, source}) => {
+        await fs.writeJson(ppath.join(path, `packages/a/package.json`), {
+          name: `a`,
+          dependencies: {
+            [`no-deps`]: `1.0.0`,
+          },
+        });
+
+        await fs.writeJson(ppath.join(path, `packages/b/package.json`), {
+          name: `b`,
+        });
+
+        await run(`install`);
+
+        const {stdout} = await run(`why`, `no-deps`, `--json`);
+
+        expect(misc.parseJsonStream(stdout)).toEqual([{
+          value: `a@workspace:packages/a`,
+          children: {
+            [`no-deps@npm:1.0.0`]: {
+              descriptor: `no-deps@npm:1.0.0`,
+              locator: `no-deps@npm:1.0.0`,
+            },
+          },
+        }]);
+      }),
+    );
+
+    test(
+      `it should list the packages using a specific dependency`,
+      makeTemporaryEnv({
+        workspaces: [`packages/*`],
+      }, async ({path, run, source}) => {
+        await fs.writeJson(ppath.join(path, `packages/a/package.json`), {
+          name: `a`,
+          dependencies: {
+            [`one-fixed-dep`]: `1.0.0`,
+          },
+        });
+
+        await fs.writeJson(ppath.join(path, `packages/b/package.json`), {
+          name: `b`,
+        });
+
+        await run(`install`);
+
+        const {stdout} = await run(`why`, `no-deps`, `--json`);
+
+        expect(misc.parseJsonStream(stdout)).toEqual([{
+          value: `one-fixed-dep@npm:1.0.0`,
+          children: {
+            [`no-deps@npm:1.0.0`]: {
+              descriptor: `no-deps@npm:1.0.0`,
+              locator: `no-deps@npm:1.0.0`,
+            },
+          },
+        }]);
+      }),
+    );
+
+    test(
+      `it should list workspaces transitively using a specific dependency`,
+      makeTemporaryEnv({
+        workspaces: [`packages/*`],
+      }, async ({path, run, source}) => {
+        await fs.writeJson(ppath.join(path, `packages/a/package.json`), {
+          name: `a`,
+          dependencies: {
+            [`one-fixed-dep`]: `1.0.0`,
+          },
+        });
+
+        await fs.writeJson(ppath.join(path, `packages/b/package.json`), {
+          name: `b`,
+        });
+
+        await run(`install`);
+
+        const {stdout} = await run(`why`, `-R`, `no-deps`, `--json`);
+
+        expect(misc.parseJsonStream(stdout)).toEqual([{
+          value: `a@workspace:packages/a`,
+          children: {
+            [`one-fixed-dep@npm:1.0.0`]: {
+              value: {
+                descriptor: `one-fixed-dep@npm:1.0.0`,
+                locator: `one-fixed-dep@npm:1.0.0`,
+              },
+              children: {
+                [`no-deps@npm:1.0.0`]: {
+                  value: {
+                    descriptor: `no-deps@npm:1.0.0`,
+                    locator: `no-deps@npm:1.0.0`,
+                  },
+                  children: {},
+                },
+              },
+            },
+          },
+        }]);
+      }),
+    );
+
+    test(
+      `it should list workspaces transitively using a specific dependency (A = regular dep, B = workspace dep)`,
+      makeTemporaryEnv({
+        workspaces: [`packages/*`],
+      }, async ({path, run, source}) => {
+        await fs.writeJson(ppath.join(path, `packages/a/package.json`), {
+          name: `a`,
+          dependencies: {
+            [`no-deps`]: `1.0.0`,
+          },
+        });
+
+        await fs.writeJson(ppath.join(path, `packages/b/package.json`), {
+          name: `b`,
+          dependencies: {
+            [`a`]: `workspace:^`,
+          },
+        });
+
+        await run(`install`);
+
+        const {stdout} = await run(`why`, `-R`, `no-deps`, `--json`);
+
+        expect(misc.parseJsonStream(stdout)).toEqual([{
+          value: `a@workspace:packages/a`,
+          children: {
+            [`no-deps@npm:1.0.0`]: {
+              value: {
+                descriptor: `no-deps@npm:1.0.0`,
+                locator: `no-deps@npm:1.0.0`,
+              },
+              children: {},
+            },
+          },
+        }, {
+          value: `b@workspace:packages/b`,
+          children: {
+            [`a@workspace:packages/a`]: {
+              value: {
+                descriptor: `a@workspace:^`,
+                locator: `a@workspace:packages/a`,
+              },
+              children: {},
+            },
+          },
+        }]);
+      }),
+    );
+
+    test(
+      `it should list workspaces transitively using a specific dependency (A = workspace dep, B = regular dep)`,
+      makeTemporaryEnv({
+        workspaces: [`packages/*`],
+      }, async ({path, run, source}) => {
+        await fs.writeJson(ppath.join(path, `packages/a/package.json`), {
+          name: `a`,
+          dependencies: {
+            [`b`]: `workspace:^`,
+          },
+        });
+
+        await fs.writeJson(ppath.join(path, `packages/b/package.json`), {
+          name: `b`,
+          dependencies: {
+            [`no-deps`]: `1.0.0`,
+          },
+        });
+
+        await run(`install`);
+
+        const {stdout} = await run(`why`, `-R`, `no-deps`, `--json`);
+
+        expect(misc.parseJsonStream(stdout)).toEqual([{
+          value: `a@workspace:packages/a`,
+          children: {
+            [`b@workspace:packages/b`]: {
+              value: {
+                descriptor: `b@workspace:^`,
+                locator: `b@workspace:packages/b`,
+              },
+              children: {},
+            },
+          },
+        }, {
+          value: `b@workspace:packages/b`,
+          children: {
+            [`no-deps@npm:1.0.0`]: {
+              value: {
+                descriptor: `no-deps@npm:1.0.0`,
+                locator: `no-deps@npm:1.0.0`,
+              },
+              children: {},
+            },
+          },
+        }]);
+      }),
+    );
+  });
+});

packages/plugin-essentials/sources/commands/why.ts

@@ -176,18 +176,18 @@ function whyRecursive(project: Project, identHash: IdentHash, {configuration, pe
     const key = structUtils.stringifyLocator(pkg);
     parentChildren[key] = node;
 
+    // We don't want to print the children of our transitive workspace
+    // dependencies, as they will be printed in their own top-level branch
+    if (dependency !== null && project.tryWorkspaceByLocator(pkg))
+      return;
+
     // We don't want to reprint the children for a package that already got
     // printed as part of another branch
     if (printed.has(pkg.locatorHash))
       return;
 
     printed.add(pkg.locatorHash);
 
-    // We don't want to print the children of our transitive workspace
-    // dependencies, as they will be printed in their own top-level branch
-    if (dependency !== null && project.tryWorkspaceByLocator(pkg))
-      return;
-
     for (const dependency of pkg.dependencies.values()) {
       if (!peers && pkg.peerDependencies.has(dependency.identHash))
         continue;