New package: Task Herder #377
Conversation
Test report for task-herder14 tests 14 ✅ 0s ⏱️ Results for commit 3bc929c. ♻️ This comment has been updated with latest results. |
Test report for scratch-vm 1 files ± 0 763 suites - 7 1m 5s ⏱️ ±0s Results for commit 3bc929c. ± Comparison against base commit 8326b04. This pull request removes 20 tests.♻️ This comment has been updated with latest results. |
cwillisf
force-pushed
the
task-herder
branch
from
71b457f to
f54154d
Compare
cwillisf
force-pushed
the
task-herder
branch
from
e5c6c1e to
1532391
Compare
cwillisf
requested review from
AndyForest,
KManolov3,
adzhindzhi,
colbygk and
Copilot
and removed request for
a team
There was a problem hiding this comment.
Pull Request Overview
This PR extracts the TaskQueue functionality from scratch-vm into a new standalone package @scratch/task-herder with several improvements including TypeScript support, modern build tooling (Vite/Rolldown), comprehensive test coverage (100%), and better API design. The package implements a token bucket algorithm for rate limiting combined with configurable concurrency control.
Key Changes:
- New package
@scratch/task-herderwith TypeScript implementation replacing the old JavaScriptTaskQueueinscratch-vm - Modern build configuration using Vite/Rolldown instead of traditional Webpack
- Enhanced API with AbortSignal support and improved error handling
- Comprehensive test suite with 6 test files achieving 100% coverage
Reviewed Changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
packages/task-herder/src/TaskHerder.ts |
Main implementation of the token bucket + concurrency limiter |
packages/task-herder/src/PromiseWithResolvers.ts |
Polyfill for Promise.withResolvers |
packages/task-herder/test/*.test.ts |
Comprehensive test suites for all functionality |
packages/task-herder/package.json |
Package configuration with modern dependencies |
packages/task-herder/vite.config.js |
Vite build configuration with Rolldown |
packages/task-herder/README.md |
Usage documentation and examples |
packages/scratch-vm/src/util/task-queue.js |
Removed old implementation |
.editorconfig |
Removed global indent_size setting |
package.json |
Added new workspace and globals dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
BREAKING CHANGE: the code in `scratch-vm/src/util/task-queue` as been migrated to `@scratch/task-herder` and any references will need to be updated. The new API supports all old use cases, but some terminology has changed.
The `abortSignal` name is more descriptive, but using `signal` instead means that the API is more consistent with `fetch`. Since I expect this to be used with `fetch` quite a bit, I think the consistency is worthwhile.
cwillisf
force-pushed
the
task-herder
branch
from
2c92c6a to
569d064
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This also makes the class name less ambiguous: I'm working on an additional feature that groups queues together, and I didn't want to end up in a situation where I'm talking about a HerdHerd. That's the kind of silliness that belongs in lunchtime conversations, not debugging sessions :D
Sorry for assigning this to so many people, but I'm not sure who has time & scope to review this. If you don't, no worries! If you do, come see the cool bike shed I made! 😅
Resolves
In support of POD-238
Proposed Changes
Extract
TaskQueuefromscratch-vminto its own package,@scratch/task-herder, with some improvements. The new package is still part of thescratch-editormono-repository.Reason for Changes
I want to reuse this code in
scratch-storage, which implies moving it to a common location.Test Coverage
There are 6 test suites totalling 100% coverage:
Since this is the first
scratch-editorcomponent using these build tools, I also wanted to check the CI behavior. To do that, I intentionally checked in failing tests during the draft phase of this PR. Check the GHA history of this branch to see how that went. These tests prompted me to make some changes, but I'm happy with where it ended up.