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 fetch response and error utils #303

Merged
merged 6 commits into from
Jul 8, 2019
Merged

Add fetch response and error utils #303

merged 6 commits into from
Jul 8, 2019

Conversation

BarryThePenguin
Copy link
Contributor

@BarryThePenguin BarryThePenguin commented May 21, 2019

Transitioning from ember-ajax to ember-fetch and noticed there weren't any error detection helpers

This PR adds similar helpers, mostly a copy/paste. Main motivation is to aid in transitioning between the two.

import {
  isNotFoundResponse,
  isForbiddenResponse
} from 'ember-fetch/errors';

I looked through npm briefly to see if something similar already existed, but couldn't find anything. If we want to move forward with this, I can add some documentation to the readme.

addon/errors.ts Outdated Show resolved Hide resolved
addon/errors.ts Outdated Show resolved Hide resolved
addon/errors.ts Outdated Show resolved Hide resolved
addon/errors.ts Outdated Show resolved Hide resolved
@xg-wang
Copy link
Member

xg-wang commented Jun 28, 2019

I think these are helpful to make people using fetch api easier, but there're some differences when migrating from Ajax to Fetch, I did a quick check on our polyfills (https://github.com/mo/abortcontroller-polyfill and https://github.com/github/fetch) my comments about response type are also true. Can you make the code changes and explicitly call out in Readme doc?

@BarryThePenguin
Copy link
Contributor Author

BarryThePenguin commented Jun 28, 2019

Thanks for the review. It was very helpful. I learned a lot reading the fetch and related specs

I've added a section to the readme too

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
addon/errors.ts Outdated Show resolved Hide resolved
tests/unit/utils/determine-body-promise-test.js Outdated Show resolved Hide resolved
tests/acceptance/error-test.js Outdated Show resolved Hide resolved
@xg-wang xg-wang changed the title add fetch errors Add fetch response and error utils Jul 8, 2019
@xg-wang xg-wang merged commit acf8620 into ember-cli:master Jul 8, 2019
@BarryThePenguin BarryThePenguin deleted the feature/fetch-errors branch July 8, 2019 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants