Skip to content
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

Request added to response and response error #36

Merged

Conversation

neorel
Copy link
Contributor

@neorel neorel commented May 20, 2019

Add the request in the response.

It help when you intercept an error and want to resend the request.

(I used to work like this with axios, and this feature is missing with fetch)

PS: Hope the commit history is now clean enough ;)

// Register fetch call
promise = promise.then(args => fetch(...args));
promise = promise.then(args => {

Choose a reason for hiding this comment

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

Is this safe in race conditions? i.e. what happens if a second fetch is started before the first one returns

Copy link
Owner

Choose a reason for hiding this comment

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

Thats correct we need to nest the a second promise that extends the response object. Then you can use const request instead of let which gives additional safety in this regard.

@vitorcamachoo
Copy link

Can this be accepted ? I need this on my project 👍

@mlegenhausen
Copy link
Owner

I can't. The current implementation is not save for concurrent requests.

@neorel
Copy link
Contributor Author

neorel commented Jun 1, 2021

I've made the modification, hope that is the expected one

@neorel neorel force-pushed the request_in_response_for_resend branch 2 times, most recently from 3e93dfb to d3f546f Compare June 1, 2021 08:51
@neorel neorel force-pushed the request_in_response_for_resend branch from d3f546f to 6a069f0 Compare June 1, 2021 08:53
Copy link
Owner

@mlegenhausen mlegenhausen left a comment

Choose a reason for hiding this comment

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

That looks good. Could you also extend the type definitions in src/index.d.ts?

@neorel
Copy link
Contributor Author

neorel commented Jun 1, 2021

Is that the right way, I don't usually use typescript.

@mlegenhausen mlegenhausen changed the title Request in response Request added to response and response error Jun 2, 2021
@mlegenhausen mlegenhausen merged commit 85bd88f into mlegenhausen:develop Jun 2, 2021
@mlegenhausen
Copy link
Owner

Released in 2.4.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants