Is your feature request related to a problem? Please describe.
I find it confusing that composeMocks() function composes _request handlers_, and not mocks.
Describe the solution you'd like
I think renaming composeMocks() function to composeHandlers() would provide a more domain-driven name, and won't confuse the users.
- const { start } = composeMocks(...)
+ const { start } = composeHandlers(...)
Benefits of the change
Drawbacks of the change
Alternatives to consider
I was also thinking about simply naming it compose(), but I'm afraid this will clash with the similar functional composition utility that is often called compose(). Also, composeMocks() is not the traditional compose function per-say, as it's logic is specific to MSW.
@hehehai, @redraushan, as valuable contributors, what is your opinion on this API change? What consequences do you see this introducing?
Hi @kettanaito,
Definitely, composeMocks is not aligned with what it actually does at the end, and compose is quite generic keywords and agree with you that many libraries already have implemented it.
Although, composeHandlers sounder better option that what we already have, however if you ask me I would suggest to look for something which doesn't include compose word at all.
These are a few of them I can think about at the moment:
responseEngine because we are taking requests and giving you response if youstart() the engine. You wont get the response if you stop() the engine.responseFactory if responseEngine sounds too fancy.workerEngine because under the hood its service worker doing all things.responseHandler because we are taking requests as it is but controlling responseWhat do you think @hehehai ?
Dropping the compose part may be a good idea. I'll try to brainstorm of the names below.
createSchema()withMocks()addMocks()@kettanaito, anything finalised if we are keeping composeMocks name as it is?
I do love how addMocks() reads and sounds, been thinking of it these two days. I find it crucial we also consider this function in isolation (i.e. close to a React component it's associated with). The name should make sense in that case also.
@kettanaito addMocks sounds readable however the only thing which is off is the parameter of addMocks are request handlers, but maybe request handlers can be also considered as mocks. I don't know I am pretty bad at naming things :D
As MSW will eventually support mocking in non-browser environments, I think that such initializer functions like composeMocks() should clearly describe what are they initializing.
I suggest the following API:
useWorker()function useWorker(...handlers: RequestHandler[]): WorkerAPI
Uses a Service Worker with the given request handlers.
useServer()function useServer(...handlers: RequestHandler[]): unknown
Stubs an XHR communication with the given request handlers, thus, mimicking a server.
I'm not found of a word "server" where there are no actual servers, so please feel free to share what name makes more sense to you.
‘useWorker()’ and ‘useServer()’ sounds clean to me, I think we can move forward with this.
in React ‘use’ is the prefix for React hooks. I propose we use the name ‘run’.
runMockServiceWorker
runMockServer
@andreawyss, you're right about the hook prefix. We've had an internal discussion, and that React convention has came up as well.
I like how run can partially replace use in meaning, but in regards to Mock Service Worker functionality, I think it's a little confusing to call runMockWorker() when that function doesn't actually run the worker. The start() function does (so you have granular control over when to start or stop the worker).
I believe such API is misleading:
import { runMockWorker } from 'msw'
// Now I expect that the worker runs after I call this function,
// but in reality it's not.
const { start, stop } = runMockWorker(...)
start()
Perhaps, there's a closer replacement to use in semantical meaning?
// Browser usage
import { setupServiceWorker } from 'msw'
const { start, stop } = setupServiceWorker(options)
start()
// Node usage
import { setupMockServer } from 'msw'
setupMockServer(options)
@andreawyss, that looks lovely!
What do you think if we drop the "Service" part, and it becomes:
// Browser usage
setupMockWorker()
// Node usage
setupMockServer()
I'm also thinking that perhaps it would be a good idea to delegate "starting" the mocked server to the usage surface. That is mainly for two reasons:
ctx.fetch() to fetch original response), but also means they need to be awaited before returned as response.This makes the request handler itself asynchronous.
beforeAll, afterAll, etc.).import { setupMockServer } from 'msw'
import handlers from '../shared/handlers'
describe('Test', () => {
beforeAll(async () => {
const mock = setupMockServer(handlers)
// Or whichever method that's suitable in the context
await mock.open()
})
afterAll(async () => {
await mock.close()
})
})
Great. I like the symmetry of the setup function names.
Also the default response latency (delay) would be about 700 ms for the mock worker and about 5ms for the mock server.
These values can be changed in the options passed to the setup functions.
const mockWorker = setupMockWorker({ handlers, latency: 1000 })
const mockServer = setupMockServer({ handlers, latency: 60 })
Using an option object will allow in the future to add new options as needed.
@andreawyss, agree on the options object. Actually, version 0.14.0 already ships with the options to start() being an object, so you can opt-out from logging, for example. Global delay (latency) may also be a viable thing to configure.
Thank you everybody for this discussion! I've decided to go with the following options:
setupWorker()setupServer()I'm omitting the "mock" part taking in consideration that you import those functions from a library that does mocking already, and it should be a pre-requisite in understanding.
Most helpful comment