Fixes the optional check depending on the order the tree is traversed (#5840)

**What's the problem this PR addresses?**

Depending on the order we traversed the dependency tree, it could happen
that we'd see a package as optional before seeing the other cases where
it was not optional. Because marking the package as "not optional"
occurred after the "has this package been traversed before?" check, we
were never updating its status from "optional" to "not optional".

Fixes #5827

**How did you fix it?**

We now check for optionality regardless of whether the package has been
seen or not before. I added an order-dependent dragon test to try to
prevent regressions.

**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.

Author: arcanis

.yarn/versions/0d12a741.yml

@@ -0,0 +1,34 @@
+releases:
+  "@yarnpkg/cli": patch
+  "@yarnpkg/core": patch
+
+declined:
+  - "@yarnpkg/plugin-compat"
+  - "@yarnpkg/plugin-constraints"
+  - "@yarnpkg/plugin-dlx"
+  - "@yarnpkg/plugin-essentials"
+  - "@yarnpkg/plugin-exec"
+  - "@yarnpkg/plugin-file"
+  - "@yarnpkg/plugin-git"
+  - "@yarnpkg/plugin-github"
+  - "@yarnpkg/plugin-http"
+  - "@yarnpkg/plugin-init"
+  - "@yarnpkg/plugin-interactive-tools"
+  - "@yarnpkg/plugin-link"
+  - "@yarnpkg/plugin-nm"
+  - "@yarnpkg/plugin-npm"
+  - "@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/doctor"
+  - "@yarnpkg/extensions"
+  - "@yarnpkg/nm"
+  - "@yarnpkg/pnpify"
+  - "@yarnpkg/sdks"

packages/acceptance-tests/pkg-tests-specs/sources/dragon.test.js

@@ -682,4 +682,48 @@ describe(`Dragon tests`, () => {
       },
     ),
   );
+
+  test(
+    `it should pass the dragon test 13`,
+    makeTemporaryEnv(
+      {
+        workspaces: [
+          `pkg-a`,
+          `pkg-b`,
+        ],
+      },
+      async ({path, run, source}) => {
+        // This dragon test represents the following scenario:
+        //
+        // .
+        // ├── pkg-a/
+        // │   └── no-deps-failing (optional)
+        // └── pkg-b/
+        //     └── no-deps-failing (not optional)
+        //
+        // Depending on the order of traversal we may end up marking no-deps-failing
+        // as being traversed, and skip all future traversals. If we're not being
+        // careful this may cause setting the "not optional" flag to be skipped as
+        // well, making Yarn believe that no-deps-failing is optional when it's not.
+
+        await xfs.mkdirPromise(`${path}/pkg-a`);
+        await xfs.writeJsonPromise(`${path}/pkg-a/package.json`, {
+          name: `pkg-a`,
+          optionalDependencies: {
+            [`no-deps-failing`]: `1.0.0`,
+          },
+        });
+
+        await xfs.mkdirPromise(`${path}/pkg-b`);
+        await xfs.writeJsonPromise(`${path}/pkg-b/package.json`, {
+          name: `pkg-b`,
+          dependencies: {
+            [`no-deps-failing`]: `1.0.0`,
+          },
+        });
+
+        await expect(run(`install`)).rejects.toThrowError(`no-deps-failing@npm:1.0.0 couldn't be built successfully`);
+      },
+    ),
+  );
 });

packages/yarnpkg-core/sources/Project.ts

@@ -2235,14 +2235,14 @@ function applyVirtualResolutionMutations({
   };
 
   const resolvePeerDependenciesImpl = (parentDescriptor: Descriptor, parentLocator: Locator, peerSlots: Map<IdentHash, LocatorHash>, {top, optional}: {top: LocatorHash, optional: boolean}) => {
+    if (!optional)
+      optionalBuilds.delete(parentLocator.locatorHash);
+
     if (accessibleLocators.has(parentLocator.locatorHash))
       return;
 
     accessibleLocators.add(parentLocator.locatorHash);
 
-    if (!optional)
-      optionalBuilds.delete(parentLocator.locatorHash);
-
     const parentPackage = allPackages.get(parentLocator.locatorHash);
     if (!parentPackage)
       throw new Error(`Assertion failed: The package (${structUtils.prettyLocator(project.configuration, parentLocator)}) should have been registered`);