From 1e841029917817556207c39d25be1ea91e2959e7 Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 24 Aug 2022 07:19:31 -0700 Subject: [PATCH] fix: create links relative to the target Added link deps need to be relative to the package they're being added to, not the project root. In the past the project root was the only place you could add things but workspaces changed this. --- .../arborist/lib/arborist/build-ideal-tree.js | 17 +- .../arborist/build-ideal-tree.js.test.cjs | 201 ++++++++++++++++++ .../test/arborist/build-ideal-tree.js | 13 ++ 3 files changed, 223 insertions(+), 8 deletions(-) diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index 64d72bedacb1c..e9a8720d7322d 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -511,17 +511,18 @@ Try using the package name instead, e.g: this[_depsQueue].push(tree) } - // This returns a promise because we might not have the name yet, - // and need to call pacote.manifest to find the name. + // This returns a promise because we might not have the name yet, and need to + // call pacote.manifest to find the name. async [_add] (tree, { add, saveType = null, saveBundle = false }) { - const path = this.idealTree.target.path + // If we have a link it will need to be added relative to the target's path + const path = tree.target.path + // get the name for each of the specs in the list. - // ie, doing `foo@bar` we just return foo - // but if it's a url or git, we don't know the name until we - // fetch it and look in its manifest. + // ie, doing `foo@bar` we just return foo but if it's a url or git, we + // don't know the name until we fetch it and look in its manifest. await Promise.all(add.map(async rawSpec => { - // We do NOT provide the path to npa here, because user-additions - // need to be resolved relative to the CWD the user is in. + // We do NOT provide the path to npa here, because user-additions need to + // be resolved relative to the tree being added to. let spec = npa(rawSpec) // if it's just @'' then we reload whatever's there, or get latest diff --git a/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs b/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs index 1b1e2d55da5de..42327a9db1924 100644 --- a/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs +++ b/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs @@ -1470,6 +1470,207 @@ ArboristNode { } ` +exports[`test/arborist/build-ideal-tree.js TAP add one workspace to another > tree with workspace a added to workspace c 1`] = ` +ArboristNode { + "children": Map { + "a" => ArboristLink { + "edgesIn": Set { + EdgeIn { + "from": "", + "name": "a", + "spec": "file:{CWD}/test/fixtures/workspaces-not-root/packages/a", + "type": "workspace", + }, + EdgeIn { + "from": "packages/c", + "name": "a", + "spec": "file:../a", + "type": "prod", + }, + }, + "isWorkspace": true, + "location": "node_modules/a", + "name": "a", + "path": "{CWD}/test/fixtures/workspaces-not-root/node_modules/a", + "realpath": "{CWD}/test/fixtures/workspaces-not-root/packages/a", + "resolved": "file:../packages/a", + "target": ArboristNode { + "location": "packages/a", + }, + "version": "1.2.3", + }, + "abbrev" => ArboristNode { + "edgesIn": Set { + EdgeIn { + "from": "", + "name": "abbrev", + "spec": "*", + "type": "prod", + }, + EdgeIn { + "from": "packages/b", + "name": "abbrev", + "spec": "*", + "type": "prod", + }, + EdgeIn { + "from": "packages/c", + "name": "abbrev", + "spec": "*", + "type": "prod", + }, + }, + "location": "node_modules/abbrev", + "name": "abbrev", + "path": "{CWD}/test/fixtures/workspaces-not-root/node_modules/abbrev", + "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", + "version": "1.1.1", + }, + "b" => ArboristLink { + "edgesIn": Set { + EdgeIn { + "from": "", + "name": "b", + "spec": "file:{CWD}/test/fixtures/workspaces-not-root/packages/b", + "type": "workspace", + }, + }, + "isWorkspace": true, + "location": "node_modules/b", + "name": "b", + "path": "{CWD}/test/fixtures/workspaces-not-root/node_modules/b", + "realpath": "{CWD}/test/fixtures/workspaces-not-root/packages/b", + "resolved": "file:../packages/b", + "target": ArboristNode { + "location": "packages/b", + }, + "version": "1.2.3", + }, + "c" => ArboristLink { + "edgesIn": Set { + EdgeIn { + "from": "", + "name": "c", + "spec": "file:{CWD}/test/fixtures/workspaces-not-root/packages/c", + "type": "workspace", + }, + }, + "isWorkspace": true, + "location": "node_modules/c", + "name": "c", + "path": "{CWD}/test/fixtures/workspaces-not-root/node_modules/c", + "realpath": "{CWD}/test/fixtures/workspaces-not-root/packages/c", + "resolved": "file:../packages/c", + "target": ArboristNode { + "location": "packages/c", + }, + "version": "1.2.3", + }, + "wrappy" => ArboristNode { + "edgesIn": Set { + EdgeIn { + "from": "", + "name": "wrappy", + "spec": "1.0.0", + "type": "prod", + }, + }, + "location": "node_modules/wrappy", + "name": "wrappy", + "path": "{CWD}/test/fixtures/workspaces-not-root/node_modules/wrappy", + "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.0.tgz", + "version": "1.0.0", + }, + }, + "edgesOut": Map { + "a" => EdgeOut { + "name": "a", + "spec": "file:{CWD}/test/fixtures/workspaces-not-root/packages/a", + "to": "node_modules/a", + "type": "workspace", + }, + "abbrev" => EdgeOut { + "name": "abbrev", + "spec": "*", + "to": "node_modules/abbrev", + "type": "prod", + }, + "b" => EdgeOut { + "name": "b", + "spec": "file:{CWD}/test/fixtures/workspaces-not-root/packages/b", + "to": "node_modules/b", + "type": "workspace", + }, + "c" => EdgeOut { + "name": "c", + "spec": "file:{CWD}/test/fixtures/workspaces-not-root/packages/c", + "to": "node_modules/c", + "type": "workspace", + }, + "wrappy" => EdgeOut { + "name": "wrappy", + "spec": "1.0.0", + "to": "node_modules/wrappy", + "type": "prod", + }, + }, + "fsChildren": Set { + ArboristNode { + "isWorkspace": true, + "location": "packages/a", + "name": "a", + "path": "{CWD}/test/fixtures/workspaces-not-root/packages/a", + "version": "1.2.3", + }, + ArboristNode { + "edgesOut": Map { + "abbrev" => EdgeOut { + "name": "abbrev", + "spec": "*", + "to": "node_modules/abbrev", + "type": "prod", + }, + }, + "isWorkspace": true, + "location": "packages/b", + "name": "b", + "path": "{CWD}/test/fixtures/workspaces-not-root/packages/b", + "version": "1.2.3", + }, + ArboristNode { + "edgesOut": Map { + "a" => EdgeOut { + "name": "a", + "spec": "file:../a", + "to": "node_modules/a", + "type": "prod", + }, + "abbrev" => EdgeOut { + "name": "abbrev", + "spec": "*", + "to": "node_modules/abbrev", + "type": "prod", + }, + }, + "isWorkspace": true, + "location": "packages/c", + "name": "c", + "path": "{CWD}/test/fixtures/workspaces-not-root/packages/c", + "version": "1.2.3", + }, + }, + "isProjectRoot": true, + "location": "", + "name": "workspaces-not-root", + "path": "{CWD}/test/fixtures/workspaces-not-root", + "workspaces": Map { + "a" => "packages/a", + "b" => "packages/b", + "c" => "packages/c", + }, +} +` + exports[`test/arborist/build-ideal-tree.js TAP add packages to workspaces, not root > tree with abbrev removed from a and b 1`] = ` ArboristNode { "children": Map { diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index 87783086b65c3..0f7c5fecf4fd9 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -2674,6 +2674,19 @@ t.test('add packages to workspaces, not root', async t => { t.matchSnapshot(printTree(rmTree), 'tree with abbrev removed from a and b') }) +t.test('add one workspace to another', async t => { + const path = resolve(__dirname, '../fixtures/workspaces-not-root') + const packageA = resolve(path, 'packages/a') + + const addTree = await buildIdeal(path, { + add: [packageA], + workspaces: ['c'], + }) + const c = addTree.children.get('c').target + t.match(c.edgesOut.get('a'), { spec: 'file:../a' }) + t.matchSnapshot(printTree(addTree), 'tree with workspace a added to workspace c') +}) + t.test('workspace error handling', async t => { const path = t.testdir({ 'package.json': JSON.stringify({