-
Notifications
You must be signed in to change notification settings - Fork 282
Update node-fetch to ^2.3.0 (major change)
#180
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
base: master
Are you sure you want to change the base?
Conversation
|
@amitzur this is looking good and definitly a needed upgrade but the build is failing: |
|
I fixed the failing build by upgrading the node version to |
|
@emanuelelongo any chance you could work in #177 with this release, too? |
|
Any update on this ? We are having issue of whatwg-fetch being installed in 3.0 , less stable than 2.0 |
|
+1, would need response.arrayBuffer(). |
|
+1 for the response.arrayBuffer() |
|
Thank you for this and sorry for the incredible delay to reply … I'd like to incorporate your tests in the codebase … would you like to do that or would you be happy if I did this? |
|
@matthew-andrews not sure I understand the question or if it was directed at me |
|
Looks like there's low severity DoS vulnerability for versions < 2.6.1, as noted here. |
|
@matthew-andrews any update on this? 🙂 |
This PR is similar to #157 except for also refreshing other dev dependencies' major versions, otherwise the tests didn't pass (
Uncaught Error [ERR_STREAM_CANNOT_PIPE]: Cannot pipe, not readable).As noted well in that PR, the changelog for breaking changes in Node.js can be found here:
https://github.com/bitinn/node-fetch/blob/master/CHANGELOG.md#v200
I updated the version in
package.jsonto 3.0.0 (major change).I also added reading the headers in the tests, just to verify that this functionality in
node-fetchworks as expected.I believe we can close #157 and accept this. @matthew-andrews wdyt? seems like there's interest in this (#179)