Skip to content

Commit

Permalink
fix: create links relative to the target
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@wraithgar @lukekarrys
wraithgar authored and lukekarrys committed Aug 24, 2022
1 parent ea5e3a3 commit 1e84102
Show file tree
Hide file tree
Showing 3 changed files with 223 additions and 8 deletions.
17 changes: 9 additions & 8 deletions workspaces/arborist/lib/arborist/build-ideal-tree.js
View file
Expand Up @@ -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
Expand Down
201 changes: 201 additions & 0 deletions workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs
View file
Expand Up @@ -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 {
Expand Down
13 changes: 13 additions & 0 deletions workspaces/arborist/test/arborist/build-ideal-tree.js
View file
Expand Up @@ -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({
Expand Down

0 comments on commit 1e84102

Please sign in to comment.