Skip to content

Commit 22139ca

Browse files
authored
Fixes yarn run when used on workspaces + pnp (#6444)
1 parent 83b5fbc commit 22139ca

File tree

3 files changed

+44
-18
lines changed

3 files changed

+44
-18
lines changed

packages/pkg-tests/pkg-tests-specs/sources/workspace.js

+35-8
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ module.exports = (makeTemporaryEnv: PackageDriver) => {
2222
await writeFile(
2323
`${path}/packages/workspace-a/index.js`,
2424
`
25-
module.exports = 42;
26-
`,
25+
module.exports = 42;
26+
`,
2727
);
2828

2929
await run(`install`);
@@ -56,8 +56,8 @@ module.exports = (makeTemporaryEnv: PackageDriver) => {
5656
await writeFile(
5757
`${path}/packages/workspace-a/index.js`,
5858
`
59-
module.exports = require('workspace-b/package.json');
60-
`,
59+
module.exports = require('workspace-b/package.json');
60+
`,
6161
);
6262

6363
await writeJson(`${path}/packages/workspace-b/package.json`, {
@@ -71,8 +71,8 @@ module.exports = (makeTemporaryEnv: PackageDriver) => {
7171
await writeFile(
7272
`${path}/packages/workspace-b/index.js`,
7373
`
74-
module.exports = require('workspace-a/package.json');
75-
`,
74+
module.exports = require('workspace-a/package.json');
75+
`,
7676
);
7777

7878
await run(`install`);
@@ -110,8 +110,8 @@ module.exports = (makeTemporaryEnv: PackageDriver) => {
110110
await writeFile(
111111
`${path}/packages/workspace/index.js`,
112112
`
113-
module.exports = require('no-deps/package.json');
114-
`,
113+
module.exports = require('no-deps/package.json');
114+
`,
115115
);
116116

117117
await writeJson(`${path}/packages/no-deps/package.json`, {
@@ -128,5 +128,32 @@ module.exports = (makeTemporaryEnv: PackageDriver) => {
128128
},
129129
),
130130
);
131+
132+
test(
133+
`it should allow scripts defined in workspaces to run successfully`,
134+
makeTemporaryEnv(
135+
{
136+
private: true,
137+
workspaces: [`packages/*`],
138+
},
139+
async ({path, run, source}) => {
140+
await writeJson(`${path}/packages/workspace/package.json`, {
141+
name: `workspace`,
142+
version: `1.0.0`,
143+
dependencies: {
144+
[`has-bin-entries`]: `1.0.0`,
145+
},
146+
});
147+
148+
await run(`install`);
149+
150+
await expect(
151+
run(`run`, `has-bin-entries`, `foo`, {
152+
cwd: `${path}/packages/workspace`,
153+
}),
154+
).resolves.toMatchObject({stdout: `foo\n`});
155+
},
156+
),
157+
);
131158
});
132159
};

src/cli/commands/run.js

+5-4
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,15 @@ export async function getBinEntries(config: Config): Promise<Map<string, string>
3636
// Same thing, but for the pnp dependencies, located inside the cache
3737
if (await fs.exists(`${config.lockfileFolder}/${constants.PNP_FILENAME}`)) {
3838
const pnpApi = dynamicRequire(`${config.lockfileFolder}/${constants.PNP_FILENAME}`);
39-
const topLevelInformation = pnpApi.getPackageInformation({name: null, reference: null});
4039

41-
for (const [name, reference] of topLevelInformation.packageDependencies.entries()) {
40+
const packageLocator = pnpApi.findPackageLocator(`${config.cwd}/`);
41+
const packageInformation = pnpApi.getPackageInformation(packageLocator);
42+
43+
for (const [name, reference] of packageInformation.packageDependencies.entries()) {
4244
const dependencyInformation = pnpApi.getPackageInformation({name, reference});
4345

4446
if (dependencyInformation.packageLocation) {
45-
const fullPath = path.resolve(config.lockfileFolder, dependencyInformation.packageLocation);
46-
binFolders.add(`${fullPath}/.bin`);
47+
binFolders.add(`${dependencyInformation.packageLocation}/.bin`);
4748
}
4849
}
4950
}

src/util/execute-lifecycle-script.js

+4-6
Original file line numberDiff line numberDiff line change
@@ -212,15 +212,13 @@ export async function makeEnv(
212212
}
213213
}
214214

215-
// Otherwise, only add the top-level dependencies to the PATH
216-
// Note that this isn't enough when executing scripts from subdependencies, but since dependencies with postinstall
217-
// scripts have other issues that require us to make them fallback to regular node_modules installation (like sharing
218-
// artifacts), we can sit on this one until we fix everything at once.
219215
if (await fs.exists(`${config.lockfileFolder}/${constants.PNP_FILENAME}`)) {
220216
const pnpApi = dynamicRequire(`${config.lockfileFolder}/${constants.PNP_FILENAME}`);
221-
const topLevelInformation = pnpApi.getPackageInformation({name: null, reference: null});
222217

223-
for (const [name, reference] of topLevelInformation.packageDependencies.entries()) {
218+
const packageLocator = pnpApi.findPackageLocator(`${config.cwd}/`);
219+
const packageInformation = pnpApi.getPackageInformation(packageLocator);
220+
221+
for (const [name, reference] of packageInformation.packageDependencies.entries()) {
224222
const dependencyInformation = pnpApi.getPackageInformation({name, reference});
225223

226224
if (!dependencyInformation || !dependencyInformation.packageLocation) {

0 commit comments

Comments
 (0)