jefflau / jest-fetch-mock

Jest mock for fetch
MIT License
883 stars 116 forks source link

Conditional #128

Closed jefflau closed 4 years ago

jefflau commented 5 years ago
jefflau commented 5 years ago

@yinzara

I've made some changes to change never to dont and added the options object we discussed in the other comments channel. I've also reverted once back to the original implementation as I think it makes sense what you said. I think it was more of the if versions that require the additional response object. Please review what I've done and I think we will still need to make some README changes (I haven't done any) to do some simple examples so it is less confusing for newcomers

I've also added you as a contributor so we can work together on making this library even more awesome (if you don't mind). I've been working on this solo for many years, but it's nice to have someone to bounce ideas off. I'm still okay to take the bulk of the work though

yinzara commented 4 years ago

So sorry it's taken me so long to respond! I'm a serious Burning Man participant so the month of August is always awful. I'll take a look today.

yinzara commented 4 years ago

Alrighty. I think I've cleaned everything up and fixed the README. Can you give one look through yourself and if so, feel free to merge!

yinzara commented 4 years ago

On second thought, there are still some things i want to clean up. Leave this open for now please.

yinzara commented 4 years ago

When I added the ability to pass a function to "mockResponse", the function took in the same parameter types as "fetch" (i.e. urlOrRequest and requestInit). This made conditional functions difficult as you had to account for either parameter types

In the last commit, I added a "normalizeRequest" function that converts the urlOrRequest and requestInit into a http.Request if the parameter is passed as a URL string and a request init object. If a http.Request is passed to "fetch", it is passed to the conditional response function directly. If a conditional response function is not used, the urlOrRequest/requestInit is still passed to the constructor of http.Request before mocking the response. This is to keep consistency and support the new abort functionality.

The means the conditional response function only needs to take in a single argument of type Request, simplifying its usage.

i.e.

mockResponse((urlOrRequest, reqInit) => {
    const url = typeof urlOrRequest === 'string' ? urlOrRequest : urlOrRequest.url
    if (/foo/.exec(url)) {
      return "response"
    } else {
      return "response2"
    }
})

becomes

mockResponse((request) => {    
    if (/foo/.exec(request.url)) {
      return "response"
    } else {
      return "response2"
    }
})

This also follows the true behavior of the fetch library much more closely as it will use the real Request class to validate the request URL and init before returning the mock response should those parameter types be used.

This could be considered a breaking change as tests that used request URLs that are not syntactically valid will now fail

yinzara commented 4 years ago

I also added abort functionality originally defined in PR #112 however it varies some from what was outlined in that PR. Firstly the "signal" is part of the Request or RequestInit, not part of the ResponseInit.

Also, there was no need to attach an abort handler to the signal. Just checking if its aborted at the resolution of the response promise is sufficient.

Readme and tests are included.

yinzara commented 4 years ago

I updated the TypeScript definition to properly define all overloaded functions and optional arguments.

I also updated the optional 'convenience mocked response' parameters that can be passed to the conditional mocking functions to be consistent across all the API.

utricularian commented 4 years ago

Any hint as to when this PR will be merged and a new release cut?

yinzara commented 4 years ago

Just waiting on a final look over by @jefflau and I or he can merge.

KarlisZ commented 4 years ago

Still looking forward to this.

ctaylo21 commented 4 years ago

Really looking forward to this as well!

esetnik commented 4 years ago

I'd love to see this merged as well

yinzara commented 4 years ago

Hey @jefflau I see multiple people waiting for this release. mind giving it your blessing?

jefflau commented 4 years ago

Apologies. Will take a look next week

rkurbatov commented 4 years ago

@jefflau seems like .idea should be added to the .gitignore :) And also would like to see it merged along with AbortController PR.

jefflau commented 4 years ago

Okay, LGTM. @yinzara can you write some release notes and once those are written, I'll add them to the github release notes and merge/publish to npm!

yinzara commented 4 years ago

YAY! Everyone is going to be so excited.

Notes: Conditional Mocking, Request Based Responses, Enable/Disable Facade, and Abort Mocking

Fetches can now be mocked conditionally based upon URL or request matching. This gives the developer the capability to allow some requests to use the real fetch implementation and others to be mocked. See the Readme for detailed usage information.

The async functionality added with the last release has been enhanced. The async function that can be used to return a dynamic response now accepts an optional single API argument of "request" of type Request that can be used to vary the response returned from the mock fetch.

Enabling and disabling jest-fetch-mock can now be handled via the "enableMocks/disableMocks" exported functions instead of relying on custom code.

Request aborting is now supported by the mock framework. Both forced aborting via "mockAbort()" or "mockAbortOnce()" or aborting dynamically by passing a "signal" option to the "fetch" function and calling "abort" on the controller.

jefflau commented 4 years ago

I've merged and will release as 3.0.0

Although it's backward compatible, I think the API has changed enough that I'd like to release it as a major version