Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup and promise concurrency. #7124

Merged
merged 2 commits into from
Jan 23, 2024
Merged

Cleanup and promise concurrency. #7124

merged 2 commits into from
Jan 23, 2024

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
35 of 36 checks passed
@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