Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type {
FetchContext,
FetchHook,
FetchHooks,
FetchOptions,
FetchRequest,
ResolvedFetchOptions,
Expand Down Expand Up @@ -109,15 +110,51 @@ export function resolveFetchOptions<
};
}

// Merge hooks: concatenate defaults and input hooks (defaults run first)
const mergedHooks = mergeHooks(
defaults as FetchOptions | undefined,
input as FetchOptions | undefined
);

return {
...defaults,
...input,
...mergedHooks,
query,
params: query,
headers,
};
}

const hookNames: Array<keyof FetchHooks> = [
"onRequest",
"onRequestError",
"onResponse",
"onResponseError",
];

function toArray<T>(value: T | T[] | undefined): T[] {
if (!value) {
return [];
}
return Array.isArray(value) ? value : [value];
}

function mergeHooks(
defaults: FetchOptions | undefined,
input: FetchOptions | undefined
): Partial<FetchHooks> {
const merged: Partial<FetchHooks> = {};
for (const name of hookNames) {
const defaultHooks = toArray((defaults as any)?.[name]);
const inputHooks = toArray((input as any)?.[name]);
if (defaultHooks.length > 0 || inputHooks.length > 0) {
(merged as any)[name] = [...defaultHooks, ...inputHooks];
}
}
return merged;
}

function mergeHeaders(
input: HeadersInit | undefined,
defaults: HeadersInit | undefined,
Expand Down
83 changes: 83 additions & 0 deletions test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,89 @@ describe("ofetch", () => {
expect(onResponseError).toHaveBeenCalledTimes(2);
});

describe("hook merging", () => {
it("merges default and per-request hooks (defaults run first)", async () => {
const order: string[] = [];
const customFetch = $fetch.create({
onRequest() {
order.push("default");
},
});
await customFetch(getURL("ok"), {
onRequest() {
order.push("per-request");
},
});
expect(order).toEqual(["default", "per-request"]);
});

it("merges array hooks from defaults and per-request", async () => {
const order: string[] = [];
const customFetch = $fetch.create({
onRequest: [
() => {
order.push("default-1");
},
() => {
order.push("default-2");
},
],
});
await customFetch(getURL("ok"), {
onRequest: [
() => {
order.push("per-request-1");
},
() => {
order.push("per-request-2");
},
],
});
expect(order).toEqual([
"default-1",
"default-2",
"per-request-1",
"per-request-2",
]);
});

it("works when only defaults have hooks", async () => {
const onRequest = vi.fn();
const customFetch = $fetch.create({ onRequest });
await customFetch(getURL("ok"));
expect(onRequest).toHaveBeenCalledOnce();
});

it("works when only per-request has hooks", async () => {
const onRequest = vi.fn();
const customFetch = $fetch.create({});
await customFetch(getURL("ok"), { onRequest });
expect(onRequest).toHaveBeenCalledOnce();
});

it("merges all hook types", async () => {
const defaultOnResponse = vi.fn();
const defaultOnResponseError = vi.fn();
const perRequestOnResponse = vi.fn();
const perRequestOnResponseError = vi.fn();

const customFetch = $fetch.create({
onResponse: defaultOnResponse,
onResponseError: defaultOnResponseError,
});

await customFetch(getURL("403"), {
onResponse: perRequestOnResponse,
onResponseError: perRequestOnResponseError,
}).catch(() => {});

expect(defaultOnResponse).toHaveBeenCalledOnce();
expect(perRequestOnResponse).toHaveBeenCalledOnce();
expect(defaultOnResponseError).toHaveBeenCalledOnce();
expect(perRequestOnResponseError).toHaveBeenCalledOnce();
});
Comment on lines +568 to +588
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add explicit merge coverage for onRequestError.

Line 568 says “all hook types,” but this block currently exercises only response-side error hooks. Please add a defaults + per-request onRequestError merge assertion.

✅ Suggested test addition
   it("merges all hook types", async () => {
+    const defaultOnRequestError = vi.fn();
+    const perRequestOnRequestError = vi.fn();
     const defaultOnResponse = vi.fn();
     const defaultOnResponseError = vi.fn();
     const perRequestOnResponse = vi.fn();
     const perRequestOnResponseError = vi.fn();

     const customFetch = $fetch.create({
+      onRequestError: defaultOnRequestError,
       onResponse: defaultOnResponse,
       onResponseError: defaultOnResponseError,
     });

+    await customFetch("/" /* non absolute is not acceptable */, {
+      onRequestError: perRequestOnRequestError,
+    }).catch(() => {});
+
     await customFetch(getURL("403"), {
       onResponse: perRequestOnResponse,
       onResponseError: perRequestOnResponseError,
     }).catch(() => {});

+    expect(defaultOnRequestError).toHaveBeenCalledOnce();
+    expect(perRequestOnRequestError).toHaveBeenCalledOnce();
     expect(defaultOnResponse).toHaveBeenCalledOnce();
     expect(perRequestOnResponse).toHaveBeenCalledOnce();
     expect(defaultOnResponseError).toHaveBeenCalledOnce();
     expect(perRequestOnResponseError).toHaveBeenCalledOnce();
   });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it("merges all hook types", async () => {
const defaultOnResponse = vi.fn();
const defaultOnResponseError = vi.fn();
const perRequestOnResponse = vi.fn();
const perRequestOnResponseError = vi.fn();
const customFetch = $fetch.create({
onResponse: defaultOnResponse,
onResponseError: defaultOnResponseError,
});
await customFetch(getURL("403"), {
onResponse: perRequestOnResponse,
onResponseError: perRequestOnResponseError,
}).catch(() => {});
expect(defaultOnResponse).toHaveBeenCalledOnce();
expect(perRequestOnResponse).toHaveBeenCalledOnce();
expect(defaultOnResponseError).toHaveBeenCalledOnce();
expect(perRequestOnResponseError).toHaveBeenCalledOnce();
});
it("merges all hook types", async () => {
const defaultOnRequestError = vi.fn();
const perRequestOnRequestError = vi.fn();
const defaultOnResponse = vi.fn();
const defaultOnResponseError = vi.fn();
const perRequestOnResponse = vi.fn();
const perRequestOnResponseError = vi.fn();
const customFetch = $fetch.create({
onRequestError: defaultOnRequestError,
onResponse: defaultOnResponse,
onResponseError: defaultOnResponseError,
});
await customFetch("/" /* non absolute is not acceptable */, {
onRequestError: perRequestOnRequestError,
}).catch(() => {});
await customFetch(getURL("403"), {
onResponse: perRequestOnResponse,
onResponseError: perRequestOnResponseError,
}).catch(() => {});
expect(defaultOnRequestError).toHaveBeenCalledOnce();
expect(perRequestOnRequestError).toHaveBeenCalledOnce();
expect(defaultOnResponse).toHaveBeenCalledOnce();
expect(perRequestOnResponse).toHaveBeenCalledOnce();
expect(defaultOnResponseError).toHaveBeenCalledOnce();
expect(perRequestOnResponseError).toHaveBeenCalledOnce();
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/index.test.ts` around lines 568 - 588, The test claims to cover "all
hook types" but only asserts response-side hooks; add explicit coverage for
onRequestError by creating defaultOnRequestError and perRequestOnRequestError
vi.fn() handlers, pass defaultOnRequestError into $fetch.create(...) and
perRequestOnRequestError into the per-request options for customFetch(...),
invoke customFetch in a way that triggers a request-side error (e.g., invalid
URL/network failure) and add
expect(defaultOnRequestError).toHaveBeenCalledOnce() and
expect(perRequestOnRequestError).toHaveBeenCalledOnce() alongside the existing
assertions so onRequestError merging is verified.

});

it("default fetch options", async () => {
await $fetch("https://jsonplaceholder.typicode.com/todos/1", {});
expect(fetch).toHaveBeenCalledOnce();
Expand Down