feat(fetch): add fetchApi with middleware support#158
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a middleware-capable fetch core by exporting Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant FetchFn as fetch()
participant FetchApi as fetchApi.operations
participant Middleware as Middleware (around)
participant HTTP as HTTP Request
Client->>FetchFn: fetch(url, init)
FetchFn->>FetchApi: fetch(input, init, shouldExpect)
FetchApi->>Middleware: invoke wrapped operation
alt Middleware intercepts / short-circuits
Middleware->>Middleware: log/mock/transform
Middleware-->>FetchApi: return mockResponse
else Middleware passes through
Middleware->>HTTP: perform actual HTTP request
HTTP-->>Middleware: response
Middleware-->>FetchApi: return response
end
FetchApi-->>FetchFn: resolved response
FetchFn-->>Client: return result
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 (3 passed)
✨ Finishing Touches🧪 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
fetch/package.json (2)
1-34:⚠️ Potential issue | 🟡 MinorMissing
filesfield in package.json.Per coding guidelines, package.json must include a
filesfield containingdist,mod.ts, and source files. This field controls what gets published to npm.📦 Proposed fix to add files field
"engines": { "node": ">= 22" }, + "files": [ + "dist", + "mod.ts", + "fetch.ts" + ], "sideEffects": false }
3-3:⚠️ Potential issue | 🟠 MajorVersion bump required.
Source files have changed (fetch.ts, fetch.test.ts) with a breaking peerDependency change (narrowed from
"effection": "^3 || ^4"to"effection": "^4.1.0-alpha.3"). Per the version-bump policy, this requires at least a major version bump.
🤖 Fix all issues with AI agents
In `@fetch/package.json`:
- Around line 14-16: The peerDependencies change to "effection":
"^4.1.0-alpha.3" removes Effection v3 support and is a breaking change; update
package.json's "version" to a breaking bump (e.g., major or appropriate pre-1.0
bump) to reflect this, update any CHANGELOG/release notes to document the
removed v3 compatibility and the new requirement ("effection":
"^4.1.0-alpha.3"), and ensure any CI/release metadata that relies on the
package.json "version" is updated accordingly.
In `@fetch/README.md`:
- Around line 197-218: The example references an undefined helper
createMockResponse; update the README by either adding a minimal inline factory
implementation for createMockResponse that returns a FetchResponse-compatible
object (implementing properties like ok/status/statusText/headers/url and
generator methods json/text/arrayBuffer/blob/formData and expect), or explicitly
state that users must supply their own mock response factory and show a short
pseudo-signature; place this next to the example and reference fetchApi.around,
fetch, and the FetchResponse type so readers know how to integrate the helper
with the shown fetch("/api/users").json() call.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@fetch/fetch.test.ts`:
- Line 21: Tests are manually constructing FetchResponse-like stubs for fetch
and fetchApi; replace those manual stubs with the shared test helper
createMockResponse to keep the surface consistent and reduce boilerplate—locate
usages in fetch.test.ts where FetchResponse-like objects are built (near imports
of FetchResponse, HttpError, fetch, fetchApi) and swap the handcrafted objects
with calls to createMockResponse(...), ensuring the same status, headers, body,
and json/text behaviors are passed so assertions continue to work.
In `@fetch/fetch.ts`:
- Around line 130-216: The exported variable fetchApi is typed as
Api<FetchApiCore> but FetchApiCore is currently not exported, causing a
declaration emit error; export the interface FetchApiCore (e.g., change
"interface FetchApiCore" to "export interface FetchApiCore") so the public type
referenced by fetchApi is exported, then re-run the TypeScript build to ensure
declarations emit correctly.
In `@fetch/README.md`:
- Around line 199-214: The README example incorrectly constructs the
FetchResponse interface (new FetchResponse(...)) which is not instantiable in
TypeScript; replace that construction with the exported helper
createMockResponse to produce a mock response. In the fetchApi.around middleware
example (the generator method fetch), return createMockResponse(...) with the
same body, status and headers instead of new FetchResponse(...) so the snippet
compiles; ensure you import createMockResponse from "@effectionx/fetch" at the
top of the example.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@fetch/README.md`:
- Line 183: Examples use run() at lines with await run(function* () { ... })
while earlier example uses main(); update the two middleware examples to use the
same Effection entry point by replacing await run(...) with await main(...)
(keep the generator function contents unchanged) so that the examples
consistently call main() instead of run(); ensure any necessary main import or
helper used earlier is referenced the same way in those examples.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fetch/fetch.ts`:
- Around line 194-216: Bump the package minor version to 0.3.0 to reflect the
new public exports (fetchApi, createMockResponse, FetchApiCore); update the
version field in this package's package.json (and any release metadata or
changelog entries if present) so the published package version is 0.3.0 and
ensure the new symbols (fetchApi, createMockResponse, FetchApiCore) are included
in the package's public exports/entry points.
- Around line 347-353: Extend createMockResponse to accept an optional options
object so callers can customize status, headers, and raw body (allowing
non-JSON/error responses) before wrapping with createFetchResponse; update the
signature (createMockResponse(data: unknown, opts?: { status?: number; headers?:
Record<string,string>; rawBody?: string; }) ), use opts.status defaulting to
200, merge opts.headers with a default Content-Type: application/json when
rawBody is not provided, stringify data only when not using rawBody and
Content-Type is JSON, and construct the Response accordingly before passing it
to createFetchResponse so tests can mock non-200 and custom-header responses.
a2d6548 to
9dda7e6
Compare
Extract FetchApi using @effectionx/context-api for middleware decoration. Rename FetchApiCore to FetchHandler per review feedback. Keep createFetchResponse internal. Add middleware tests for logging, mocking, and scope isolation.
Summary
Convert
@effectionx/fetchto use Effection'screateApipattern, enabling middleware for logging, mocking, and instrumentation.Changes
fetchApiexport witharound()method for middlewareUsage
Backward Compatibility
The
fetch()function remains unchanged. UsefetchApi.around()to add middleware that intercepts all requests in the current scope and its children.