fix!: make HoldMyTask and its constructor aliases real classes - #11
Merged
Conversation
index.mjs exported HoldMyTask, Queue, TaskManager, TaskQueue, QueueManager, and TaskProcessor as async factory functions (createHoldMyTask() and friends) rather than the actual HoldMyTask class, so `new QueueManager()` etc. threw "QueueManager is not a constructor" - the entire CommonAliases.test.vitest.mjs suite (35/36 tests) was failing and had been worked around by skipping the whole file rather than fixed (#3). index.cjs already assumed the ESM HoldMyTask export was the real class (`module.exports = HoldMyTask`), and every README/example already used `new HoldMyTask(...)` - the async-factory pattern was the actual bug, not the tests or the CJS bridge. Switches index.mjs to a static top-level import of the real HoldMyTask class from @cldmv/holdmytask/main and re-exports it (and its aliases) directly. The createHoldMyTask/createQueue/createTaskManager/ createTaskProcessor async factory functions are kept as-is for backward compatibility, just simplified to use the now-eagerly-loaded class instead of a fresh dynamic import per call. Un-skips CommonAliases.test.vitest.mjs (all 36 tests now pass) and regenerates types/index.d.mts via `npm run build:types` to match the new export shapes.
cldmv-bot
Bot
added
the
! fix → next
Contributor
|
Resolves #3 — closed when this ships to the default branch. |
cldmv-bot
Bot
added
the
area: tests
There was a problem hiding this comment.
Pull request overview
This PR updates the package entrypoints and type declarations so HoldMyTask and the common aliases (Queue, TaskManager, etc.) are exported as real constructors (classes), aligning runtime behavior with the documented “new Queue()”-style usage and re-enabling the previously skipped alias-compatibility test suite.
Changes:
- Update
index.mjsexports to makeHoldMyTask(and alias names) refer to the underlying class instead of async factory functions. - Update
types/index.d.mtsto exportHoldMyTaskas the default and as the constructor aliases. - Unskip alias/compatibility tests and adjust generated type output for the priority stress test example.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| index.mjs | Switches exports so HoldMyTask + aliases are real class constructors; keeps async factory helpers. |
| tests/CommonAliases.test.vitest.mjs | Re-enables tests that validate alias constructor behavior and method aliases. |
| types/index.d.mts | Updates type exports to match the new runtime export surface (HoldMyTask as default + aliases). |
| types/index.d.mts.map | Regenerated sourcemap corresponding to the updated types/index.d.mts. |
| types/examples/priority-stress-test.d.mts | Updates generated type signature for the stress-test result shape. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Addresses PR #11 review feedback: the PR's auto-generated changelog claimed "No breaking changes", but cbc1ac7 changed the default export (and the HoldMyTask/Queue/TaskManager/TaskQueue/QueueManager/TaskProcessor named aliases) from an async factory function to the real HoldMyTask class. Code that previously called these as functions - e.g. `await HoldMyTask()`, or `await (await import("@cldmv/holdmytask")).default()` - now gets "Class constructor HoldMyTask cannot be invoked without 'new'" and must switch to `new HoldMyTask()` / `new QueueManager()` etc. instead. This matches the already-documented `new HoldMyTask(options)` usage throughout README and every example, and the existing CJS bridge (index.cjs), which already assumed the ESM default export was the class - only the previously-buggy async-factory calling convention on these specific export names is removed. BREAKING CHANGE: HoldMyTask, Queue, TaskManager, TaskQueue, QueueManager, TaskProcessor, and the package default export are now the real HoldMyTask class instead of an async factory function. Use `new HoldMyTask(options)` (or `new QueueManager(options)`, etc.) instead of calling them as functions. The createHoldMyTask/createQueue/createTaskManager/ createTaskProcessor named async factory functions are unchanged.
cldmv-bot
Bot
changed the title
Contributor
Author
|
Auto-normalized PR title: rewrote PR title to match the highest-priority commit type (
If this isn't what you want, edit the title — the normalizer won't re-fire as long as the title stays conventional. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
index.mjs:23
- The header comment says the devcheck “must happen before holdmytask imports”, but this module now has a static ESM import of
@cldmv/holdmytask/main. Since ESM imports are evaluated before the module body, that ordering guarantee isn’t true and the comment is misleading for future maintainers.
import { HoldMyTask } from "@cldmv/holdmytask/main";
Addresses the suppressed Copilot comment on PR #11 (index.mjs:23): the comment claimed the devcheck "must happen before holdmytask imports", but the static `import` of the core is hoisted and evaluated before the devcheck IIFE runs, so that ordering isn't real. Reworded to state devcheck is a best-effort, fire-and-forget dev-time warning that does NOT run before the core loads, and why (running it strictly first needs top-level await, which breaks index.cjs's synchronous require). Regenerated the types sourcemap (source positions shifted; the .d.ts itself is unchanged).
Contributor
|
Addressing the suppressed comment from the 2026-08-08 review ( Fixed in |
Shinrai
approved these changes
Aug 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚀 What's Changed
💥 Breaking Changes
✨ Features
No new features
🐛 Bug Fixes
📦 Dependencies
No dependency updates
🔧 Other Changes
👥 Contributors