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

Fetch timeout polyfill #1

Merged
merged 2 commits into from
Mar 17, 2017
Merged

Fetch timeout polyfill #1

merged 2 commits into from
Mar 17, 2017

Conversation

atticoos
Copy link
Contributor

@atticoos atticoos commented Mar 16, 2017

Adds support for fetch to respect a timeout value.

fetch(url, {...options, timeout: 30 * 1000})

See the README for a description of how this works and how it will be maintained.

TL;DR

Networking on the JS side is all done through a library called fetch (specification). fetch is an abstraction on top of XMLHttpRequest, which is a browser API for networking.

React Native implements the XMLHttpRequest API with a backing native networking module, RCTNetworking (interfaced by an ios and android JS file).

This supports all the bells and whistles for timeouts and aborting requests, the problem is that the the fetch specification doesn't describe a standard for aborting and timing out requests.

The polyfill allows the timeout to reach the underlying XMLHttpRequest instance.

@@ -0,0 +1,81 @@
'use strict';

// Polyfill from https://github.com/github/fetch/blob/v1.1.1/fetch.js#L8-L21
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's get some permanent links to all linked files.

While viewing the file in github, you can press y to transform the URL into a permanent form.

Copy link
Contributor Author

@atticoos atticoos Mar 17, 2017

Choose a reason for hiding this comment

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

This link is for a tag -- same idea

Copy link
Contributor

@christophermark christophermark left a comment

Choose a reason for hiding this comment

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

Code looks good. Let's just get those permanent links and we should be good here.

@christophermark christophermark merged commit 8f0f1c5 into master Mar 17, 2017
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.

2 participants