-
-
Notifications
You must be signed in to change notification settings - Fork 347
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(msw): make delay optional #1360
Conversation
@@ -5,7 +5,7 @@ export const getDelay = ( | |||
options?: GlobalMockOptions, | |||
): GlobalMockOptions['delay'] => { | |||
const overrideDelay = | |||
typeof override?.mock?.delay === 'number' | |||
override?.mock?.delay !== undefined |
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.
A bit unsure about this, but it feels like override always should override when present, and not only when it's overridden by a number?
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.
Good question. Not sure why this decision was made?
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.
Maybe it was missed when support for functions was added?
ecf6f5f
to
d7b10f3
Compare
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.
@AffeJonsson
Thank you for the nice update. I made a comment in one place, so please check it.
return http.${verb}('${route}', ${ | ||
delay === false | ||
? '() => {' | ||
: `async () => { | ||
await delay(${isFunction(delay) ? `(${delay})()` : delay});` | ||
} |
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.
I thought that the definition itself is unnecessary if delay
is false
, but what do you think?
return http.${verb}('${route}', ${ | |
delay === false | |
? '() => {' | |
: `async () => { | |
await delay(${isFunction(delay) ? `(${delay})()` : delay});` | |
} | |
return http.${verb}('${route}', ${ | |
delay === isFunction(delay) || isNumber(delay) | |
? `async () => { | |
await delay(${isFunction(delay) ? `(${delay})()` : delay});` | |
: '' | |
} |
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.
No, the second argument to http.get must be a function, so we cannot remove the () => {
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.
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.
Oh, that's nice. That's what I wanted.
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.
Thanks!
Status
READY
Description
This allows disabling the
delay
call in mocked api calls, which makes the MockHandler sync instead of async.As per request of Nate on discord
Todos
Steps to Test or Reproduce
Make sure delay still can be set by both number and function and that the MockHandler is async.
Try changing the
delay
settings tofalse
and make sure the MockHandlers become synchronous and no call todelay
is made.