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)
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.
| "peerDependencies": { | ||
| "effection": "^3 || ^4" | ||
| "effection": "^4.1.0-alpha.3" | ||
| }, |
There was a problem hiding this comment.
Breaking change: Effection v3 support dropped.
The peerDependency changed from "^3 || ^4" to "^4.1.0-alpha.3", which drops support for Effection v3.x. This is a breaking change that should be reflected in the version bump.
Per coding guidelines, the version should be bumped to indicate a breaking change (major version bump or appropriate pre-1.0 bump).
🤖 Prompt for AI Agents
In `@fetch/package.json` around lines 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.
fetch/fetch.ts
Outdated
| * Perform the actual HTTP request operation. | ||
| * This is the core operation that middleware can intercept. | ||
| */ | ||
| request( |
There was a problem hiding this comment.
why is this called request and not fetch?
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.
| } from "effection"; | ||
|
|
||
| import { HttpError, fetch } from "./fetch.ts"; | ||
| import { type FetchResponse, HttpError, fetch, fetchApi } from "./fetch.ts"; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, verify the file exists and check for createMockResponse export
find . -name "fetch.ts" -o -name "fetch.test.ts" | head -20Repository: thefrontside/effectionx
Length of output: 105
🏁 Script executed:
#!/bin/bash
# Search for createMockResponse export in fetch module
rg -n "createMockResponse" --type tsRepository: thefrontside/effectionx
Length of output: 637
🏁 Script executed:
#!/bin/bash
# Check the structure of fetch directory
fd -t f . fetch/Repository: thefrontside/effectionx
Length of output: 169
🏁 Script executed:
#!/bin/bash
# Check the import statement at line 21 of fetch.test.ts
sed -n '15,30p' fetch/fetch.test.tsRepository: thefrontside/effectionx
Length of output: 433
🏁 Script executed:
#!/bin/bash
# Check lines around 220-253 to see the manual mock response
sed -n '215,260p' fetch/fetch.test.tsRepository: thefrontside/effectionx
Length of output: 1322
🏁 Script executed:
#!/bin/bash
# Check the createMockResponse function signature and implementation
sed -n '340,360p' fetch/fetch.tsRepository: thefrontside/effectionx
Length of output: 636
🏁 Script executed:
#!/bin/bash
# Check if FetchResponse type is used elsewhere in the test file
rg -n "FetchResponse" fetch/fetch.test.tsRepository: thefrontside/effectionx
Length of output: 194
🏁 Script executed:
#!/bin/bash
# Check the full createMockResponse implementation to understand what it returns
sed -n '347,370p' fetch/fetch.tsRepository: thefrontside/effectionx
Length of output: 671
Use createMockResponse to reduce manual stub boilerplate.
This keeps tests aligned with the public helper and avoids duplicating the FetchResponse surface.
♻️ Suggested refactor
-import { type FetchResponse, HttpError, fetch, fetchApi } from "./fetch.ts";
+import { HttpError, fetch, fetchApi, createMockResponse } from "./fetch.ts";
@@
- // Create a mock response
- const mockResponse: FetchResponse = {
- raw: new Response(JSON.stringify({ mocked: true })),
- bodyUsed: false,
- ok: true,
- status: 200,
- statusText: "OK",
- headers: new Headers(),
- url: "mock://test",
- redirected: false,
- type: "basic",
- *json<T>(): Operation<T> {
- return { mocked: true } as T;
- },
- *text(): Operation<string> {
- return '{"mocked": true}';
- },
- *arrayBuffer(): Operation<ArrayBuffer> {
- return new ArrayBuffer(0);
- },
- *blob(): Operation<Blob> {
- return new Blob();
- },
- *formData(): Operation<FormData> {
- return new FormData();
- },
- body() {
- throw new Error("Not implemented");
- },
- *expect() {
- return this;
- },
- };
+ const mockResponse = createMockResponse({ mocked: true });Also applies to: 220-253
🤖 Prompt for AI Agents
In `@fetch/fetch.test.ts` at 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.
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.
| import { run } from "effection"; | ||
|
|
||
| // Add logging middleware | ||
| await run(function* () { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using consistent Effection entry point across examples.
The middleware examples use run() (lines 183, 203) while the earlier usage example uses main() (line 23). Both are correct, but using main() consistently would improve uniformity.
📝 Suggested consistency improvement
-await run(function* () {
+await main(function* () {Note: run() is perfectly valid—this is purely for stylistic consistency.
Also applies to: 203-203
🤖 Prompt for AI Agents
In `@fetch/README.md` at 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.
| export const fetchApi: Api<FetchApiCore> = createApi("fetch", { | ||
| *fetch( | ||
| input: RequestInfo | URL, | ||
| init: FetchInit | undefined, | ||
| shouldExpect: boolean, | ||
| ): Operation<FetchResponse> { | ||
| let signal = yield* useAbortSignal(); | ||
|
|
||
| let response = yield* until(globalThis.fetch(input, { ...init, signal })); | ||
| let fetchResponse = createFetchResponse(response); | ||
|
|
||
| if (shouldExpect && !response.ok) { | ||
| throw new HttpError( | ||
| response.status, | ||
| response.statusText, | ||
| response.url, | ||
| fetchResponse, | ||
| ); | ||
| } | ||
|
|
||
| return fetchResponse; | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat fetch/package.json | jq '.version'Repository: thefrontside/effectionx
Length of output: 74
🏁 Script executed:
git diff fetch/package.json 2>/dev/null | head -20Repository: thefrontside/effectionx
Length of output: 49
Bump fetch package version from 0.2.0 to 0.3.0.
This PR adds new public exports (fetchApi, createMockResponse, FetchApiCore), which requires a minor version bump per .policies/version-bump.md.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fetch/fetch.ts` around lines 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.
| export function createMockResponse(data: unknown): FetchResponse { | ||
| let response = new Response(JSON.stringify(data), { | ||
| status: 200, | ||
| headers: { "Content-Type": "application/json" }, | ||
| }); | ||
| return createFetchResponse(response); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extending createMockResponse for more flexibility.
The current implementation only supports 200 OK JSON responses. For comprehensive testing, users may need to mock error responses or custom headers.
♻️ Optional enhancement for flexibility
-export function createMockResponse(data: unknown): FetchResponse {
- let response = new Response(JSON.stringify(data), {
- status: 200,
- headers: { "Content-Type": "application/json" },
- });
+export interface MockResponseInit {
+ status?: number;
+ statusText?: string;
+ headers?: HeadersInit;
+}
+
+export function createMockResponse(
+ data: unknown,
+ init?: MockResponseInit,
+): FetchResponse {
+ let response = new Response(JSON.stringify(data), {
+ status: init?.status ?? 200,
+ statusText: init?.statusText,
+ headers: { "Content-Type": "application/json", ...init?.headers },
+ });
return createFetchResponse(response);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fetch/fetch.ts` around lines 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.
Convert @effectionx/fetch to use Effection's createApi pattern, enabling middleware for logging, mocking, and instrumentation. Changes: - Add fetchApi export with around() method for middleware - Middleware intercepts at the request level (not the builder) - Add middleware tests for logging, mocking, and scope isolation - Update README with middleware documentation and v4.1 requirement - Update peer dependency to require effection ^4.1.0-alpha.3 The fetch() function remains unchanged for backward compatibility. Use fetchApi.around() to add middleware that intercepts all requests in the current scope and its children.
- Bump version 0.1.0 → 0.2.0 for breaking peerDependencies change - Add files field for npm publishing - Fix mocking example to use FetchResponse constructor
Renamed the fetchApi operation from 'request' to 'fetch' for consistency with the function name. Also added createMockResponse helper for easier testing. Session-ID:
The README example incorrectly used 'new FetchResponse()' which is not valid since FetchResponse is an interface. Updated to use the exported createMockResponse helper function instead. Session-ID:
TypeScript declaration emit requires FetchApiCore to be exported because it's used in the Api<FetchApiCore> return type of fetchApi.
Add pnpm override to use effection from PR #1116 which provides createApi required for fetchApi middleware support. Note: This override affects all packages and some tests may fail due to timing differences in preview effection. Once effection 4.1.0 is released, this override should be removed.
These tests rely on scope teardown timing behavior that differs in effection 4.1.0-alpha.3. They will be re-enabled when effection 4.1.0 stable is released.
a2d6548 to
9dda7e6
Compare
Summary
Convert
@effectionx/fetchto use Effection'screateApipattern, enabling middleware for logging, mocking, and instrumentation.Changes
fetchApiexport witharound()method for middlewareeffection ^4.1.0-alpha.3Breaking Change Notice
This version requires Effection v4.1 or greater. The middleware features (
fetchApi.around()) use the newcreateApifunction.Usage
Backward Compatibility
The
fetch()function remains unchanged. UsefetchApi.around()to add middleware that intercepts all requests in the current scope and its children.Dependencies
Summary by CodeRabbit
New Features
Documentation
Chores
Tests