-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
ref(browser): Improve fetchTransport
error handling
#17661
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
Conversation
...options.fetchOptions, | ||
}; | ||
|
||
if (!nativeFetch) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this lead to pendingXX
not being cleaned up in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
*/ | ||
export function makeFetchTransport( | ||
options: BrowserTransportOptions, | ||
nativeFetch: typeof WINDOW.fetch | undefined = getNativeImplementation('fetch'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this type was not what we expected it to be, as nativeFetch
was interpreted to be typeof fetch
anyhow because the undefined just lead to it assuming the return type of getNativeImplementation('fetch')
, which was also typeof fetch
😅
size-limit report 📦
|
...options.fetchOptions, | ||
}; | ||
|
||
if (!nativeFetch) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
3996942
to
dd26af9
Compare
This changes our fetch transport a bit, making use of async functions and slightly adjusting error handling:
pendingBodySize
/pendingCount
and never decrease it.Extracted this out of #17641