From 672c27cfe6ff1e7ddb9fbc58c6554f00d9510c8a Mon Sep 17 00:00:00 2001
From: Zoltan Kochan <zkochan@users.noreply.github.com>
Date: Sun, 10 Jan 2021 18:12:25 +0200
Subject: [PATCH] fix: don't create broken symlinks during hoisting

ref #2916
PR #3062
---
 .changeset/poor-points-poke.md      |  5 +++++
 packages/supi/src/install/link.ts   |  9 ++++++++-
 packages/supi/test/install/hoist.ts | 23 +++++++++++------------
 3 files changed, 24 insertions(+), 13 deletions(-)
 create mode 100644 .changeset/poor-points-poke.md

diff --git a/.changeset/poor-points-poke.md b/.changeset/poor-points-poke.md
new file mode 100644
index 0000000000..f8ce0fad67
--- /dev/null
+++ b/.changeset/poor-points-poke.md
@@ -0,0 +1,5 @@
+---
+"supi": patch
+---
+
+Don't create broken symlinks to skipped optional dependencies, when hoisting. This issue was already fixed in pnpm v5.13.7 for the case when the lockfile is up-to-date. This fixes the same issue for cases when the lockfile need updates. For instance, when adding a new package.
diff --git a/packages/supi/src/install/link.ts b/packages/supi/src/install/link.ts
index 8f1af136a9..d24b7ede87 100644
--- a/packages/supi/src/install/link.ts
+++ b/packages/supi/src/install/link.ts
@@ -231,8 +231,15 @@ export default async function linkPackages (
 
   let newHoistedDependencies!: HoistedDependencies
   if ((opts.hoistPattern != null || opts.publicHoistPattern != null) && (newDepPaths.length > 0 || removedDepPaths.size > 0)) {
+    // It is important to keep the skipped packages in the lockfile which will be saved as the "current lockfile".
+    // pnpm is comparing the current lockfile to the wanted one and they should much.
+    // But for hoisting, we need a version of the lockfile w/o the skipped packages, so we're making a copy.
+    const hoistLockfile = {
+      ...currentLockfile,
+      packages: R.omit(Array.from(opts.skipped), currentLockfile.packages),
+    }
     newHoistedDependencies = await hoist({
-      lockfile: currentLockfile,
+      lockfile: hoistLockfile,
       lockfileDir: opts.lockfileDir,
       privateHoistedModulesDir: opts.hoistedModulesDir,
       privateHoistPattern: opts.hoistPattern ?? [],
diff --git a/packages/supi/test/install/hoist.ts b/packages/supi/test/install/hoist.ts
index 90fdb89b00..3ecac354f5 100644
--- a/packages/supi/test/install/hoist.ts
+++ b/packages/supi/test/install/hoist.ts
@@ -502,28 +502,27 @@ test('should recreate node_modules with hoisting', async () => {
 
 test('hoisting should not create a broken symlink to a skipped optional dependency', async () => {
   const project = prepareEmpty()
+  const rootModules = assertProject(process.cwd())
 
-  await install({
+  const manifest = {
+    dependencies: {
+      'is-positive': '1.0.0',
+    },
     optionalDependencies: {
       'not-compatible-with-any-os': '*',
     },
-  }, await testDefaults({ publicHoistPattern: '*' }))
+  }
+
+  await install(manifest, await testDefaults({ publicHoistPattern: '*' }))
 
   await project.hasNot('dep-of-optional-pkg')
+  expect(await rootModules.readCurrentLockfile()).toStrictEqual(await rootModules.readLockfile())
 
   // Verifying the same with headless installation
   await rimraf('node_modules')
 
-  await install({
-    optionalDependencies: {
-      'not-compatible-with-any-os': '*',
-    },
-  }, await testDefaults({ publicHoistPattern: '*' }))
+  await install(manifest, await testDefaults({ publicHoistPattern: '*' }))
 
   await project.hasNot('dep-of-optional-pkg')
-
-  const rootModules = assertProject(process.cwd())
-  const currentLockfile = await rootModules.readCurrentLockfile()
-  const wantedLockfile = await rootModules.readLockfile()
-  expect(currentLockfile).toStrictEqual(wantedLockfile)
+  expect(await rootModules.readCurrentLockfile()).toStrictEqual(await rootModules.readLockfile())
 })
\ No newline at end of file