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

feat(fetch): Making fetch happen #4702

Merged
merged 2 commits into from
Apr 23, 2019
Merged

Conversation

benlesh
Copy link
Member

@benlesh benlesh commented Apr 10, 2019

  • Adds fromFetch implementation that uses native fetch
  • Adds tests and basic documentation
  • DOES NOT polyfill fetch

Overview

I think, long term, we should deprecate the ajax methods and try to move people towards fetch now that both fetch and AbortController are universally implemented (everywhere but IE) and polyfills are available.

Maintaining our own ajax implementation is challenging, and this is lot more lightweight and full featured.

- Adds `fromFetch` implementation that uses native `fetch`
- Adds tests and basic documentation
- DOES NOT polyfill fetch
@benlesh benlesh added the AGENDA ITEM Flagged for discussion at core team meetings label Apr 10, 2019
@coveralls
Copy link

coveralls commented Apr 10, 2019

Pull Request Test Coverage Report for Build 8363

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.669%

Totals Coverage Status
Change from base Build 8343: 0.0%
Covered Lines: 5252
Relevant Lines: 5433

💛 - Coveralls

@benlesh
Copy link
Member Author

benlesh commented Apr 10, 2019

From Core Team Meeting

  • Node concerns: Node users are probably just going to use whatever they're currently using
  • AbortController isn't something that can be easily polyfilled, but exists in all modern browers.
  • We should add warnings to the documentation that parts of the fetch API are experimental.
  • It is published under rxjs/fetch which separates it from the other functionality a bit. Also document that this is a browser-only API.
  • Perhaps test it with node-fetch just to see/make sure it works?
  • If fetch exists and AbortController does not, we need to add clarity about the fact that you can't really polyfill AbortController without also patching fetch to use it.
  • Add examples into the documentation... include streaming example? (idea: @trxcllnt)

👍

@benlesh benlesh removed the AGENDA ITEM Flagged for discussion at core team meetings label Apr 10, 2019
@OliverJAsh
Copy link
Contributor

🎉 I think this will close #4534

spec/observables/dom/fetch-spec.ts Outdated Show resolved Hide resolved
src/internal/observable/dom/fetch.ts Outdated Show resolved Hide resolved
@benlesh benlesh requested a review from cartant April 16, 2019 22:27
Copy link
Collaborator

@cartant cartant left a comment

Choose a reason for hiding this comment

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

LGTM

@benlesh benlesh merged commit 5a1ef86 into ReactiveX:master Apr 23, 2019
BioPhoton pushed a commit to BioPhoton/rxjs that referenced this pull request May 15, 2019
* feat(fetch): Making fetch happen

- Adds `fromFetch` implementation that uses native `fetch`
- Adds tests and basic documentation
- DOES NOT polyfill fetch

* fixup! feat(fetch): Making fetch happen
@lock lock bot locked as resolved and limited conversation to collaborators May 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants