From 26dc660ad4850a03c066a5da8a7befb3352bae82 Mon Sep 17 00:00:00 2001 From: Seung-gi Lim Date: Wed, 3 Mar 2021 18:07:55 +0900 Subject: [PATCH] Use cwd node_modules/.bin entry first in workspace - If there are multiple project which has dependency for the different version of 3rd party modules, only workspace root .bin was resolved. - This change fixes this bug by changing order of bin entries when resolving binary locations which are in several different package folders - See https://github.com/sglim/yarn-workspace-bin-bug to reproduce the bug --- __tests__/commands/bin.js | 21 +++++++++++++++++++ __tests__/commands/run.js | 2 +- .../my_project_with_bin/v1/cli_v1.js | 4 ++++ .../my_project_with_bin/v1/package.json | 8 +++++++ .../my_project_with_bin/v2/cli_v2.js | 4 ++++ .../my_project_with_bin/v2/package.json | 8 +++++++ .../package.json | 7 +++++++ .../packages/workspace-1/package.json | 8 +++++++ .../packages/workspace-2/package.json | 8 +++++++ src/cli/commands/run.js | 2 +- 10 files changed, 70 insertions(+), 2 deletions(-) create mode 100755 __tests__/fixtures/install/workspaces-install-link-bin-2-versions/my_project_with_bin/v1/cli_v1.js create mode 100644 __tests__/fixtures/install/workspaces-install-link-bin-2-versions/my_project_with_bin/v1/package.json create mode 100755 __tests__/fixtures/install/workspaces-install-link-bin-2-versions/my_project_with_bin/v2/cli_v2.js create mode 100644 __tests__/fixtures/install/workspaces-install-link-bin-2-versions/my_project_with_bin/v2/package.json create mode 100644 __tests__/fixtures/install/workspaces-install-link-bin-2-versions/package.json create mode 100644 __tests__/fixtures/install/workspaces-install-link-bin-2-versions/packages/workspace-1/package.json create mode 100644 __tests__/fixtures/install/workspaces-install-link-bin-2-versions/packages/workspace-2/package.json diff --git a/__tests__/commands/bin.js b/__tests__/commands/bin.js index 56730ab9dc..8c9a9caa42 100644 --- a/__tests__/commands/bin.js +++ b/__tests__/commands/bin.js @@ -10,6 +10,15 @@ const fixturesLoc = path.join(__dirname, '..', 'fixtures', 'bin'); const runBin = buildRun.bind(null, BufferReporter, fixturesLoc, (args, flags, config, reporter): Promise => { return run(config, reporter, flags, args); }); +const runInWorkspacePackage = function(cwd, config, reporter, flags, args): Promise { + const originalCwd = config.cwd; + config.cwd = path.join(originalCwd, cwd); + const retVal = run(config, reporter, flags, args); + retVal.then(() => { + config.cwd = originalCwd; + }); + return retVal; +}; test('running bin without arguments should return the folder where the binaries are stored', (): Promise => { return runBin([], {}, '../install/install-production-bin', (config, reporter): ?Promise => { @@ -24,3 +33,15 @@ test('running bin with a binary name as the argument should return its full path expect(reporter.getBufferText()).toMatch(/[\\\/]node_modules[\\\/]\.bin[\\\/]rimraf$/); }); }); + +test('should use .bin in workspace node modules respectively', (): Promise => { + return runInstall({binLinks: true}, 'workspaces-install-link-bin-2-versions', async (config): ?Promise => { + const reporter1 = new BufferReporter(); + await runInWorkspacePackage('packages/workspace-1', config, reporter1, {}, ['myBin']); + expect(reporter1.getBufferText()).toMatch(/[\\\/]workspace-1[\\\/]node_modules[\\\/]\.bin[\\\/]myBin$/); + + const reporter2 = new BufferReporter(); + await runInWorkspacePackage('packages/workspace-2', config, reporter2, {}, ['myBin']); + expect(reporter2.getBufferText()).toMatch(/[\\\/]workspace-2[\\\/]node_modules[\\\/]\.bin[\\\/]myBin$/); + }); +}); diff --git a/__tests__/commands/run.js b/__tests__/commands/run.js index 35a1221e56..a1c9cdbdf8 100644 --- a/__tests__/commands/run.js +++ b/__tests__/commands/run.js @@ -207,7 +207,7 @@ test('adds workspace root node_modules/.bin to path when in a workspace', (): Pr expect(envPaths).toContain(path.join(config.cwd, 'packages', 'pkg1', 'node_modules', '.bin')); })); -test('adds cwd node_modules/.bin to path when in a workspace usig nohoist', (): Promise => +test('adds cwd node_modules/.bin to path when in a workspace using nohoist', (): Promise => runRunInWorkspacePackage('packages/pkg1', ['env'], {}, 'nohoist-workspace', (config, reporter): ?Promise => { const logEntry = reporter.getBuffer().find(entry => entry.type === 'log'); const parsedLogData = JSON.parse(logEntry ? logEntry.data.toString() : '{}'); diff --git a/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/my_project_with_bin/v1/cli_v1.js b/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/my_project_with_bin/v1/cli_v1.js new file mode 100755 index 0000000000..325abb8eee --- /dev/null +++ b/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/my_project_with_bin/v1/cli_v1.js @@ -0,0 +1,4 @@ +#!/usr/bin/env node + +console.log('v1') + diff --git a/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/my_project_with_bin/v1/package.json b/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/my_project_with_bin/v1/package.json new file mode 100644 index 0000000000..c7fd1aa81d --- /dev/null +++ b/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/my_project_with_bin/v1/package.json @@ -0,0 +1,8 @@ +{ + "name": "my_project_with_bin", + "version": "1.0.0", + "private": true, + "bin": { + "myBin": "./cli_v1.js" + } +} diff --git a/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/my_project_with_bin/v2/cli_v2.js b/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/my_project_with_bin/v2/cli_v2.js new file mode 100755 index 0000000000..91ee788e4f --- /dev/null +++ b/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/my_project_with_bin/v2/cli_v2.js @@ -0,0 +1,4 @@ +#!/usr/bin/env node + +console.log('v2') + diff --git a/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/my_project_with_bin/v2/package.json b/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/my_project_with_bin/v2/package.json new file mode 100644 index 0000000000..e8e06465a4 --- /dev/null +++ b/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/my_project_with_bin/v2/package.json @@ -0,0 +1,8 @@ +{ + "name": "my_project_with_bin", + "version": "2.0.0", + "private": true, + "bin": { + "myBin": "./cli_v2.js" + } +} diff --git a/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/package.json b/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/package.json new file mode 100644 index 0000000000..eb3dc95856 --- /dev/null +++ b/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/package.json @@ -0,0 +1,7 @@ +{ + "name": "my-project", + "private": true, + "workspaces": [ + "packages/*" + ] +} diff --git a/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/packages/workspace-1/package.json b/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/packages/workspace-1/package.json new file mode 100644 index 0000000000..1364ce0bdc --- /dev/null +++ b/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/packages/workspace-1/package.json @@ -0,0 +1,8 @@ +{ + "name": "workspace-1", + "version": "1.0.0", + "private": true, + "dependencies": { + "my_project_with_bin": "file:../../my_project_with_bin/v1" + } +} diff --git a/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/packages/workspace-2/package.json b/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/packages/workspace-2/package.json new file mode 100644 index 0000000000..9583019935 --- /dev/null +++ b/__tests__/fixtures/install/workspaces-install-link-bin-2-versions/packages/workspace-2/package.json @@ -0,0 +1,8 @@ +{ + "name": "workspace-2", + "version": "1.0.0", + "private": true, + "dependencies": { + "my_project_with_bin": "file:../../my_project_with_bin/v2" + } +} diff --git a/src/cli/commands/run.js b/src/cli/commands/run.js index f0dfbf455d..c36956612c 100644 --- a/src/cli/commands/run.js +++ b/src/cli/commands/run.js @@ -31,8 +31,8 @@ export async function getBinEntries(config: Config): Promise // Setup the node_modules/.bin folders for analysis for (const registryFolder of config.registryFolders) { - binFolders.add(path.resolve(config.cwd, registryFolder, '.bin')); binFolders.add(path.resolve(config.lockfileFolder, registryFolder, '.bin')); + binFolders.add(path.resolve(config.cwd, registryFolder, '.bin')); } // Same thing, but for the pnp dependencies, located inside the cache