From 548e70e0f87167fb96929b29787620391a77b826 Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 24 Aug 2022 13:58:26 -0700 Subject: [PATCH] fix: link.target setter There were guards in place to protect when setting a promise as a target, which is not something the code actually does, which was discovered after unpacking load-actual. This removes those guards which are dangerous because either an attribute is a promise or it's not. Letting things access attributes as non-promises that are sometimes promises is dangerous. --- workspaces/arborist/lib/link.js | 22 ---------------------- workspaces/arborist/test/link.js | 17 ----------------- 2 files changed, 39 deletions(-) diff --git a/workspaces/arborist/lib/link.js b/workspaces/arborist/lib/link.js index d58c6e2375099..ebdbc94285f1c 100644 --- a/workspaces/arborist/lib/link.js +++ b/workspaces/arborist/lib/link.js @@ -1,4 +1,3 @@ -const debug = require('./debug.js') const relpath = require('./relpath.js') const Node = require('./node.js') const _loadDeps = Symbol.for('Arborist.Node._loadDeps') @@ -53,27 +52,6 @@ class Link extends Node { return } - if (current && current.then) { - debug(() => { - throw Object.assign(new Error('cannot set target while awaiting'), { - path: this.path, - realpath: this.realpath, - }) - }) - } - - if (target && target.then) { - // can set to a promise during an async tree build operation - // wait until then to assign it. - this[_target] = target - // eslint-disable-next-line promise/always-return, promise/catch-or-return - target.then(node => { - this[_target] = null - this.target = node - }) - return - } - if (!target) { if (current && current.linksIn) { current.linksIn.delete(this) diff --git a/workspaces/arborist/test/link.js b/workspaces/arborist/test/link.js index b9b9f232847e0..ec648eda256d4 100644 --- a/workspaces/arborist/test/link.js +++ b/workspaces/arborist/test/link.js @@ -94,23 +94,6 @@ t.test('link.target setter', async t => { t.equal(oldTarget.linksIn.size, 0, 'old target still has no links') t.equal(newTarget.linksIn.size, 0, 'new target has no links in now') - const laterTarget = new Promise((res) => - setTimeout(() => res(new Node({ - path: '/node-later', - realpath: '/node-later', - pkg: { name: 'node-later', version: '1.2.3' }, - })))) - link.target = laterTarget - t.equal(link.target, laterTarget, 'waiting for a new target to resolve') - t.throws(() => link.target = oldTarget, { - message: 'cannot set target while awaiting', - path: link.path, - realpath: link.realpath, - }) - const node = await laterTarget - t.equal(link.target, node, 'target resolved and assigned') - t.equal(link.package, node.package, 'took on new targets package') - t.equal(node.linksIn.has(link), true, 'link in node.linksIn') link.target = null t.equal(link.target, null, 'target is now null') t.strictSame(link.package, {}, 'removed target, package is now empty')