Skip to content

Commit

Permalink
feat(run): Include the workspace root .bin in env path (#4848)
Browse files Browse the repository at this point in the history
**Summary**

Partial resolution for #4543.

Previously, when running `yarn run` the env PATH would be set to look in node_modules/.bin, however, in workspaces the root workspace .bin path was not being included.

This PR adds the workspace root
node_modules/.bin path after the individual package's path.

This is generally needed because #4730 ensures bin links in a workspace will be at the workspace root. With this PR, you can now `yarn run` commands in an individual package again.

**Test plan**

Manually tested by adding a script that runs `echo $PATH`
  • Loading branch information
rally25rs authored and BYK committed Nov 20, 2017
1 parent 80d6e05 commit 1ea7ef8
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 0 deletions.
22 changes: 22 additions & 0 deletions __tests__/commands/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,17 @@ const fixturesLoc = path.join(__dirname, '..', 'fixtures', 'run');
const runRun = buildRun.bind(null, BufferReporter, fixturesLoc, (args, flags, config, reporter): Promise<void> => {
return run(config, reporter, flags, args);
});
const runRunInWorkspacePackage = function(cwd, ...args): Promise<void> {
return buildRun.bind(null, BufferReporter, fixturesLoc, (args, flags, config, reporter): Promise<void> => {
const originalCwd = config.cwd;
config.cwd = path.join(originalCwd, cwd);
const retVal = run(config, reporter, flags, args);
retVal.then(() => {
config.cwd = originalCwd;
});
return retVal;
})(...args);
};

test('lists all available commands with no arguments', (): Promise<void> => {
return runRun([], {}, 'no-args', (config, reporter): ?Promise<void> => {
Expand Down Expand Up @@ -133,3 +144,14 @@ test('adds quotes if args have spaces and quotes', (): Promise<void> => {
expect(execCommand).toBeCalledWith(...args);
});
});

test('adds workspace root node_modules/.bin to path when in a workspace', (): Promise<void> => {
return runRunInWorkspacePackage('packages/pkg1', ['env'], {}, 'workspace', (config, reporter): ?Promise<void> => {
const logEntry = reporter.getBuffer().find(entry => entry.type === 'log');
const parsedLogData = JSON.parse(logEntry ? logEntry.data.toString() : '{}');
const envPaths = (parsedLogData.PATH || parsedLogData.Path).split(path.delimiter);

expect(envPaths).toContain(path.join(config.cwd, 'node_modules', '.bin'));
expect(envPaths).toContain(path.join(config.cwd, 'packages', 'pkg1', 'node_modules', '.bin'));
});
});
Empty file.
5 changes: 5 additions & 0 deletions __tests__/fixtures/run/workspace/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"workspaces": [
"packages/*"
]
}
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
3 changes: 3 additions & 0 deletions src/util/execute-lifecycle-script.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ export async function makeEnv(
// add .bin folders to PATH
for (const registry of Object.keys(registries)) {
const binFolder = path.join(config.registries[registry].folder, '.bin');
if (config.workspacesEnabled && config.workspaceRootFolder) {
pathParts.unshift(path.join(config.workspaceRootFolder, binFolder));
}
pathParts.unshift(path.join(config.linkFolder, binFolder));
pathParts.unshift(path.join(cwd, binFolder));
}
Expand Down

1 comment on commit 1ea7ef8

@jonaskello
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not seem to work in 1.3.2. Was it reverted?

Please sign in to comment.