Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,18 @@ export const createClient = (config: Config = {}): Client => {
opts.headers.delete('Content-Type');
}

return opts;
};

const finalizeRequest = (opts: any) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

opts: any weakens the type safety that the previous inline construction had — opts is the inferred return of requestOptions and the rest of the file (e.g. line 92's cast opts as Config & RequestOptions) implicitly relies on that. Consider (opts: ReturnType<typeof requestOptions>) so the cast is the only erasure.

const url = buildUrl(opts as Config & RequestOptions);

const req = new HttpRequest<unknown>(opts.method ?? 'GET', url, getValidRequestBody(opts), {
redirect: 'follow',
...opts,
});

return { opts, req, url };
return { req, url };
};

const beforeRequest = async <
Expand All @@ -103,10 +107,10 @@ export const createClient = (config: Config = {}): Client => {
>(
options: RequestOptions<TData, TResponseStyle, ThrowOnError, Url>,
) => {
const { opts, req, url } = requestOptions(options);
const opts = requestOptions(options);

if (opts.security) {
await setAuthParams({
opts.headers = await setAuthParams({
...opts,
security: opts.security,
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Query-auth tokens still get dropped here. setAuthParams receives a shallow copy via { ...opts }; when opts.query is undefined, setAuthParams (utils.ts:200-203) does options.query = {} on the copy and writes the token into that throwaway object — the assignment never propagates back to outer opts, so finalizeRequest(opts)buildUrl(opts) runs without the query-auth params. (When opts.query is already an object, the shared reference saves you.) Either return query alongside headers from setAuthParams, pre-initialize opts.query = opts.query ?? {} before this call, or stop shallow-copying and let setAuthParams mutate opts directly.

Expand All @@ -116,6 +120,8 @@ export const createClient = (config: Config = {}): Client => {
await opts.requestValidator(opts);
}

const { req, url } = finalizeRequest(opts);

return { opts, req, url };
};

Expand Down Expand Up @@ -237,7 +243,7 @@ export const createClient = (config: Config = {}): Client => {
throw new Error('Request validation is not supported in requestOptions');
}

return requestOptions(options).req;
return finalizeRequest(requestOptions(options)).req;
},
setConfig,
sse: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,9 @@ export const setAuthParams = async (
break;
}

return;
return options.headers;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pre-existing, but worth confirming intent: return options.headers inside the for loop means only the first security entry that produces a token is applied — subsequent entries (e.g. a second scheme, or a header + cookie combination) are silently dropped. Peer clients (client-fetch/utils.ts:126-154 and friends) iterate every entry. If this PR is the right place to align, drop the inner return and keep only the post-loop one.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yep, you're right. I don't think that should be there

}
return options.headers;
};

export const buildUrl: Client['buildUrl'] = (options) => {
Expand Down
Loading