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

Add non-standardized timeout option to the fetch API #6504

Closed
wants to merge 2 commits into from
Closed

Add non-standardized timeout option to the fetch API #6504

wants to merge 2 commits into from

Conversation

lassebm
Copy link
Contributor

@lassebm lassebm commented Mar 17, 2016

This is required to specify a timeout value other than the XMLHttpRequest default which is currently 0 (zero), i.e. no timeout.

The standardization work is tracked here: whatwg/fetch#20, whatwg/fetch#179.

Test: Unit test and manual verification in iOS project.

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @sahrens, @brentvatne and @whokilledtheelectricmonk to be potential reviewers.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Mar 17, 2016
jest.runAllTicks()
expect(XMLHttpRequest.mock.instances.length).toBe(1)
expect(XMLHttpRequest.mock.instances[0].timeout).toBe(42)
})

Choose a reason for hiding this comment

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

semi: Missing semicolon.

@facebook-github-bot
Copy link
Contributor

@lassebm updated the pull request.

@lexs
Copy link
Contributor

lexs commented Apr 15, 2016

I may have glanced over the standard but it seems like deadline is actually what we have implemented in our XHR? timeout in whatwg/fetch#180 is when data hasn't arrived for x time.

@lexs lexs self-assigned this Apr 15, 2016
@lexs
Copy link
Contributor

lexs commented Apr 15, 2016

I'm inclined to wait with this until fetch actually has it standardized (which also means the polyfill will be updated, see JakeChampion/fetch#175). In the meantime I think the workaround on that issue is sufficent. Closing for now.

@lexs lexs closed this Apr 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants