jefflau / jest-fetch-mock

Jest mock for fetch
MIT License
886 stars 117 forks source link

Use cross-fetch to avoid missing "Response.blob()" #80

Closed GrahamTheCoder closed 5 years ago

GrahamTheCoder commented 6 years ago

Fixes #45 Problem: Isomorphic-fetch hasn't been updated in ages and hence is missing things like the definition of Response.blob().

Solution:

jefflau commented 6 years ago

@drewler does this implementation work for you?

spirosikmd commented 6 years ago

Hi @GrahamTheCoder and @jefflau! I tried that implementation. I was indeed getting errors regarding the non-absolute URLs. I updated my tests to fix these errors using, for example, this URL http://localhost. However, now I am getting another error

request to http://localhost/ failed, reason: connect ECONNREFUSED 127.0.0.1:80

It seems like the mocking doesn't work and fetch is trying to make an actual call? Any idea? Thanks!

spirosikmd commented 6 years ago

I fixed the issue! I didn't change my setup file to mock cross-fetch. So after doing jest.setMock('cross-fetch', fetch); everything works with the implementation of this PR.

jefflau commented 6 years ago

@spirosikmd Can you create a barebones repository with this PR to show it working? I've merged PRs in the past like this that have broken many things for many people which I've had to revert, so I'd like to avoid that. However quite a few people have been requesting supporting for blob()

spirosikmd commented 6 years ago

@jefflau https://github.com/spirosikmd/example-jest-fetch-mock is an example repository that uses this branch. Let me know what you think. I can add more test cases to test things out.

jefflau commented 6 years ago

Thanks for that. I’m going to look into integrating this into the main repo with some other tests to keep things more stable moving forward. If there are any other more comprehensive tests you want to add, please let me know

spirosikmd commented 6 years ago

I wrote a few more tests cases. I wrote one to cover specifically the Response.blob() case!

GrahamTheCoder commented 5 years ago

^ Good to see people are making use of this work, what do we need to do to get it merged?

jefflau commented 5 years ago

Hi @GrahamTheCoder. I've been trying to integrate @spirosikmd's tests into the main library. I had some issues with it, but my work on setting up tests is on the `setting-up-tests' branch. I've been a bit busy so I haven't had to take a proper look at why the CSV test is failing. If anyone wants to try and get these tests working, I'd be happy to merge this in.

If not I'll try and find some time soon.

jefflau commented 5 years ago

Accidentally merged this pre-emptively, but still working on the tests

jefflau commented 5 years ago

Ok I unreverted the revert here: https://github.com/jefflau/jest-fetch-mock/pull/90

Tests should be running correctly and I'm going to deploy to a new minor version upgrade