-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat: allow more types of overloads #26
Conversation
096a4d5
to
6133016
Compare
"eslint": "^8.56.0", | ||
"jsdom": "^24.1.0", | ||
"prettier": "^3.3.2", | ||
"typescript": "~5.5.0", | ||
"vitest": "2.0.2" | ||
"vitest": "^2.1.3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bumped the devDependency
in order to allow checking coverage
Hmmmm, I'm not sure if I love the idea of even more ways to accomplish the same thing, I think this project already has too many overloads already. I might even suggest that we move towards a future where a But I see that today, the only time a Response can be provided is when returned in a function. So this PR would be a necessary step anyway. I think this makes sense to do for now, and hopefully we can find a way to deprecate simple strings and objects being passed to Do you think there are any docs that should be changed if we merge this feature? |
Agreed. I'd say we could narrow it down by limiting the current definition: type ResponseProvider = ResponseLike | ((request: Request) => ResponseLike | Promise<ResponseLike>);
type ResponseLike = MockResponse | ResponseBody | Response;
type ResponseBody = string; down to: type ResponseProvider = ResponseLike | ((request: Request) => ResponseLike | Promise<ResponseLike>);
type ResponseLike = ResponseBody | Response;
type ResponseBody = string; or even type ResponseProvider = Response | ((request: Request) => Response | Promise<Response>); although I see the added value of keeping the response body overload.
It's a superset of the current API, so I wouldn't mind just keeping it as-is. But I am also in favor of rewriting the whole docs for the 1.0 version, when we introduce breaking changes. My ideas for 1.0 would be:
|
This PR makes it possible to use more types of overloaded functions:
This could make certain cases less awkward, like #25 (comment):