allow Request class to be provided in options (#1563) #1593
Conversation
|
| @@ -87,7 +89,7 @@ export default function createClient(clientOptions) { | |||
| request.params = params; // (re)attach params | |||
| const result = await m.onRequest(request, mergedOptions); | |||
| if (result) { | |||
| if (!(result instanceof Request)) { | |||
| if (!(result instanceof baseRequest)) { | |||
|
Yup this is looking great, and what I was thinking. This will need 2 things:
|
|
I had issues with the Request & the fetch on Vercel's environment. The fix we used was in two steps:
export const OpenAPI = createClient<paths>({
baseUrl: env.API_URL,
fetch(request) {
return fetch(request.url, request);
},
});
const authMiddleware: Middleware = {
onRequest(req) {
if (UNPROTECTED_ROUTES.some((pathname) => req.url.startsWith(pathname))) {
return undefined;
}
req.headers.set("Authorization", `Bearer ${accessToken}`);
return new Request(req.url, req);
},
};We lost a few hours debugging this and I'm still unsure what was the cause but it was related to the fetch + Request |
|
Still open to this landing if this PR gets updated resolving the conflicts and passing CI! (the last commit still wasn’t mergeable). |
|
Hey, I would really like that change. Is there any way I can help to get this merged? Seems like it's only about some linting error that unfortunately I cannot see in the logs. |
@jscheffner it seems that it is only about linting and some conflicts, feel free to open a new PR so we can merge this. |
Changes
What does this PR change? Link to any related issue(s).
Allows you to provide the Request class to use.
see: #1563
Caused issues for us using node 20's fetch, unidici's fetch, and node-fetch in different places.
Providing back as a base for someone else to continue, I may not have time soon.
How to Review
How can a reviewer review your changes? What should be kept in mind for this review?
Checklist
docs/updated (if necessary)pnpm run update:examplesrun (only applicable for openapi-typescript)