✅ Add scope middleware test suite for context-api#201
Conversation
Comprehensive cross-scope middleware behavior coverage with 14 tests: inheritance, ordering, isolation, spawn semantics, arg/result transformation, and handler-shape coverage across scopes.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaces per-field pre-composed middleware with runtime scope-based middleware collection/composition and adds a comprehensive BDD test suite validating scope middleware registration, inheritance, min/max ordering, isolation, spawn semantics, and cumulative transformations. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Scope as Scope (current)
participant Collector as collectMiddleware
participant Composer as combine(...)
participant Middleware as MiddlewareStack
participant Handler
Caller->>Scope: request API field (useScope()/invoke)
Scope->>Collector: collectMiddleware(contextKey, field)
Collector->>Scope: traverse prototype chain, gather {max,min}
Collector->>Composer: return arrays
Composer->>Middleware: compose([...max, ...min])
Caller->>Middleware: invoke wrapper chain (enter)
Middleware->>Handler: call underlying handler (may spawn)
Handler-->>Middleware: return/result
Middleware->>Caller: unwind wrappers (exit)
note right of Handler: spawned tasks consult live Scope for middleware updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: |
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 175-191: The contextWindow function currently reads undocumented
internals (scope.contexts and scope.frame.context); replace this with a safe
approach: first attempt to use any public Effection API for context
introspection (call the public getter if available), otherwise add explicit
runtime version/feature guards in contextWindow by detecting known Effection
versions or feature flags (e.g., check process.env or a runtime
Effection.version property) and then branch to the existing internal-path logic
only when the detected version matches a supported internal shape, returning a
clear, descriptive Error that names contextWindow and the detected version/shape
when unsupported; also add an integration test covering both v3(via
frame.context) and v4(via contexts) code paths and add a short doc note and an
issue in the repo to request a public API if missing.
🪄 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: 9ca3fedb-7a76-48b9-bbb5-e5ceb8786a14
📒 Files selected for processing (2)
context-api/mod.tscontext-api/scope-middleware.test.ts
Version bump for scope middleware changes merged in #201.
Version bump for scope middleware changes merged in #201.
Motivation
Effection 4.1 will introduce a new way to provide APIs with middleware built in. We
@effectionx/context-apito serve as an upgrade path from Effection 3 to 4.1. The current implementation of@effectionx/context-apialready provides most of the compatibility except for one specific use case: once a child scope calledaround(), it captured a merged snapshot of middleware state. If the parent added middleware afterward, the child did not observe that update.Effection internals (v3
frame.context, v4scope.contexts) model contexts as prototype chains, where descendant scopes should continue to see ancestor updates unless they intentionally override the same key locally. We wantcreateApi().around()to match that behavior across both v3 and v4.Approach
context-api/scope-middleware.test.tsfor the missing scenario: child extends middleware first, then parent adds more middleware, and child must see both inherited and local layers.context-api/mod.tsto stop storing pre-composed per-field snapshots in context.around()now stores only per-scope middleware layers (max/min) on the current scope.scope.contextsscope.frame.contextmaxoutermost,minclosest to core) and scope isolation guarantees.Validation
node --env-file=.env --test context-api/scope-middleware.test.tsnode --env-file=.env --test context-api/context-api.test.tspnpm test:matrix(passes with effection3.0.0and4.0.2matrix entries)Summary by CodeRabbit
Tests
Refactor