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

Make sure useFetch rejects with an Error type. #114

Merged
merged 3 commits into from
Sep 19, 2019

Conversation

artdent
Copy link
Contributor

@artdent artdent commented Sep 17, 2019

Description

Failed http responses in useFetch now reject with a FetchError,
with the underlying Response object available as error.response.

Previously, a non-ok http response would reject with the response
object directly. It's better for rejections to be of type Error
so that the full stack trace information is available; plus, the
TypeScript type definition assumes that the error object is always
instanceof Error.

Fixes #113.

Breaking changes

The error property of the state object returned by useFetch
is no longer a Response object. If you need the response object,
replace references to state.error with state.error.response.

Checklist

  • Added / updated the unit tests
  • Added / updated the documentation
  • Updated the TypeScript type definitions

Previously, a non-ok http response would reject with the response
object. It's better for rejections to be of type Error so that the
full stack trace information is available; plus, the TypeScript type
definition assumes that the error object is always instanceof Error.

Instead, failed responses reject with a FetchError, with the
underlying Response object available as error.response.

This is a backward-incompatible change: users who expected `error` to
be of type Response now have to refer to `error.response` instead.
@codecov
Copy link

codecov bot commented Sep 17, 2019

Codecov Report

Merging #114 into next will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next     #114      +/-   ##
==========================================
+ Coverage   98.93%   98.94%   +<.01%     
==========================================
  Files           8        8              
  Lines         375      378       +3     
  Branches      140      140              
==========================================
+ Hits          371      374       +3     
  Misses          4        4
Impacted Files Coverage Δ
packages/react-async/src/useAsync.js 99.08% <100%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9ca8ce...dba0bd7. Read the comment docs.

Copy link
Member

@ghengeveld ghengeveld left a comment

Choose a reason for hiding this comment

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

Awesome, well done! Nice to see you added a unit test as well. I have only a single remark and then it's good to go.

packages/react-async/src/useAsync.js Outdated Show resolved Hide resolved
This is necessary for TypeScript code to be permitted to use
FetchError as a value at runtime, e.g. to perform an `instanceof
FetchError` check.
@artdent
Copy link
Contributor Author

artdent commented Sep 19, 2019

Thanks for the review! I added one more small commit to fix an issue I ran into trying to actually use FetchError in client code.

The CI failure looks spurious, but I haven't dug in...

@ghengeveld
Copy link
Member

Looking good. Thanks!

I'll look into what's causing CodeSandbox CI to fail. It's something I added very recently and it's still in closed beta. We are early adopters 😅

@ghengeveld ghengeveld merged commit 428256a into async-library:next Sep 19, 2019
@ghengeveld ghengeveld mentioned this pull request Sep 30, 2019
ghengeveld added a commit that referenced this pull request Oct 7, 2019
* Update upgrade instructions.

