diff --git a/CHANGELOG.md b/CHANGELOG.md index d287c78698..e74725d637 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ Please add one entry in this file for each change in Yarn's behavior. Use the sa ## Master +- Correctly installs workspace child dependencies when workspace child not symlinked to root. + + [#7289](https://github.com/yarnpkg/yarn/pull/7289) - [**Daniel Tschinder**](https://github.com/danez) + - Makes running scripts with Plug'n Play possible on node 13 [#7650](https://github.com/yarnpkg/yarn/pull/7650) - [**Sander Verweij**](https://github.com/sverweij) diff --git a/__tests__/commands/install/workspaces-install.js b/__tests__/commands/install/workspaces-install.js index a0e0c0be15..5181352fc3 100644 --- a/__tests__/commands/install/workspaces-install.js +++ b/__tests__/commands/install/workspaces-install.js @@ -103,6 +103,25 @@ test.concurrent('install should install unhoistable dependencies in workspace no }); }); +test.concurrent( + 'install should install unhoistable dependencies in workspace node_modules even when no symlink exists', + (): Promise => { + return runInstall({}, 'workspaces-install-conflict-without-symlink', async (config): Promise => { + // node_modules/isarray@1.0.0 + let packageFile = await fs.readFile(path.join(config.cwd, 'node_modules', 'isarray', 'package.json')); + expect(JSON.parse(packageFile).version).toEqual('1.0.0'); + + // node_modules/arrify@1.0.0 + packageFile = await fs.readFile(path.join(config.cwd, 'node_modules', 'arrify', 'package.json')); + expect(JSON.parse(packageFile).version).toEqual('1.0.0'); + + // node_modules/arrify/isarray@2.0.0 + packageFile = await fs.readFile(path.join(config.cwd, 'arrify', 'node_modules', 'isarray', 'package.json')); + expect(JSON.parse(packageFile).version).toEqual('2.0.0'); + }); + }, +); + test.concurrent('install should link workspaces that refer each other', (): Promise => { return runInstall({}, 'workspaces-install-link', async (config): Promise => { // packages/workspace-1/node_modules/left-pad - missing because it is hoisted to the root diff --git a/__tests__/fixtures/install/workspaces-install-conflict-without-symlink/arrify/package.json b/__tests__/fixtures/install/workspaces-install-conflict-without-symlink/arrify/package.json new file mode 100644 index 0000000000..6bd08c8c85 --- /dev/null +++ b/__tests__/fixtures/install/workspaces-install-conflict-without-symlink/arrify/package.json @@ -0,0 +1,7 @@ +{ + "name": "arrify", + "version": "2.0.0", + "dependencies": { + "isarray": "2.0.0" + } +} diff --git a/__tests__/fixtures/install/workspaces-install-conflict-without-symlink/package.json b/__tests__/fixtures/install/workspaces-install-conflict-without-symlink/package.json new file mode 100644 index 0000000000..01b65e9fa7 --- /dev/null +++ b/__tests__/fixtures/install/workspaces-install-conflict-without-symlink/package.json @@ -0,0 +1,11 @@ +{ + "name": "my-project", + "private": true, + "dependencies": { + "arrify": "1.0.0", + "isarray": "1.0.0" + }, + "workspaces": [ + "arrify" + ] +} diff --git a/__tests__/fixtures/request-cache/GET/registry.yarnpkg.com/arrify/-/arrify-1.0.0.tgz.bin b/__tests__/fixtures/request-cache/GET/registry.yarnpkg.com/arrify/-/arrify-1.0.0.tgz.bin new file mode 100644 index 0000000000..2f5b1d63b4 Binary files /dev/null and b/__tests__/fixtures/request-cache/GET/registry.yarnpkg.com/arrify/-/arrify-1.0.0.tgz.bin differ diff --git a/__tests__/fixtures/request-cache/GET/registry.yarnpkg.com/isarray/-/isarray-2.0.0.tgz.bin b/__tests__/fixtures/request-cache/GET/registry.yarnpkg.com/isarray/-/isarray-2.0.0.tgz.bin new file mode 100644 index 0000000000..55b2d8eec0 Binary files /dev/null and b/__tests__/fixtures/request-cache/GET/registry.yarnpkg.com/isarray/-/isarray-2.0.0.tgz.bin differ diff --git a/src/package-hoister.js b/src/package-hoister.js index 84a873184a..dc8990018d 100644 --- a/src/package-hoister.js +++ b/src/package-hoister.js @@ -835,6 +835,15 @@ export default class PackageHoister { // up of modules from different registries so we need to handle this specially const parts: Array = []; const keyParts = key.split('#'); + const isWorkspaceEntry = this.workspaceLayout && keyParts[0] === this.workspaceLayout.virtualManifestName; + + // Don't add the virtual manifest (keyParts.length === 1) + // or ws childs which were not hoisted to the root (keyParts.length === 2). + // If a ws child was hoisted its key would not contain the virtual manifest name + if (isWorkspaceEntry && keyParts.length <= 2) { + continue; + } + for (let i = 0; i < keyParts.length; i++) { const key = keyParts.slice(0, i + 1).join('#'); const hoisted = this.tree.get(key); @@ -843,16 +852,26 @@ export default class PackageHoister { parts.push(keyParts[i]); } - const shallowLocs = []; - if (this.config.modulesFolder) { - // remove the first part which will be the folder name and replace it with a - // hardcoded modules folder - parts.splice(0, 1, this.config.modulesFolder); + // Check if the destination is pointing to a sub folder of the virtualManifestName + // e.g. _project_/node_modules/workspace-aggregator-123456/node_modules/workspaceChild/node_modules/dependency + // This probably happened because the hoister was not able to hoist the workspace child to the root + // So we have to change the folder to the workspace package location + if (this.workspaceLayout && isWorkspaceEntry) { + const wspPkg = this.workspaceLayout.workspaces[keyParts[1]]; + invariant(wspPkg, `expected workspace package to exist for "${keyParts[1]}"`); + parts.splice(0, 4, wspPkg.loc); } else { - // first part will be the registry-specific module folder - parts.splice(0, 0, this.config.lockfileFolder); + if (this.config.modulesFolder) { + // remove the first part which will be the folder name and replace it with a + // hardcoded modules folder + parts.splice(0, 1, this.config.modulesFolder); + } else { + // first part will be the registry-specific module folder + parts.splice(0, 0, this.config.lockfileFolder); + } } + const shallowLocs = []; info.shallowPaths.forEach(shallowPath => { const shallowCopyParts = parts.slice(); shallowCopyParts[0] = this.config.cwd; diff --git a/src/package-linker.js b/src/package-linker.js index 66ae879872..f66bda80fa 100644 --- a/src/package-linker.js +++ b/src/package-linker.js @@ -251,10 +251,6 @@ export default class PackageLinker { } else if (workspaceLayout && remote.type === 'workspace' && !isShallow) { src = remote.reference; type = 'symlink'; - if (dest.indexOf(workspaceLayout.virtualManifestName) !== -1) { - // we don't need to install virtual manifest - continue; - } // to get real path for non hoisted dependencies symlinkPaths.set(dest, src); } else {