From 24004d26abb65ef49d0ada1d7e64c186e64e10c9 Mon Sep 17 00:00:00 2001 From: Salvador Jacobi Date: Wed, 12 Jan 2022 14:58:16 +0100 Subject: [PATCH 1/2] test: no errors for nested deps Dependencies nested in node_modules triggers an issue with a file path used as a cache key in the _findMissingEdges function on Windows due to paths with mixed path separtors. --- .../arborist/test/arborist/load-actual.js | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/workspaces/arborist/test/arborist/load-actual.js b/workspaces/arborist/test/arborist/load-actual.js index 0da044c5aee4a..13203c40250c9 100644 --- a/workspaces/arborist/test/arborist/load-actual.js +++ b/workspaces/arborist/test/arborist/load-actual.js @@ -422,3 +422,48 @@ t.test('load global space with link deps', async t => { }, }) }) + +t.test('no edge errors for nested deps', async t => { + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'a', + version: '1.0.0', + dependencies: { + b: '1.0.0', + }, + }), + node_modules: { + b: { + 'package.json': JSON.stringify({ + name: 'b', + version: '1.0.0', + dependencies: { + c: '1.0.0', + }, + }), + }, + c: { + 'package.json': JSON.stringify({ + name: 'c', + version: '1.0.0', + }), + }, + }, + }) + + // disable treeCheck since it prevents the original issue from occuring + const ArboristNoTreeCheck = t.mock('../../lib/arborist', { + '../../lib/tree-check.js': tree => tree, + }) + const loadActualNoTreeCheck = (path, opts) => + new ArboristNoTreeCheck({ path, ...opts }).loadActual(opts) + + const tree = await loadActualNoTreeCheck(path) + + // assert that no outgoing edges have errors + for (const node of tree.inventory.values()) { + for (const [name, edge] of node.edgesOut.entries()) { + t.equal(edge.error, null, `node ${node.name} has outgoing edge to ${name} with error ${edge.error}`) + } + } +}) From db6427d931fec1b554cc9451cda24ac221edecb1 Mon Sep 17 00:00:00 2001 From: Salvador Jacobi Date: Wed, 12 Jan 2022 15:02:44 +0100 Subject: [PATCH 2/2] fix: normalize path used as cache key Normalize path to avoid mixed path separators on Windows causing a cache miss for paths that refer to the same thing. This would otherwise cause a new node with the path of an existing node to be created which in turns deletes the existing node and its children. However, the children will not be loaded again from the file system due to the _actualTreeLoaded check in _loadFSTree. --- workspaces/arborist/lib/arborist/load-actual.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workspaces/arborist/lib/arborist/load-actual.js b/workspaces/arborist/lib/arborist/load-actual.js index 0d260858d81c6..f70983f87b98b 100644 --- a/workspaces/arborist/lib/arborist/load-actual.js +++ b/workspaces/arborist/lib/arborist/load-actual.js @@ -430,7 +430,7 @@ module.exports = cls => class ActualLoader extends cls { if (d.dummy) { // it's a placeholder, so likely would not have loaded this dep, // unless another dep in the tree also needs it. - const depPath = `${p}/node_modules/${name}` + const depPath = normalize(`${p}/node_modules/${name}`) const cached = this[_cache].get(depPath) if (!cached || cached.dummy) { depPromises.push(this[_loadFSNode]({