Conversation
This ensures that 'isomorphic-fetch' is imported before anything else. Previously 'isomorphic-fetch' was imported at the beginning of each separate module that used 'fetch', which was needlessly verbose and easy to forget to do (it was missing from one module already). This would normally break the tests, as the tests don't import modules via the main index file. This was resolved by adding a `setupTests` file that imports `isomorphic-fetch` before running any tests.
The `fetch` API will throw an exception if a network error is encountered during a `fetch` invocation, but an unsuccessful response will not. We should never assume that the response from `fetch` is successful without checking first. We can't even assume that successfully calling `.json()` on a request implies that it succeeded, because failed responses can pass information in the response body as well. A `successfulFetch` utility method has been added that verifies the fetch response was successful, and throws an error if it was not. This method is now used for the other two `fetch` utility methods, and each instance of `fetch` has been updated to use one of these methods. New tests have been added for `successfulFetch`. `fetch-mock` was updated to make it easier to specify the status of responses from calls to the mock fetch instance.
Codecov Report
@@ Coverage Diff @@
## develop #157 +/- ##
======================================
Coverage 100% 100%
======================================
Files 22 22
Lines 1484 1472 -12
Branches 206 207 +1
======================================
- Hits 1484 1472 -12
Continue to review full report at Codecov.
|
danfinlay
approved these changes
Sep 20, 2019
Contributor
danfinlay
left a comment
danfinlay
left a comment
There was a problem hiding this comment.
This looks good. Great that mobile will benefit from these improvements!
kanthesha
pushed a commit
that referenced
this pull request
Oct 11, 2023
MajorLift
pushed a commit
that referenced
this pull request
Oct 11, 2023
* Add 'isomorphic-fetch' import to index This ensures that 'isomorphic-fetch' is imported before anything else. Previously 'isomorphic-fetch' was imported at the beginning of each separate module that used 'fetch', which was needlessly verbose and easy to forget to do (it was missing from one module already). This would normally break the tests, as the tests don't import modules via the main index file. This was resolved by adding a `setupTests` file that imports `isomorphic-fetch` before running any tests. * Handle unsuccessful fetch attempts The `fetch` API will throw an exception if a network error is encountered during a `fetch` invocation, but an unsuccessful response will not. We should never assume that the response from `fetch` is successful without checking first. We can't even assume that successfully calling `.json()` on a request implies that it succeeded, because failed responses can pass information in the response body as well. A `successfulFetch` utility method has been added that verifies the fetch response was successful, and throws an error if it was not. This method is now used for the other two `fetch` utility methods, and each instance of `fetch` has been updated to use one of these methods. New tests have been added for `successfulFetch`. `fetch-mock` was updated to make it easier to specify the status of responses from calls to the mock fetch instance.
MajorLift
pushed a commit
that referenced
this pull request
Oct 11, 2023
* Add 'isomorphic-fetch' import to index This ensures that 'isomorphic-fetch' is imported before anything else. Previously 'isomorphic-fetch' was imported at the beginning of each separate module that used 'fetch', which was needlessly verbose and easy to forget to do (it was missing from one module already). This would normally break the tests, as the tests don't import modules via the main index file. This was resolved by adding a `setupTests` file that imports `isomorphic-fetch` before running any tests. * Handle unsuccessful fetch attempts The `fetch` API will throw an exception if a network error is encountered during a `fetch` invocation, but an unsuccessful response will not. We should never assume that the response from `fetch` is successful without checking first. We can't even assume that successfully calling `.json()` on a request implies that it succeeded, because failed responses can pass information in the response body as well. A `successfulFetch` utility method has been added that verifies the fetch response was successful, and throws an error if it was not. This method is now used for the other two `fetch` utility methods, and each instance of `fetch` has been updated to use one of these methods. New tests have been added for `successfulFetch`. `fetch-mock` was updated to make it easier to specify the status of responses from calls to the mock fetch instance.
Mrtenz
pushed a commit
that referenced
this pull request
Oct 16, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
fetchAPI will throw an exception if a network error is encountered during afetchinvocation, but an unsuccessful response will not. We should never assume that the response fromfetchissuccessful without checking first. We can't even assume that successfully calling
.json()on a request implies that it succeeded, because failed responses can pass information in the response body as well.A
successfulFetchutility method has been added that verifies the fetch response was successful, and throws an error if it was not. This method is now used for the other twofetchutility methods, and each instance offetchhas been updated to use one of these methods.New tests have been added for
successfulFetch.fetch-mockwas updated to make it easier to specify the status of responses from calls to the mock fetch instance.Also, 'isomorphic-fetch' has been imported in the index. This ensures that 'isomorphic-fetch' is imported before anything else.
Previously 'isomorphic-fetch' was imported at the beginning of each separate module that used 'fetch', which was needlessly verbose and easy to forget to do (it was missing from one module already).
This would normally break the tests, as the tests don't import modules via the main index file. This was resolved by adding a
setupTestsfile that importsisomorphic-fetchbefore running any tests.