* Update dependency eslint to v6.2.2 (#89)

* Change root package name to avoid ambiguity.

* Update dependency babel-eslint to v10.0.3 (#90)

* Update contribution guide.

* Text wrapping.

* Clarify behavior when 'initialValue' is set.

* Update dependency eslint-plugin-jest to v22.16.0 (#94)

* Auto detect React version for ESLint.

* Add Discord badge.

* Add brand images.

* Update dependency @storybook/react to v5.2.0-rc.2 (#91)

* Update dependency eslint to v6.3.0 (#95)

* Setup Chromatic.

* Update babel monorepo to v7.6.0 (#106)

* Update dependency eslint-plugin-jest to v22.17.0 (#101)

* Update dependency @testing-library/react to v9.1.4 (#99)

* Pin dependency storybook-chromatic to 2.2.2 (#100)

* Add displayName to the createInstance type signature. (#102)

* Transferred ownership of the repo to async-library.

* Fix merge error.

* remove non implemented types (#107)

* Add a unit test for the createInstance displayName arg. (#108)

* Update dependency eslint-config-prettier to v6.2.0 (#98)

* Update dependency now to v16.2.0 (#96)

* updated contribute.md  to run examples (#110)

* updated contribute to run examples

* updated examples md

* Replace occurences of Async.Loading with Async.Pending and isLoading with isPending, since this is the preferred terminology.

* Update bootstrap script to include building packages.

* Replace synthetic default imports with star import in ts definit… (#112)

* Update Chromatic app code.

* Add delay for Chromatic to avoid flake.

* Add Chromatic badge.

* Use the right color for Chromatic.

* Setup CodeSandbox CI.

* Fix a merge issue.

* Update dependency @storybook/react to v5.2.0-rc.6 (#97)

* docs: add Khartir as a contributor (#117)

* docs: update README.md

* docs: create .all-contributorsrc

* Update README.md

* docs: add phryneas as a contributor (#118)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add FredKSchott as a contributor (#120)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add Avi98 as a contributor (#119)

* docs: update README.md

* docs: update .all-contributorsrc

* Update README.md

* Update README.md

* docs: add byCedric as a contributor (#121)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add tomshane as a contributor (#122)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add ghengeveld as a contributor (#123)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add philip-peterson as a contributor (#124)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add sibelius as a contributor (#125)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add jimthedev as a contributor (#126)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add msokk as a contributor (#127)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add brabeji as a contributor (#128)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add unorsk as a contributor (#129)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add matthisk as a contributor (#130)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add dhurlburtusa as a contributor (#131)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add dhurlburtusa as a contributor (#132)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add noelyoo as a contributor (#133)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add aratcliffe as a contributor (#134)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add kentcdodds as a contributor (#135)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add noelyoo as a contributor (#136)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add walter-ind as a contributor (#137)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add phryneas as a contributor (#138)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add artdent as a contributor (#139)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add Avi98 as a contributor (#140)

* docs: update README.md

* docs: update .all-contributorsrc

* docs: add rokoroku as a contributor (#141)

* docs: update README.md

* docs: update .all-contributorsrc

* Update README.md

* Drop the avatar size and add the badge back.

* Add the All Contributors badge.

* No need to brag.

* docs: add elsangedy as a contributor (#142)

* docs: update README.md

* docs: update .all-contributorsrc

* Make sure useFetch rejects with an Error type. (#114)

* Make sure useFetch rejects with an Error type.

Previously, a non-ok http response would reject with the response
object. It's better for rejections to be of type Error so that the
full stack trace information is available; plus, the TypeScript type
definition assumes that the error object is always instanceof Error.

Instead, failed responses reject with a FetchError, with the
underlying Response object available as error.response.

This is a backward-incompatible change: users who expected `error` to
be of type Response now have to refer to `error.response` instead.

* FetchError: add status code to the error message.

* Define FetchError as a class, not just an interface.

This is necessary for TypeScript code to be permitted to use
FetchError as a value at runtime, e.g. to perform an `instanceof
FetchError` check.

* Bump all dependencies. (#147)

* Make sure the promise render prop is always defined (#148)

* Make sure the promise render prop is always a Promise.

* Add a warning about providing a rejection handler.

* Fix test for promise prop.

* Use catch instead of then(..., onReject).

* Allow overriding the 'resource' argument of 'fetch' when invokin… (#150)

* Add upgrade note for v9.

* Lock down all version ranges.

* Fix eslint config.

* Attempt at fixing CircleCI memory issue.

* Bump deps.

* Update lockfile.

* Add experimental Suspense support (#153)

* Add experimental Suspense support.

* Add PropType and type definition.

* Skip Suspense test when running against 16.3.

* Lock down all version ranges.

* Fix eslint config.

* Disable rules of hooks for examples.

* Attempt at fixing CircleCI memory issue.

* Update lockfile.

* Bump deps.

* Revert "Disable rules of hooks for examples."

This reverts commit d3d931a.

* Disable propType eslint rule for stories.

* Update dependency now to v16.3.0 (#156)

* Add CodeFund sponsorship message to README (#144)

[CodeFund](https://codefund.io) provides ethical sponsorships to open source maintainers. This PR will place the "Sponsored by" image at the top of the README. The sponsoring companies are not paying per click nor impression. They are paying the maintainer(s) on a per-month basis to be the primary sponsor of this project.

* Setup gitbook.

* Fix gitbook config.

* Use gitbook summary.

* GitBook: [next] 7 pages modified

* Clean up usage examples.

* Add shortcut links to API docs.

* Move gitbook to docs.

* Clean up the readme and docs.

* Fix introduction link.

* Restructure docs.

* Place introduction outside the getting started section.

* Improve interfaces docs.

* Minor improvements.

* Fix links.

* Fix links.

* Fix links.

* Add createInstance to interfaces.

* Upgrade dependencies and remove the lockfile.

* Fix Travis link.

* Minor clarification.

* Make a clear distinction between 'state' and 'options' by avoiding 'props'.

* Fix link.

* Add upgrade docs.

* Change the way to override 'resource' from useFetch's 'run' function.

* Avoid memory leaks by using a mock promise.

* Fix FetchError prototype chain.

* Avoid using ESLint config override file because CodeFactor doesn't support it properly.

* Make sure neverSettle is an instance of Promise.

* Exclude some impossible to test paths from code coverage.
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.

useFetch should reject with an Error object
2 participants