Skip to content

fetch support - #230

Merged
stefanpenner merged 7 commits into
pretenderjs:masterfrom
xg-wang:feat/fetch
Jul 16, 2018
Merged

stefanpenner merged 7 commits into
pretenderjs:masterfrom
xg-wang:feat/fetch

Conversation

@xg-wang

@xg-wang xg-wang commented May 23, 2018 •

Copy link
Copy Markdown
Member

tl;dr

  • This commit adds support for fetch, fix Using Pretender with fetch. #60.
  • pretender swap native fetch related API if exists
  • pretender.shutdown() restore native fetch related API
  • doesn't work with AbortController

Changes

Include a fetch ponyfill and swap the native fetch during pretender
creation, then restore them when shutdown. Since fetch polyfill uses
xhr behind the scene, pretender should "just work".

Caveats

  1. The supplement set of the polyfill implementation with the spec includes (not complete):
  1. Abort Update: Fixed by Enable Abortable fetch #234
  • xhr.abort() first set state to done, finally response to a
    network error;
  • fetch will reject promise with a new "AbortError" DOMException.

As implemented in fake_xml_http_request, the request is resolved once its
state is changed to DONE.
So the scenario happens in pretender is:
1). state changes to DONE, trigger resolve request
2). abort, trigger reject
3). xhr.onerror, trigger reject
The first resolve wins, error thus not rejected but an empty request is resolved.

  1. Though polyfilled by xhr, fetch returns a Promise and is asynchronous by
    nature.

  2. No streaming Streaming support JakeChampion/fetch#198

tl;dr
- This commit adds support for fetch, fix pretenderjs#60.
- pretender swap native fetch related API if exists
- pretender.shutdown() restore native fetch related API
- doesn't work with AbortController

Changes
------
Include a fetch ponyfill and swap the native fetch during pretender
creation, then restore them when `shutdown`. Since fetch polyfill uses
xhr behind the scene, pretender should "just work".

Caveats
------
1. The supplement set of yetch impl and spec includes (not complete):
- Inability to [set the redirect mode](JakeChampion/fetch#137)
- Inability to [change the cache directive](JakeChampion/fetch#438 (comment))
- Inability to [disable same-origin cookies](JakeChampion/fetch#56 (comment))

2. Abort
- `xhr.abort()` first set state to done, finally response to a
[network error](https://xhr.spec.whatwg.org/#the-abort()-method);
- [fetch](https://dom.spec.whatwg.org/#aborting-ongoing-activities) will
reject promise with a new "AbortError" DOMException.

As implemented in `fake_xml_http_request`, the request is resolved once its
state is changed to `DONE`.
So the scenario happens in pretender is:
  1). state changes to `DONE`, trigger resolve request
  2). abort, trigger reject
  3). xhr.onerror, trigger reject
The first resolve wins, error thus not rejected but an empty request is resolved.

3. Though polyfilled by xhr, fetch returns a Promise and is asynchronous by
nature.
@xg-wang

xg-wang commented May 23, 2018 •

Copy link
Copy Markdown
Member Author

Yetch doesn't provide a ponyfill now, include the file temporarily.
Netflix/yetch#5

Thomas Wang added 2 commits May 24, 2018 13:31
Github/fetch has merged PR for UMD distribution:
JakeChampion/fetch#616

Also abort support has been added:
JakeChampion/fetch#592

We can use Github fetch instead of yetch now.
But AbortableFetch is still not supported, see comment
in test.
remove yetch, use FakeFetch to reference WHATWGFetch
@xg-wang

xg-wang commented May 24, 2018 •

Copy link
Copy Markdown
Member Author

I released a fork version to have umd available, @xg-wang/whatwg-fetch need to be removed once official release is out.

@billymoon

billymoon commented Jul 12, 2018 •

Copy link
Copy Markdown

Any updates on this PR? I am super keen to consume fetch-pretender or ideally, pretender from a cdn like http://cdn.jsdelivr.net/npm/pretender.

@xg-wang

xg-wang commented Jul 14, 2018 •

Copy link
Copy Markdown
Member Author

CI error:
bower EINVRES Request to https://bower.herokuapp.com/packages/FakeXMLHttpRequest failed with 502
likely to be fixed by restarting
Fixed by upgrading bower to 1.8.2

Thomas Wang added 2 commits July 14, 2018 21:33
@xg-wang

xg-wang commented Jul 16, 2018

Copy link
Copy Markdown
Member Author

This is ready now. Need review @stefanpenner @rwjblue

@stefanpenner
stefanpenner merged commit 3646491 into pretenderjs:master Jul 16, 2018
Comment thread test/fetch_test.js
false
);

fetch('/some/path').then(function() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is async, so this needs to block

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if this is qunit lets use the async promise assert helpers

Comment thread test/fetch_test.js
setTimeout(function() {
controller.abort();
}, 10);
fetch('/downloads', { signal: signal })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if this is qunit lets use the async promise assert helpers

@stefanpenner

Copy link
Copy Markdown
Contributor

Oops, I merged this prematurely. @xg-wang can you address the above two comments today? If not, i'll have to revert until addressed.

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.

Using Pretender with fetch.

3 participants