Skip to content

Commit e9a2981

Browse files
ruyadornoruyadorno
authored
fix(arborist): save workspace version (#4578)
When declaring dependencies to workspaces the common practice is to refer to their version numbers, currently the cli adds a link reference instead of the proper semver range when trying to install/declare as a direct direct dependency one of its own workspaces. This change fixes it by adding a new condition for handling workspace edges when saving the current ideal tree. Relates to: #3403
1 parent 84d1921 commit e9a2981

3 files changed

Lines changed: 258 additions & 12 deletions

File tree

‎workspaces/arborist/lib/arborist/reify.js‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1225,14 +1225,23 @@ module.exports = cls => class Reifier extends cls {
12251225
newSpec = h.shortcut(opt)
12261226
}
12271227
} else if (isLocalDep) {
1228-
// save the relative path in package.json
1229-
// Normally saveSpec is updated with the proper relative
1230-
// path already, but it's possible to specify a full absolute
1231-
// path initially, in which case we can end up with the wrong
1232-
// thing, so just get the ultimate fetchSpec and relativize it.
1233-
const p = req.fetchSpec.replace(/^file:/, '')
1234-
const rel = relpath(addTree.realpath, p)
1235-
newSpec = `file:${rel}`
1228+
// when finding workspace nodes, make sure that
1229+
// we save them using their version instead of
1230+
// using their relative path
1231+
if (edge.type === 'workspace') {
1232+
const { version } = edge.to.target
1233+
const prefixRange = version ? this[_savePrefix] + version : '*'
1234+
newSpec = prefixRange
1235+
} else {
1236+
// save the relative path in package.json
1237+
// Normally saveSpec is updated with the proper relative
1238+
// path already, but it's possible to specify a full absolute
1239+
// path initially, in which case we can end up with the wrong
1240+
// thing, so just get the ultimate fetchSpec and relativize it.
1241+
const p = req.fetchSpec.replace(/^file:/, '')
1242+
const rel = relpath(addTree.realpath, p)
1243+
newSpec = `file:${rel}`
1244+
}
12361245
} else {
12371246
newSpec = req.saveSpec
12381247
}

‎workspaces/arborist/tap-snapshots/test/arborist/reify.js.test.cjs‎

Lines changed: 230 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,235 @@ exports[`test/arborist/reify.js TAP add a new pkg to a prefix that needs to be m
240240

241241
`
242242

243+
exports[`test/arborist/reify.js TAP add deps to workspaces add a to root > lockfile added workspace as dep 1`] = `
244+
Object {
245+
"lockfileVersion": 3,
246+
"name": "tap-testdir-reify-add-deps-to-workspaces-add-a-to-root",
247+
"packages": Object {
248+
"": Object {
249+
"dependencies": Object {
250+
"a": "^1.2.3",
251+
"mkdirp": "^1.0.4",
252+
},
253+
"workspaces": Array [
254+
"packages/*",
255+
],
256+
},
257+
"node_modules/a": Object {
258+
"link": true,
259+
"resolved": "packages/a",
260+
},
261+
"node_modules/b": Object {
262+
"link": true,
263+
"resolved": "packages/b",
264+
},
265+
"node_modules/minimist": Object {
266+
"integrity": "sha512-FM9nNUYrRBAELZQT3xeZQ7fmMOBg6nWNmJKTcgsJeaLstP/UODVpGsr5OhXhhXg6f+qtJ8uiZ+PUxkDWcgIXLw==",
267+
"resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.5.tgz",
268+
"version": "1.2.5",
269+
},
270+
"node_modules/mkdirp": Object {
271+
"bin": Object {
272+
"mkdirp": "bin/cmd.js",
273+
},
274+
"engines": Object {
275+
"node": ">=10",
276+
},
277+
"integrity": "sha512-vVqVZQyf3WLx2Shd0qJ9xuvqgAyKPLAiqITEtqW0oIUjzo3PePDd6fW9iFz30ef7Ysp/oiWqbhszeGWW2T6Gzw==",
278+
"resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-1.0.4.tgz",
279+
"version": "1.0.4",
280+
},
281+
"packages/a": Object {
282+
"dependencies": Object {
283+
"mkdirp": "^0.5.0",
284+
},
285+
"version": "1.2.3",
286+
},
287+
"packages/a/node_modules/mkdirp": Object {
288+
"bin": Object {
289+
"mkdirp": "bin/cmd.js",
290+
},
291+
"dependencies": Object {
292+
"minimist": "^1.2.5",
293+
},
294+
"integrity": "sha512-NKmAlESf6jMGym1++R0Ra7wvhV+wFW63FaSOFPwRahvea0gMUcGUhVeAg/0BC0wiv9ih5NYPB1Wn1UEI1/L+xQ==",
295+
"resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.5.tgz",
296+
"version": "0.5.5",
297+
},
298+
"packages/b": Object {
299+
"version": "1.2.3",
300+
},
301+
},
302+
"requires": true,
303+
}
304+
`
305+
306+
exports[`test/arborist/reify.js TAP add deps to workspaces add a to root > package.json added workspace as dep 1`] = `
307+
Object {
308+
"dependencies": Object {
309+
"a": "^1.2.3",
310+
"mkdirp": "^1.0.4",
311+
},
312+
"workspaces": Array [
313+
"packages/*",
314+
],
315+
}
316+
`
317+
318+
exports[`test/arborist/reify.js TAP add deps to workspaces add a to root > returned tree 1`] = `
319+
ArboristNode {
320+
"children": Map {
321+
"a" => ArboristLink {
322+
"edgesIn": Set {
323+
EdgeIn {
324+
"from": "",
325+
"name": "a",
326+
"spec": "file:{CWD}/test/arborist/tap-testdir-reify-add-deps-to-workspaces-add-a-to-root/packages/a",
327+
"type": "workspace",
328+
},
329+
},
330+
"isWorkspace": true,
331+
"location": "node_modules/a",
332+
"name": "a",
333+
"path": "{CWD}/test/arborist/tap-testdir-reify-add-deps-to-workspaces-add-a-to-root/node_modules/a",
334+
"realpath": "{CWD}/test/arborist/tap-testdir-reify-add-deps-to-workspaces-add-a-to-root/packages/a",
335+
"resolved": "file:../packages/a",
336+
"target": ArboristNode {
337+
"location": "packages/a",
338+
},
339+
"version": "1.2.3",
340+
},
341+
"b" => ArboristLink {
342+
"edgesIn": Set {
343+
EdgeIn {
344+
"from": "",
345+
"name": "b",
346+
"spec": "file:{CWD}/test/arborist/tap-testdir-reify-add-deps-to-workspaces-add-a-to-root/packages/b",
347+
"type": "workspace",
348+
},
349+
},
350+
"isWorkspace": true,
351+
"location": "node_modules/b",
352+
"name": "b",
353+
"path": "{CWD}/test/arborist/tap-testdir-reify-add-deps-to-workspaces-add-a-to-root/node_modules/b",
354+
"realpath": "{CWD}/test/arborist/tap-testdir-reify-add-deps-to-workspaces-add-a-to-root/packages/b",
355+
"resolved": "file:../packages/b",
356+
"target": ArboristNode {
357+
"location": "packages/b",
358+
},
359+
"version": "1.2.3",
360+
},
361+
"minimist" => ArboristNode {
362+
"edgesIn": Set {
363+
EdgeIn {
364+
"from": "packages/a/node_modules/mkdirp",
365+
"name": "minimist",
366+
"spec": "^1.2.5",
367+
"type": "prod",
368+
},
369+
},
370+
"location": "node_modules/minimist",
371+
"name": "minimist",
372+
"path": "{CWD}/test/arborist/tap-testdir-reify-add-deps-to-workspaces-add-a-to-root/node_modules/minimist",
373+
"resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.5.tgz",
374+
"version": "1.2.5",
375+
},
376+
"mkdirp" => ArboristNode {
377+
"edgesIn": Set {
378+
EdgeIn {
379+
"from": "",
380+
"name": "mkdirp",
381+
"spec": "^1.0.4",
382+
"type": "prod",
383+
},
384+
},
385+
"location": "node_modules/mkdirp",
386+
"name": "mkdirp",
387+
"path": "{CWD}/test/arborist/tap-testdir-reify-add-deps-to-workspaces-add-a-to-root/node_modules/mkdirp",
388+
"resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-1.0.4.tgz",
389+
"version": "1.0.4",
390+
},
391+
},
392+
"edgesOut": Map {
393+
"a" => EdgeOut {
394+
"name": "a",
395+
"spec": "file:{CWD}/test/arborist/tap-testdir-reify-add-deps-to-workspaces-add-a-to-root/packages/a",
396+
"to": "node_modules/a",
397+
"type": "workspace",
398+
},
399+
"b" => EdgeOut {
400+
"name": "b",
401+
"spec": "file:{CWD}/test/arborist/tap-testdir-reify-add-deps-to-workspaces-add-a-to-root/packages/b",
402+
"to": "node_modules/b",
403+
"type": "workspace",
404+
},
405+
"mkdirp" => EdgeOut {
406+
"name": "mkdirp",
407+
"spec": "^1.0.4",
408+
"to": "node_modules/mkdirp",
409+
"type": "prod",
410+
},
411+
},
412+
"fsChildren": Set {
413+
ArboristNode {
414+
"children": Map {
415+
"mkdirp" => ArboristNode {
416+
"edgesIn": Set {
417+
EdgeIn {
418+
"from": "packages/a",
419+
"name": "mkdirp",
420+
"spec": "^0.5.0",
421+
"type": "prod",
422+
},
423+
},
424+
"edgesOut": Map {
425+
"minimist" => EdgeOut {
426+
"name": "minimist",
427+
"spec": "^1.2.5",
428+
"to": "node_modules/minimist",
429+
"type": "prod",
430+
},
431+
},
432+
"location": "packages/a/node_modules/mkdirp",
433+
"name": "mkdirp",
434+
"path": "{CWD}/test/arborist/tap-testdir-reify-add-deps-to-workspaces-add-a-to-root/packages/a/node_modules/mkdirp",
435+
"resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.5.tgz",
436+
"version": "0.5.5",
437+
},
438+
},
439+
"edgesOut": Map {
440+
"mkdirp" => EdgeOut {
441+
"name": "mkdirp",
442+
"spec": "^0.5.0",
443+
"to": "packages/a/node_modules/mkdirp",
444+
"type": "prod",
445+
},
446+
},
447+
"isWorkspace": true,
448+
"location": "packages/a",
449+
"name": "a",
450+
"path": "{CWD}/test/arborist/tap-testdir-reify-add-deps-to-workspaces-add-a-to-root/packages/a",
451+
"version": "1.2.3",
452+
},
453+
ArboristNode {
454+
"isWorkspace": true,
455+
"location": "packages/b",
456+
"name": "b",
457+
"path": "{CWD}/test/arborist/tap-testdir-reify-add-deps-to-workspaces-add-a-to-root/packages/b",
458+
"version": "1.2.3",
459+
},
460+
},
461+
"isProjectRoot": true,
462+
"location": "",
463+
"name": "tap-testdir-reify-add-deps-to-workspaces-add-a-to-root",
464+
"path": "{CWD}/test/arborist/tap-testdir-reify-add-deps-to-workspaces-add-a-to-root",
465+
"workspaces": Map {
466+
"a" => "packages/a",
467+
"b" => "packages/b",
468+
},
469+
}
470+
`
471+
243472
exports[`test/arborist/reify.js TAP add deps to workspaces add mkdirp 0.5.0 to b > lockfile 1`] = `
244473
Object {
245474
"dependencies": Object {
@@ -33222,7 +33451,7 @@ Object {
3322233451
"a": "github:foo/bar#baz",
3322333452
"b": "^1.2.3",
3322433453
"d": "npm:c@1.x <1.9.9",
33225-
"e": "file:e",
33454+
"e": "*",
3322633455
"f": "git+https://user:pass@github.com/baz/quux.git#asdf",
3322733456
"g": "*",
3322833457
"h": "~1.2.3",

‎workspaces/arborist/test/arborist/reify.js‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -894,8 +894,7 @@ t.test('saving the ideal tree', t => {
894894
a: 'git+ssh://git@github.com:foo/bar#baz',
895895
b: '',
896896
d: 'npm:c@1.x <1.9.9',
897-
// XXX: should we remove dependencies that are also workspaces?
898-
e: 'file:e',
897+
e: '*',
899898
f: 'git+https://user:pass@github.com/baz/quux#asdf',
900899
g: '',
901900
h: '~1.2.3',
@@ -1028,7 +1027,7 @@ t.test('saving the ideal tree', t => {
10281027
a: 'github:foo/bar#baz',
10291028
b: '^1.2.3',
10301029
d: 'npm:c@1.x <1.9.9',
1031-
e: 'file:e',
1030+
e: '*',
10321031
f: 'git+https://user:pass@github.com/baz/quux.git#asdf',
10331032
g: '*',
10341033
h: '~1.2.3',
@@ -2045,6 +2044,15 @@ t.test('add deps to workspaces', async t => {
20452044
t.matchSnapshot(require(path + '/packages/a/package.json'), 'package.json a')
20462045
t.matchSnapshot(require(path + '/package-lock.json'), 'lockfile')
20472046
})
2047+
2048+
t.test('add a to root', async t => {
2049+
const path = t.testdir(fixture)
2050+
await reify(path)
2051+
const tree = await reify(path, { add: ['a'], lockfileVersion: 3 })
2052+
t.matchSnapshot(printTree(tree), 'returned tree')
2053+
t.matchSnapshot(require(path + '/package.json'), 'package.json added workspace as dep')
2054+
t.matchSnapshot(require(path + '/package-lock.json'), 'lockfile added workspace as dep')
2055+
})
20482056
})
20492057

20502058
t.test('reify audit only workspace deps when reifying workspace', async t => {

0 commit comments

Comments
 (0)