feat: add option to set asyncmap/asyncfilter concurrency pool limit#58
feat: add option to set asyncmap/asyncfilter concurrency pool limit#58eglitise merged 9 commits intoappium:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds a concurrency pool limit option to the asyncmap and asyncfilter functions, allowing users to control the level of parallelism when processing collections. This feature aims to replace Bluebird's B.map functionality throughout the organization.
Changes:
- Added
MapFilterOptionstype that accepts boolean or{concurrency: number}to control execution mode - Implemented worker pool pattern for both
asyncmapandasyncfilterwith configurable concurrency - Updated tests to verify concurrent execution behavior and validation of invalid concurrency values
- Updated documentation with examples of sequential, parallel, and limited concurrency execution
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| lib/types.ts | Added MapFilterOptions type definition to support boolean or concurrency object options |
| lib/asyncbox.ts | Refactored asyncmap and asyncfilter to support concurrency pool limits using worker pattern |
| test/asyncbox-specs.ts | Added tests for concurrency mode, adjusted timing expectations, and added validation tests |
| README.md | Updated documentation with concurrency examples and corrected function signatures |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/asyncbox.ts
Outdated
| coll: T[], | ||
| mapper: (value: T) => R | Promise<R>, | ||
| runInParallel = true, | ||
| mapper: (value: T) => PromiseLike<R>, |
There was a problem hiding this comment.
is this a breaking change? I assume the previous implementation did not force the mapper to be a promise
There was a problem hiding this comment.
I changed the signature only because p-limit required a PromiseLike input. Ultimately the function is intended to be used only for promises, but I'll see if I can somehow adjust it to keep the signature.
There was a problem hiding this comment.
Added async wrappers for both mapper and filter, which allows to keep the previous signature. I've also added new tests to confirm this.
| "singleQuote": true | ||
| }, | ||
| "dependencies": { | ||
| "p-limit": "^7.2.0" |
There was a problem hiding this comment.
I assume the main reason for the major version bump was to get rid of any third party dependencies, however now we still add one.
Would you mind if I try to actually implement concurrent mapping and filtering as part of this module without using third party modules?
There was a problem hiding this comment.
I don't consider the removal of dependencies on its own to require a major version bump. I was merely investigating how to achieve that, and decided to modernize the package along the way, which resulted in the major bumps.
Regarding p-limit specifically, while I still maintain my stance that it's preferable to minimize third party dependencies here, there's 3 main reasons why I added it:
- Replacing it with native functionality requires somewhat non-trivial code (unlike the previous uses of
bluebirdandlodash). This is also why I have not attempted to removelodashfromteen_process, since it uses_.isBuffer, whose replacement would require more than a one-liner. - It is a very small module (12KB unpacked, compared to 632KB for
bluebirdand 1.41MB forlodash), and is purpose-built for this use case. - It comes from a trusted developer, which gives confidence in the module's coverage of any edge cases.
Hand-rolling the concurrency is certainly still possible, but given the above, I'm not sure if it's worth the effort, even with AI assistance.
|
🎉 This PR is included in version 6.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This should allow us to replace Bluebird's
B.mapthroughout the org.Worth noting that
B.filterdoes not have a concurrency option at all.