Skip to content

Correct pooled API return types - #13

Merged
natanelia merged 8 commits into
mainfrom
agent/fix-pooled-api-types
Aug 5, 2026
Merged

Correct pooled API return types#13
natanelia merged 8 commits into
mainfrom
agent/fix-pooled-api-types

Conversation

@natanelia

@natanelia natanelia commented Aug 5, 2026

Copy link
Copy Markdown
Owner

What changed

  • Add a public PooledApi<TProxy> type that converts every scheduled method to Promise<Awaited<TResult>>.
  • Type WorkerPool#getApi() with that scheduled surface while preserving the existing runtime constructor and instance identity.
  • Propagate the corrected surface through useWorkerPool, including its tracked call() helper.
  • Omit the reserved string key then and all symbol keys from the scheduled proxy type; those methods remain available through WorkerPool#run() where applicable.
  • Extend packed-consumer type checks across React 17, 18, and 19 with synchronous APIs and negative @ts-expect-error assertions.

Why

getApi() always returns Promise-producing scheduled methods at runtime, but its public declaration previously returned the raw proxy type. A synchronous proxy method could therefore compile as a synchronous value and fail at runtime. The declaration also exposed then and symbol members that the runtime proxy deliberately suppresses.

Validation

  • Validated the constructor/type adapter and React hook surface with a strict standalone TypeScript 5.8 harness.
  • Verified synchronous methods type as Promise results, while then and symbol access fail type checking.
  • Added the same assertions to the repository's packed-package consumer test for React 17, 18, and 19.
  • All nine GitHub Actions jobs pass, including lint, typecheck, unit tests, coverage, security audit, Changesets version preview, builds/package validation, Node 18 consumers, and Chromium/Firefox/WebKit browser tests.

Summary by CodeRabbit

  • Improvements
    • Improved TypeScript support for pooled worker APIs, including accurate Promise-based return types for scheduled calls.
    • Added stronger typing for worker pool constructors, APIs, and React hook results.
    • Excluded reserved then and symbol properties from generated APIs to prevent invalid or unexpected usage.
    • Improved type coverage for synchronous APIs, tracked calls, and returned results.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@natanelia, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 59 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b9e0fe52-c61b-411d-b724-aba13343ab1d

📥 Commits

Reviewing files that changed from the base of the PR and between 7e5bc0d and 5127afe.

📒 Files selected for processing (2)
  • packages/comlink-worker-pool/src/index.ts
  • scripts/smoke-package-consumer.mjs
📝 Walkthrough

Walkthrough

The PR adds Promise-aware pooled API typings, updates useWorkerPool to use those types, restricts reserved keys, and expands the TypeScript consumer smoke test.

Changes

Typed pooled API contracts

Layer / File(s) Summary
Public pool API contract
packages/comlink-worker-pool/src/index.ts, .changeset/typed-pools-return.md
PooledApi maps callable proxy methods to Promise-returning methods and excludes reserved keys. WorkerPool exposes generic typed construction and explicit exports. The changeset records patch releases.
React and consumer typing
packages/comlink-worker-pool-react/src/useWorkerPool.ts, scripts/smoke-package-consumer.mjs
useWorkerPool derives API and call signatures from PooledApi and invokes methods with Reflect.apply. The consumer fixture checks scheduled results, synchronous APIs, restricted keys, tracked calls, and expected type errors.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change to pooled API return types.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/fix-pooled-api-types

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@natanelia
natanelia marked this pull request as ready for review August 5, 2026 04:18

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/comlink-worker-pool/src/index.ts`:
- Around line 12-20: Update PooledApi to explicitly override then with a
non-callable type after the mapped type, covering string-indexed proxy APIs
while preserving existing callable members. In
scripts/smoke-package-consumer.mjs, add negative consumer assertions verifying
that both direct and tracked then calls are rejected.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 68eb3a2f-8be8-4ab0-9ccb-f7de778a2e25

📥 Commits

Reviewing files that changed from the base of the PR and between 42d2459 and 7e5bc0d.

📒 Files selected for processing (4)
  • .changeset/typed-pools-return.md
  • packages/comlink-worker-pool-react/src/useWorkerPool.ts
  • packages/comlink-worker-pool/src/index.ts
  • scripts/smoke-package-consumer.mjs

Comment thread packages/comlink-worker-pool/src/index.ts
@natanelia
natanelia merged commit 2f9c7b3 into main Aug 5, 2026
10 checks passed
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.

1 participant