Fixes yarn run when used on workspaces + pnp

yarnpkg · Sep 28, 2018 · 2f913c9 · 2f913c9
1 parent 3fc13c1
commit 2f913c9
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 18 deletions.
diff --git a/packages/pkg-tests/pkg-tests-specs/sources/workspace.js b/packages/pkg-tests/pkg-tests-specs/sources/workspace.js
@@ -22,8 +22,8 @@ module.exports = (makeTemporaryEnv: PackageDriver) => {
           await writeFile(
             `${path}/packages/workspace-a/index.js`,
             `
-        module.exports = 42;
-      `,
+              module.exports = 42;
+            `,
           );
 
           await run(`install`);
@@ -56,8 +56,8 @@ module.exports = (makeTemporaryEnv: PackageDriver) => {
           await writeFile(
             `${path}/packages/workspace-a/index.js`,
             `
-        module.exports = require('workspace-b/package.json');
-      `,
+              module.exports = require('workspace-b/package.json');
+            `,
           );
 
           await writeJson(`${path}/packages/workspace-b/package.json`, {
@@ -71,8 +71,8 @@ module.exports = (makeTemporaryEnv: PackageDriver) => {
           await writeFile(
             `${path}/packages/workspace-b/index.js`,
             `
-        module.exports = require('workspace-a/package.json');
-      `,
+              module.exports = require('workspace-a/package.json');
+            `,
           );
 
           await run(`install`);
@@ -110,8 +110,8 @@ module.exports = (makeTemporaryEnv: PackageDriver) => {
           await writeFile(
             `${path}/packages/workspace/index.js`,
             `
-        module.exports = require('no-deps/package.json');
-      `,
+              module.exports = require('no-deps/package.json');
+            `,
           );
 
           await writeJson(`${path}/packages/no-deps/package.json`, {
@@ -128,5 +128,32 @@ module.exports = (makeTemporaryEnv: PackageDriver) => {
         },
       ),
     );
+
+    test(
+      `it should allow scripts defined in workspaces to run successfully`,
+      makeTemporaryEnv(
+        {
+          private: true,
+          workspaces: [`packages/*`],
+        },
+        async ({path, run, source}) => {
+          await writeJson(`${path}/packages/workspace/package.json`, {
+            name: `workspace`,
+            version: `1.0.0`,
+            dependencies: {
+              [`has-bin-entries`]: `1.0.0`,
+            },
+          });
+
+          await run(`install`);
+
+          await expect(
+            run(`run`, `has-bin-entries`, `foo`, {
+              cwd: `${path}/packages/workspace`,
+            }),
+          ).resolves.toMatchObject({stdout: `foo\n`});
+        },
+      ),
+    );
   });
 };
diff --git a/src/cli/commands/run.js b/src/cli/commands/run.js
@@ -36,14 +36,15 @@ export async function getBinEntries(config: Config): Promise<Map<string, string>
   // Same thing, but for the pnp dependencies, located inside the cache
   if (await fs.exists(`${config.lockfileFolder}/${constants.PNP_FILENAME}`)) {
     const pnpApi = dynamicRequire(`${config.lockfileFolder}/${constants.PNP_FILENAME}`);
-    const topLevelInformation = pnpApi.getPackageInformation({name: null, reference: null});
 
-    for (const [name, reference] of topLevelInformation.packageDependencies.entries()) {
+    const packageLocator = pnpApi.findPackageLocator(`${config.cwd}/`);
+    const packageInformation = pnpApi.getPackageInformation(packageLocator);
+
+    for (const [name, reference] of packageInformation.packageDependencies.entries()) {
       const dependencyInformation = pnpApi.getPackageInformation({name, reference});
 
       if (dependencyInformation.packageLocation) {
-        const fullPath = path.resolve(config.lockfileFolder, dependencyInformation.packageLocation);
-        binFolders.add(`${fullPath}/.bin`);
+        binFolders.add(`${dependencyInformation.packageLocation}/.bin`);
       }
     }
   }

diff --git a/src/util/execute-lifecycle-script.js b/src/util/execute-lifecycle-script.js
@@ -212,15 +212,13 @@ export async function makeEnv(
     }
   }
 
-  // Otherwise, only add the top-level dependencies to the PATH
-  // Note that this isn't enough when executing scripts from subdependencies, but since dependencies with postinstall
-  // scripts have other issues that require us to make them fallback to regular node_modules installation (like sharing
-  // artifacts), we can sit on this one until we fix everything at once.
   if (await fs.exists(`${config.lockfileFolder}/${constants.PNP_FILENAME}`)) {
     const pnpApi = dynamicRequire(`${config.lockfileFolder}/${constants.PNP_FILENAME}`);
-    const topLevelInformation = pnpApi.getPackageInformation({name: null, reference: null});
 
-    for (const [name, reference] of topLevelInformation.packageDependencies.entries()) {
+    const packageLocator = pnpApi.findPackageLocator(`${config.cwd}/`);
+    const packageInformation = pnpApi.getPackageInformation(packageLocator);
+
+    for (const [name, reference] of packageInformation.packageDependencies.entries()) {
       const dependencyInformation = pnpApi.getPackageInformation({name, reference});
 
       if (!dependencyInformation || !dependencyInformation.packageLocation) {