Skip to content

Commit 2ba09cc

Browse files
authored
fix(arborist): check if a spec is a workspace before fetching a manifest, closes #3637 (#4371)
1 parent 415a70b commit 2ba09cc

File tree

2 files changed

+132
-17
lines changed

2 files changed

+132
-17
lines changed

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

+33-17
Original file line numberDiff line numberDiff line change
@@ -1250,24 +1250,40 @@ This is a one-time fix-up, please be patient...
12501250
// Don't bother to load the manifest for link deps, because the target
12511251
// might be within another package that doesn't exist yet.
12521252
const { legacyPeerDeps } = this
1253-
return spec.type === 'directory'
1254-
? this[_linkFromSpec](name, spec, parent, edge)
1255-
: this[_fetchManifest](spec)
1256-
.then(pkg => new Node({ name, pkg, parent, legacyPeerDeps }), error => {
1257-
error.requiredBy = edge.from.location || '.'
1258-
1259-
// failed to load the spec, either because of enotarget or
1260-
// fetch failure of some other sort. save it so we can verify
1261-
// later that it's optional, otherwise the error is fatal.
1262-
const n = new Node({
1263-
name,
1264-
parent,
1265-
error,
1266-
legacyPeerDeps,
1267-
})
1268-
this[_loadFailures].add(n)
1269-
return n
1253+
1254+
// spec is a directory, link it
1255+
if (spec.type === 'directory') {
1256+
return this[_linkFromSpec](name, spec, parent, edge)
1257+
}
1258+
1259+
// if the spec matches a workspace name, then see if the workspace node will
1260+
// satisfy the edge. if it does, we return the workspace node to make sure it
1261+
// takes priority.
1262+
if (this.idealTree.workspaces && this.idealTree.workspaces.has(spec.name)) {
1263+
const existingNode = this.idealTree.edgesOut.get(spec.name).to
1264+
if (existingNode && existingNode.isWorkspace && existingNode.satisfies(edge)) {
1265+
return edge.to
1266+
}
1267+
}
1268+
1269+
// spec isn't a directory, and either isn't a workspace or the workspace we have
1270+
// doesn't satisfy the edge. try to fetch a manifest and build a node from that.
1271+
return this[_fetchManifest](spec)
1272+
.then(pkg => new Node({ name, pkg, parent, legacyPeerDeps }), error => {
1273+
error.requiredBy = edge.from.location || '.'
1274+
1275+
// failed to load the spec, either because of enotarget or
1276+
// fetch failure of some other sort. save it so we can verify
1277+
// later that it's optional, otherwise the error is fatal.
1278+
const n = new Node({
1279+
name,
1280+
parent,
1281+
error,
1282+
legacyPeerDeps,
12701283
})
1284+
this[_loadFailures].add(n)
1285+
return n
1286+
})
12711287
}
12721288

12731289
[_linkFromSpec] (name, spec, parent, edge) {

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

+99
Original file line numberDiff line numberDiff line change
@@ -877,6 +877,105 @@ t.test('workspaces', t => {
877877
t.matchSnapshot(printTree(await tree))
878878
})
879879

880+
t.test('workspace nodes are used instead of fetching manifests when they are valid', async t => {
881+
// turn off networking, this should never make a registry request
882+
nock.disableNetConnect()
883+
t.teardown(() => nock.enableNetConnect())
884+
885+
const path = t.testdir({
886+
'package.json': JSON.stringify({
887+
name: 'root',
888+
workspaces: ['workspace-a', 'workspace-b'],
889+
}),
890+
// the package-lock.json references version 1.0.0 of the workspace deps
891+
// as it would if a user hand edited their workspace's package.json and
892+
// now are attempting to reify with a stale package-lock
893+
'package-lock.json': JSON.stringify({
894+
name: 'root',
895+
lockfileVersion: 2,
896+
requires: true,
897+
packages: {
898+
'': {
899+
name: 'root',
900+
workspaces: ['workspace-a', 'workspace-b'],
901+
},
902+
'node_modules/workspace-a': {
903+
resolved: 'workspace-a',
904+
link: true,
905+
},
906+
'node_modules/workspace-b': {
907+
resolved: 'workspace-b',
908+
link: true,
909+
},
910+
'workspace-a': {
911+
name: 'workspace-a',
912+
version: '1.0.0',
913+
dependencies: {
914+
'workspace-b': '1.0.0',
915+
},
916+
},
917+
'workspace-b': {
918+
name: 'workspace-b',
919+
version: '1.0.0',
920+
},
921+
},
922+
dependencies: {
923+
'workspace-a': {
924+
version: 'file:workspace-a',
925+
requires: {
926+
'workspace-b': '1.0.0',
927+
},
928+
},
929+
'workspace-b': {
930+
version: 'file:workspace-b',
931+
},
932+
},
933+
}),
934+
node_modules: {
935+
'workspace-a': t.fixture('symlink', '../workspace-a'),
936+
'workspace-b': t.fixture('symlink', '../workspace-b'),
937+
},
938+
// the workspaces themselves are at 2.0.0 because they're what was edited
939+
'workspace-a': {
940+
'package.json': JSON.stringify({
941+
name: 'workspace-a',
942+
version: '2.0.0',
943+
dependencies: {
944+
'workspace-b': '2.0.0',
945+
},
946+
}),
947+
},
948+
'workspace-b': {
949+
'package.json': JSON.stringify({
950+
name: 'workspace-b',
951+
version: '2.0.0',
952+
}),
953+
},
954+
})
955+
956+
const arb = new Arborist({
957+
...OPT,
958+
path,
959+
workspaces: ['workspace-a', 'workspace-b'],
960+
})
961+
962+
// this will reject if we try to fetch a manifest for some reason
963+
const tree = await arb.buildIdealTree({
964+
path,
965+
})
966+
967+
const edgeA = tree.edgesOut.get('workspace-a')
968+
t.ok(edgeA.valid, 'workspace-a should be valid')
969+
const edgeB = tree.edgesOut.get('workspace-b')
970+
t.ok(edgeB.valid, 'workspace-b should be valid')
971+
const nodeA = edgeA.to.target
972+
t.ok(nodeA.isWorkspace, 'workspace-a is definitely a workspace')
973+
const nodeB = edgeB.to.target
974+
t.ok(nodeB.isWorkspace, 'workspace-b is definitely a workspace')
975+
const nodeBfromA = nodeA.edgesOut.get('workspace-b').to.target
976+
t.equal(nodeBfromA, nodeB, 'workspace-b edgeOut from workspace-a is the workspace')
977+
})
978+
880979
t.end()
881980
})
882981

0 commit comments

Comments
 (0)