Skip to content

Commit 1e84102

Browse files
wraithgarlukekarrys
wraithgar
authored and
lukekarrys
committed
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.
1 parent ea5e3a3 commit 1e84102

File tree

3 files changed

+223
-8
lines changed

3 files changed

+223
-8
lines changed

‎workspaces/arborist/lib/arborist/build-ideal-tree.js

+9-8
Original file line numberDiff line numberDiff line change
@@ -511,17 +511,18 @@ Try using the package name instead, e.g:
511511
this[_depsQueue].push(tree)
512512
}
513513

514-
// This returns a promise because we might not have the name yet,
515-
// and need to call pacote.manifest to find the name.
514+
// This returns a promise because we might not have the name yet, and need to
515+
// call pacote.manifest to find the name.
516516
async [_add] (tree, { add, saveType = null, saveBundle = false }) {
517-
const path = this.idealTree.target.path
517+
// If we have a link it will need to be added relative to the target's path
518+
const path = tree.target.path
519+
518520
// get the name for each of the specs in the list.
519-
// ie, doing `foo@bar` we just return foo
520-
// but if it's a url or git, we don't know the name until we
521-
// fetch it and look in its manifest.
521+
// ie, doing `foo@bar` we just return foo but if it's a url or git, we
522+
// don't know the name until we fetch it and look in its manifest.
522523
await Promise.all(add.map(async rawSpec => {
523-
// We do NOT provide the path to npa here, because user-additions
524-
// need to be resolved relative to the CWD the user is in.
524+
// We do NOT provide the path to npa here, because user-additions need to
525+
// be resolved relative to the tree being added to.
525526
let spec = npa(rawSpec)
526527

527528
// if it's just @'' then we reload whatever's there, or get latest

‎workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs

+201
Original file line numberDiff line numberDiff line change
@@ -1470,6 +1470,207 @@ ArboristNode {
14701470
}
14711471
`
14721472

1473+
exports[`test/arborist/build-ideal-tree.js TAP add one workspace to another > tree with workspace a added to workspace c 1`] = `
1474+
ArboristNode {
1475+
"children": Map {
1476+
"a" => ArboristLink {
1477+
"edgesIn": Set {
1478+
EdgeIn {
1479+
"from": "",
1480+
"name": "a",
1481+
"spec": "file:{CWD}/test/fixtures/workspaces-not-root/packages/a",
1482+
"type": "workspace",
1483+
},
1484+
EdgeIn {
1485+
"from": "packages/c",
1486+
"name": "a",
1487+
"spec": "file:../a",
1488+
"type": "prod",
1489+
},
1490+
},
1491+
"isWorkspace": true,
1492+
"location": "node_modules/a",
1493+
"name": "a",
1494+
"path": "{CWD}/test/fixtures/workspaces-not-root/node_modules/a",
1495+
"realpath": "{CWD}/test/fixtures/workspaces-not-root/packages/a",
1496+
"resolved": "file:../packages/a",
1497+
"target": ArboristNode {
1498+
"location": "packages/a",
1499+
},
1500+
"version": "1.2.3",
1501+
},
1502+
"abbrev" => ArboristNode {
1503+
"edgesIn": Set {
1504+
EdgeIn {
1505+
"from": "",
1506+
"name": "abbrev",
1507+
"spec": "*",
1508+
"type": "prod",
1509+
},
1510+
EdgeIn {
1511+
"from": "packages/b",
1512+
"name": "abbrev",
1513+
"spec": "*",
1514+
"type": "prod",
1515+
},
1516+
EdgeIn {
1517+
"from": "packages/c",
1518+
"name": "abbrev",
1519+
"spec": "*",
1520+
"type": "prod",
1521+
},
1522+
},
1523+
"location": "node_modules/abbrev",
1524+
"name": "abbrev",
1525+
"path": "{CWD}/test/fixtures/workspaces-not-root/node_modules/abbrev",
1526+
"resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
1527+
"version": "1.1.1",
1528+
},
1529+
"b" => ArboristLink {
1530+
"edgesIn": Set {
1531+
EdgeIn {
1532+
"from": "",
1533+
"name": "b",
1534+
"spec": "file:{CWD}/test/fixtures/workspaces-not-root/packages/b",
1535+
"type": "workspace",
1536+
},
1537+
},
1538+
"isWorkspace": true,
1539+
"location": "node_modules/b",
1540+
"name": "b",
1541+
"path": "{CWD}/test/fixtures/workspaces-not-root/node_modules/b",
1542+
"realpath": "{CWD}/test/fixtures/workspaces-not-root/packages/b",
1543+
"resolved": "file:../packages/b",
1544+
"target": ArboristNode {
1545+
"location": "packages/b",
1546+
},
1547+
"version": "1.2.3",
1548+
},
1549+
"c" => ArboristLink {
1550+
"edgesIn": Set {
1551+
EdgeIn {
1552+
"from": "",
1553+
"name": "c",
1554+
"spec": "file:{CWD}/test/fixtures/workspaces-not-root/packages/c",
1555+
"type": "workspace",
1556+
},
1557+
},
1558+
"isWorkspace": true,
1559+
"location": "node_modules/c",
1560+
"name": "c",
1561+
"path": "{CWD}/test/fixtures/workspaces-not-root/node_modules/c",
1562+
"realpath": "{CWD}/test/fixtures/workspaces-not-root/packages/c",
1563+
"resolved": "file:../packages/c",
1564+
"target": ArboristNode {
1565+
"location": "packages/c",
1566+
},
1567+
"version": "1.2.3",
1568+
},
1569+
"wrappy" => ArboristNode {
1570+
"edgesIn": Set {
1571+
EdgeIn {
1572+
"from": "",
1573+
"name": "wrappy",
1574+
"spec": "1.0.0",
1575+
"type": "prod",
1576+
},
1577+
},
1578+
"location": "node_modules/wrappy",
1579+
"name": "wrappy",
1580+
"path": "{CWD}/test/fixtures/workspaces-not-root/node_modules/wrappy",
1581+
"resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.0.tgz",
1582+
"version": "1.0.0",
1583+
},
1584+
},
1585+
"edgesOut": Map {
1586+
"a" => EdgeOut {
1587+
"name": "a",
1588+
"spec": "file:{CWD}/test/fixtures/workspaces-not-root/packages/a",
1589+
"to": "node_modules/a",
1590+
"type": "workspace",
1591+
},
1592+
"abbrev" => EdgeOut {
1593+
"name": "abbrev",
1594+
"spec": "*",
1595+
"to": "node_modules/abbrev",
1596+
"type": "prod",
1597+
},
1598+
"b" => EdgeOut {
1599+
"name": "b",
1600+
"spec": "file:{CWD}/test/fixtures/workspaces-not-root/packages/b",
1601+
"to": "node_modules/b",
1602+
"type": "workspace",
1603+
},
1604+
"c" => EdgeOut {
1605+
"name": "c",
1606+
"spec": "file:{CWD}/test/fixtures/workspaces-not-root/packages/c",
1607+
"to": "node_modules/c",
1608+
"type": "workspace",
1609+
},
1610+
"wrappy" => EdgeOut {
1611+
"name": "wrappy",
1612+
"spec": "1.0.0",
1613+
"to": "node_modules/wrappy",
1614+
"type": "prod",
1615+
},
1616+
},
1617+
"fsChildren": Set {
1618+
ArboristNode {
1619+
"isWorkspace": true,
1620+
"location": "packages/a",
1621+
"name": "a",
1622+
"path": "{CWD}/test/fixtures/workspaces-not-root/packages/a",
1623+
"version": "1.2.3",
1624+
},
1625+
ArboristNode {
1626+
"edgesOut": Map {
1627+
"abbrev" => EdgeOut {
1628+
"name": "abbrev",
1629+
"spec": "*",
1630+
"to": "node_modules/abbrev",
1631+
"type": "prod",
1632+
},
1633+
},
1634+
"isWorkspace": true,
1635+
"location": "packages/b",
1636+
"name": "b",
1637+
"path": "{CWD}/test/fixtures/workspaces-not-root/packages/b",
1638+
"version": "1.2.3",
1639+
},
1640+
ArboristNode {
1641+
"edgesOut": Map {
1642+
"a" => EdgeOut {
1643+
"name": "a",
1644+
"spec": "file:../a",
1645+
"to": "node_modules/a",
1646+
"type": "prod",
1647+
},
1648+
"abbrev" => EdgeOut {
1649+
"name": "abbrev",
1650+
"spec": "*",
1651+
"to": "node_modules/abbrev",
1652+
"type": "prod",
1653+
},
1654+
},
1655+
"isWorkspace": true,
1656+
"location": "packages/c",
1657+
"name": "c",
1658+
"path": "{CWD}/test/fixtures/workspaces-not-root/packages/c",
1659+
"version": "1.2.3",
1660+
},
1661+
},
1662+
"isProjectRoot": true,
1663+
"location": "",
1664+
"name": "workspaces-not-root",
1665+
"path": "{CWD}/test/fixtures/workspaces-not-root",
1666+
"workspaces": Map {
1667+
"a" => "packages/a",
1668+
"b" => "packages/b",
1669+
"c" => "packages/c",
1670+
},
1671+
}
1672+
`
1673+
14731674
exports[`test/arborist/build-ideal-tree.js TAP add packages to workspaces, not root > tree with abbrev removed from a and b 1`] = `
14741675
ArboristNode {
14751676
"children": Map {

‎workspaces/arborist/test/arborist/build-ideal-tree.js

+13
Original file line numberDiff line numberDiff line change
@@ -2674,6 +2674,19 @@ t.test('add packages to workspaces, not root', async t => {
26742674
t.matchSnapshot(printTree(rmTree), 'tree with abbrev removed from a and b')
26752675
})
26762676

2677+
t.test('add one workspace to another', async t => {
2678+
const path = resolve(__dirname, '../fixtures/workspaces-not-root')
2679+
const packageA = resolve(path, 'packages/a')
2680+
2681+
const addTree = await buildIdeal(path, {
2682+
add: [packageA],
2683+
workspaces: ['c'],
2684+
})
2685+
const c = addTree.children.get('c').target
2686+
t.match(c.edgesOut.get('a'), { spec: 'file:../a' })
2687+
t.matchSnapshot(printTree(addTree), 'tree with workspace a added to workspace c')
2688+
})
2689+
26772690
t.test('workspace error handling', async t => {
26782691
const path = t.testdir({
26792692
'package.json': JSON.stringify({

0 commit comments

Comments
 (0)