-
-
Notifications
You must be signed in to change notification settings - Fork 532
proposal: openapi-fetch new send function #1558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
sry for choosed wrong label, help me pls! |
You’ve described how the internals should change, but I’d love to see this from the perspective of the end-user. What code do they write? What does the API look like? What could they not do before but now they can? (is there any way to accomplish it with the current API, or was it impossible)? I’m more interested in how writing middleware could be easier with this change, rather than describing what a PR would do codewise. |
It mainly solves the issue here #1122 (comment) , user can use it to resend some request in middleware like this. client.use({
async onResponse(response, options) {
if(response.status === 401) {
// do some other request...
// then resend it.
return await client.send(new Request(options.requestUrl,options.requestOptions), options);
}
return response;
},
}); |
Currently, a simple way to solve this problem is to use a custom fetch function. |
Ah I see. You’re right, I haven’t explicitly replied to that, but that is something I would definitely not want to add to the library—retries. I think retry mechanisms should always be handled at a layer above openapi-fetch, in its current design. Reason is, it inevitably bleeds into the caching and orchestration layer (e.g. React Query) which have retry mechanisms baked in. I do not want this library to take on caching, or orchestration, or retries, ever. I want this to remain a low-level typed fetch wrapper that can be used with anything on top of it, and therefore work in any stack and setup. Perhaps in the future I’ll reverse the decision, but I feel this won’t happen in 0.x, and I haven’t started planning what 1.x would look like.
That’s true. Though it’s not always a design flaw to be able to do things in multiple ways! Custom fetch can own everything, but requires you to manage 100% of the code. Middleware is more modular and DRY, and also lets you use multiple third-party packages that all plays nicely with your code. There’s a reason why most libraries use this pattern. |
I agree that this library shouldn't have first class support for retries or caching! However, providing "around" style middleware e.g. It's great that you have the custom fetch option, and maybe that's what people should reach for if they need that flexibility. However, it doesn't seem like much of a lift compared to the existing middleware implemenation. |
On a related note, the current middleware doesn't provide the option to catch fetch errors it seems? onResponse is not called when the fetch throws, which makes some sense, but makes a common use of middleware difficult and/or puts you back to the "just use a custom fetch function" option. |
This issue is stale because it has been open for 90 days with no activity. If there is no activity in the next 7 days, the issue will be closed. |
This issue was closed because it has been inactive for 7 days since being marked as stale. Please open a new issue if you believe you are encountering a related problem. |
background
In #1122 we discussed that resend request in middleware, it's hard to do that in current middleware design. But things like refresh token in real production, this is a very useful feature. So I want to openapi-fetch can do that.
investigation
for other request lib like axios, it alose provide a way to do this, Their methods are similar. Basically, when an error occurs or during the response phase, the callback is called with the requested config as a parameter, and then can call the send again.
for axios, there is a function
request
, and the requestConfigconfig
.plan
This is a mature experience that I think we can consider learning from.
we can do that for this step.
coreSend
function just to send request with middware, it accept the Stand fetch Request param and return the Stand fetch Response. it's the warpper offetch
.coreFetch
to invokecoreSend
coreSend
to user withsend
, this function like axios's request.new Request(input, options)
url and options, this is exist coreFetch alreay, we put this 2 param to mergeOption object. 'after that, use can remake the Request object by url and requestOption from
mergedOptions
, and resend it bysend
. This is no BREAK CHANGE now.the only BREAK CHANGE is optional, I moved schemaPath and params from MiddlewareRequest to mergedOptions, just because I don't want to pollute the request object, we should put it to own objects like mergedOptions. but for users, the BREAK CHANGE is more important, so this break change can be optional.
for reference only
#1556
The text was updated successfully, but these errors were encountered: