Conversation
Bring context-api to parity with effection v4-1-alpha's API system by allowing API members to be any kind of value — not just Operations. - Sync functions are lifted to Operation-returning functions - Plain constants are lifted to Operations - Middleware type matches the handler's raw signature (sync stays sync) - Added isOperation() runtime guard with native iterable exclusion - Operation check comes before function check in type mapping to prevent misclassification of Operation implementations
📝 WalkthroughWalkthroughRefactors context-api types and runtime to treat functions/constants as Operations when needed, changes middleware insertion order for Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@context-api/mod.ts`:
- Around line 152-164: The isOperation type guard misidentifies TypedArray
instances (e.g., Uint8Array, Int32Array) as Operations because they implement
[Symbol.iterator]; update the guard to exclude them by adding a TypedArray
check—either extend isNativeIterable to return true for TypedArray instances or
add a dedicated helper like isTypedArray(target) using
ArrayBuffer.isView(target) && !(target instanceof DataView) (or explicit
instanceof checks for Uint8Array/Int8Array/etc.), and use that in the
isOperation return condition alongside the existing !isNativeIterable and
Symbol.iterator checks.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d6c9e03e-989a-49e5-9caa-36486035fb04
📒 Files selected for processing (3)
context-api/context-api.test.tscontext-api/mod.tscontext-api/package.json
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
context-api/mod.ts (1)
158-172:⚠️ Potential issue | 🟠 Major
isOperation()still misclassifies iterable constants.The guard only excludes strings, arrays, Maps, and Sets.
Uint8Array/otherArrayBufferviews—and any domain object with[Symbol.iterator]—still take the Operation path and getyield*ed instead of returned, so the new constant/sync handler support is still incorrect for those values. At minimum, hardenisNativeIterable()against typed arrays here; supporting arbitrary iterables as plain values will need a stronger runtime brand than[Symbol.iterator].💡 Minimum hardening
function isNativeIterable(target: unknown): boolean { return ( typeof target === "string" || Array.isArray(target) || target instanceof Map || - target instanceof Set + target instanceof Set || + ArrayBuffer.isView(target) ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@context-api/mod.ts` around lines 158 - 172, The type-guard misclassifies typed-array/ArrayBuffer view values as Operations; update isNativeIterable() (used by isOperation()) to treat ArrayBuffer views as native iterables by returning true when ArrayBuffer.isView(target) (and also handle DataView if needed), so typed arrays (e.g., Uint8Array) are excluded from the Operation path and will be returned as constants rather than being yield*-ed; do not try to treat arbitrary custom iterables as native here (that requires a stronger runtime brand).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@context-api/mod.ts`:
- Around line 62-63: The generic constraint on createApi is too broad (A extends
{}) and allows non-string keys while Object.keys(handler) only captures
string-keyed properties; update the generic to A extends Record<string, unknown>
so the type matches runtime behavior (affecting createApi, fields,
Object.keys(handler) and the returned Api<A>) and ensure operations/around and
other consumers only see string-keyed members.
- Around line 13-15: The public handler generic A currently allows PromiseLike
values and functions that return PromiseLike, which bypasses your
structured-concurrency guarantees; update the handler type A to exclude
PromiseLike members (e.g. constrain A with a utility like
NonPromiseLikeKeys/ExcludePromiseLike or require A extends Record<string,
unknown> & { [K in keyof A]: A[K] extends PromiseLike<any> ? never : A[K] }) and
also exclude functions whose ReturnType is PromiseLike. In addition, in the
handler-to-middleware lifting logic and at the isOperation() branches (the
checks referenced by isOperation and the code paths around lines ~37–44, ~91 and
~102), bridge any raw Promise/PromiseLike into an Operation (using your
Operation factory/constructor or an explicit wrapper) before returning so that
Promises are yielded as scope-owned Operations rather than returned directly.
Ensure Middleware and Operation type mappings (the mapped type using
Middleware<TArgs,TReturn>) use Awaited/Unwrap to work with non-Promise returns
after the constraint is applied.
---
Duplicate comments:
In `@context-api/mod.ts`:
- Around line 158-172: The type-guard misclassifies typed-array/ArrayBuffer view
values as Operations; update isNativeIterable() (used by isOperation()) to treat
ArrayBuffer views as native iterables by returning true when
ArrayBuffer.isView(target) (and also handle DataView if needed), so typed arrays
(e.g., Uint8Array) are excluded from the Operation path and will be returned as
constants rather than being yield*-ed; do not try to treat arbitrary custom
iterables as native here (that requires a stronger runtime brand).
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9957aa2a-ef87-4a43-913a-c0d8e873a26d
📒 Files selected for processing (3)
context-api/context-api.test.tscontext-api/mod.tscontext-api/package.json
2 participants
Motivation
The
@effectionx/context-apionly supported API members that areOperation<T>or(...args) => Operation<T>. In effection v4-1-alpha, API members can be any kind of value — sync functions, Operation-returning functions, plain Operations, and plain constants. This PR brings context-api to parity with v4-1-alpha's API system.*fn(): Operation<T>Operation<T>(value)() => T(sync function)Handlertype() => Operation<T>T(plain constant)HandlertypeOperation<T>Approach
Handlerconstraint —createApinow acceptsA extends {}instead ofA extends Record<string, Handler>, matching v4-1-alphaOperations<A>type mapping — checksOperation<unknown>first (before function check) to prevent Operations from being misclassified, then handles sync functions by lifting(...args) => Tto(...args) => Operation<T>, and constants by liftingTtoOperation<T>Around<A>type — middleware mirrors the handler's raw type, so sync function middleware is sync and Operation middleware is Operation-returningisOperation()runtime guard — with native iterable exclusion (strings, arrays, Maps, Sets) to prevent them from being mistaken for Operations when deciding whether toyield*a resultisOperation()to decide at runtime whether toyield*the result or return it directlyNote:
invoke(calling API members from aScopedirectly) is deferred to a follow-up.Summary by CodeRabbit
Breaking Changes
Bug Fixes
Tests