search

jefflau / jest-fetch-mock

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

Incorrect TypeScript type definition in UrlOrPredicate (do* fns) #152

Closed samhh closed 4 years ago

samhh commented 4 years ago

The doMock functions take a UrlOrPredicate, and the types for this appear to be wrong - the callback says it's providing a Request, but it actually provides a string.

See here a minimal repro.

jefflau commented 4 years ago

Hi there,

Thanks for reporting this. Is there any chance you would be able to fix this in a PR. All of the types have been done by external contributors, and I've yet to learn typescript!

yinzara commented 4 years ago

Wow Sam, you're absolutely right. This is completely a mistake on my part. Let me see if there's a way to fix this situation. I'll get back to you. I would like it to follow the typescript type but right now it's not as simple as I'd hoped.

yinzara commented 4 years ago

Would you mind testing my branch with your test library to ensure it works as you expect? The TypeScript definitions have not changed. The JS has been fixed to correctly return the proper types. Just change your dependency from whatever version you specified for jest-fetch-mock in your package.json to "jefflau/fest-fetch-mock#bugfix/152-incorrect-typescript-def"

samhh commented 4 years ago

Hey @yinzara, that branch appears to work for me for both a string and a callback function expecting a request :+1:

yinzara commented 4 years ago

@jefflau You wanna take it from here? I believe this is a good fix. I'm only a little concerned with users of the new feature who weren't using the TypeScript definitions. I feel like this is still worthy of a minor version increase as it's really a bug fix.

jefflau commented 4 years ago

@yinzara Would you recommend just a patch version then since it's a bug fix?