Skip to content

Conversation

wraithgar
Copy link
Member

@wraithgar wraithgar commented Jan 10, 2024

This is a refactor done last year to clean up more of Arborist. I was
holding off submitting it till we could land
isaacs/promise-call-limit#12, since reify tests
randomly timed out for me without it.

Other PRs are now showing that behavior and solving the promise
concurrency / request timeout issue has now become something that cannot
wait. This PR can be where we update to that.

The diff that fixed it for me locally running off a linked version of
the promise-call-limit PR was:

diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js
index 0981afdae..d07717984 100644
--- a/workspaces/arborist/lib/arborist/reify.js
+++ b/workspaces/arborist/lib/arborist/reify.js
@@ -31,6 +31,7 @@ const relpath = require('../relpath.js')
 const Diff = require('../diff.js')
 const retirePath = require('../retire-path.js')
 const promiseAllRejectLate = require('promise-all-reject-late')
+const promiseCallLimit = require('promise-call-limit')
 const optionalSet = require('../optional-set.js')
 const calcDepFlags = require('../calc-dep-flags.js')
 const { saveTypeMap, hasSubKey } = require('../add-rm-pkg-deps.js')
@@ -817,9 +818,11 @@ module.exports = cls => class Reifier extends cls {
     }

     // extract all the nodes with bundles
-    return promiseAllRejectLate(set.map(node => {
-      this[_bundleUnpacked].add(node)
-      return this[_reifyNode](node)
+    return promiseCallLimit(set.map(node => {
+      return () => {
+        this[_bundleUnpacked].add(node)
+        return this[_reifyNode](node)
+      }
     }))
     // then load their unpacked children and move into the ideal tree
       .then(nodes =>

(In that linked code I hard set rejectLate to true)

@wraithgar wraithgar requested a review from a team as a code owner January 10, 2024 16:17
@wraithgar wraithgar changed the title fix: clean up idealTree code Cleanup and promise concurrency. Jan 10, 2024
@wraithgar wraithgar mentioned this pull request Jan 10, 2024
@wraithgar wraithgar force-pushed the gar/arborist-cleanup branch from 6e1f284 to d49c29a Compare January 19, 2024 20:19
@wraithgar wraithgar merged commit ec77e81 into latest Jan 23, 2024
@wraithgar wraithgar deleted the gar/arborist-cleanup branch January 23, 2024 20:40
@github-actions github-actions bot mentioned this pull request Jan 23, 2024
lukekarrys added a commit that referenced this pull request May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants