axios-mock-adapter
axios-mock-adapter copied to clipboard
Reconsider order of operations with regards to replyOnce
Love the library but this one detail is preventing me from adopting it. Consider Jest's implemention of mockImplementationOnce https://jestjs.io/docs/en/mock-function-api.html#mockfnmockimplementationoncefn
const myMockFn = jest
.fn()
.mockImplementationOnce(cb => cb(null, true))
.mockImplementationOnce(cb => cb(null, false));
myMockFn((err, val) => console.log(val)); // true
myMockFn((err, val) => console.log(val)); // false
So far looks very similar to replyOnce right? This next section is where it differs:
const myMockFn = jest
.fn(() => 'default')
.mockImplementationOnce(() => 'first call')
.mockImplementationOnce(() => 'second call');
// 'first call', 'second call', 'default', 'default'
console.log(myMockFn(), myMockFn(), myMockFn(), myMockFn());
With axios-mock-adapter's current order of operations the log would be 'default', 'default', 'default', 'default'. Reason being is that replyOnce and reply are kept in the same list and replyOnce handlers are pushed to the end https://github.com/ctimmerm/axios-mock-adapter/blob/master/src/index.js#L213
Adopting this pattern would be a breaking change and require a bit of reorganization in terms of the internal data structures. If you think this fits in with the project I'd be happy to contribute rather than purse a fork
Could you give an example of how you're trying to use axios-mock-adapter?
@ctimmerm here's an example of how I would like to use the library in an integration test setting
it('should show an error when API is 500-ing', async () => {
mockHttpClient.onAny().replyOnce(500);
const { queryByText } = render(<App />);
await waitFor(() => expect(queryByText(/error/i)).toBeVisible());
})
The problem is that it would be a very large breaking change requiring large effort and having some behavior that can't be migrated.
I think it would work better as an addition to the current API rather than a change, e.g. by introducing a new replyOnceFirst method.
Happy to implement it as an additional method 👍
@slightlytyler was wondering if you have any plan to open a PR? I see you've published your fork on npm
@sanpoChew I'm no longer on the project that needs this so probably not. My fork has changes that match what I originally envisioned which would be a breaking change, not ctimmerm's proposal. I made a similar proposal for a different tool which has landed now so there are other options if you need something like this API right now in an available library https://github.com/mswjs/msw/issues/128
any progress on this override topic?