Skip to content

Commit

Permalink
fix: link.target setter
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
wraithgar committed Sep 8, 2022
1 parent 2db6c08 commit 548e70e
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 39 deletions.
22 changes: 0 additions & 22 deletions 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')
Expand Down Expand Up @@ -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)
Expand Down
17 changes: 0 additions & 17 deletions workspaces/arborist/test/link.js
Expand Up @@ -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')
Expand Down

0 comments on commit 548e70e

Please sign in to comment.