mswjs / msw

Industry standard API mocking for JavaScript.
https://mswjs.io
MIT License
16.03k stars 526 forks source link

Feature request: better support for stateful response logic #718

Closed drivasperez closed 3 years ago

drivasperez commented 3 years ago

(This might well be a duplicate. If so I'm sorry, it was a tough one to search for.)

Is your feature request related to a problem? Please describe. I have a few mocks where I'm simulating a polling endpoint, where I want the endpoint to return a 'processing' response continually for some number of times (a hundred, let's say), then finally return either 'success' or 'error'.

At the moment, as I understand it, this is a bit unergonomic with MSW. I have to maintain some state in my test code and reference it in the response handlers like this:

function() {
    let count = 0;
    server.use(
        rest.get('/polling-endpoint', (_req, res, ctx) => {
            if (count < 100) {
                count += 1;
                return res(ctx.json(pendingResponse));
            }
            return res(ctx.json(successResponse));
        })
    )
}

This is fine, but it makes pulling out the resolver into a utility function that I can use elsewhere quite difficult. Alternatively I can do this, which works but is definitely not ideal:

function() {
    const pending = (req, res, ctx) => res.once(ctx.json(pendingResponse));
    const ok = (req, res, ctx) => res.once(ctx.json(okResponse));
    server.use(
        rest.get('/polling-endpoint', pending),
        rest.get('/polling-endpoint', pending),
        rest.get('/polling-endpoint', pending),
        // ...repeat 97 more times
        rest.get('/polling-endpoint', ok),
    )
}

Describe the solution you'd like The simplest solution to this specific problem I can think of is to add a .times() method to res, which would work like .once() but allow you to specify the number of times the handler works before its internal shouldSkip state goes to true.

function() {
    const pending = (req, res, ctx) => res.times(100, ctx.json(pendingResponse));
    const ok = (req, res, ctx) => res.once(ctx.json(okResponse));
    server.use(
        rest.get('/polling-endpoint', pending),
        rest.get('/polling-endpoint', ok)
    )
}

Then .once() would just be an alias for .times(1).

A more complicated but more powerful solution might be to allow you to pass a generator to rest.get. Then my count variable from the first example can live inside the handler:

function() {
    server.use(
        rest.get('/polling-endpoint', function* (_req, res, ctx) {
            let count = 0;
            if (count < 100) {
                count += 1;
                yield res(ctx.json(pendingResponse));
            }
            return res.once(ctx.json(successResponse));
        })
    )
}

Describe alternatives you've considered

I might just be missing something and this isn't a problem. If so, great!

Additional context Add any other context or screenshots about the feature request here.

kettanaito commented 3 years ago

Hey, @drivasperez. Thanks for raising this.

Adding support for an API like this is always a delicate matter. One of our core design principles is keeping the library's public API as plain and straightforward as possible. On the other hand, we want to support some of the common scenarios our users are facing. It comes down to the logic encapsulation and designing an API versatile enough to feel empowering rather than limiting.

When speaking of something like res.times(), I can definitely see its practical use, but I can also see how strict that response modifier becomes. If we establish more such modifiers alongside res.once, the library starts to give quite too many promises as to what it's responsible for. In the case of res.times, that'd be keeping internal counter and conditional resolver logic for you. Once again, this is practical, but I find it introducing more problems than it solves. Just a fact of us exposing such API leads developers to believe MSW should be responsible for more than request/response pairing, which it really shouldn't. Then some developers would want to have a custom internal state predicate instead of a simple counter, others would want for the conditional response logic to be integrated in a different way, and so on.

What I particularly like about the current API is that it's designed to be composable. MSW encourages you to encapsulate your custom logic in higher-order resolvers and handlers, if necessary. This introduces 0 overhead, makes you in charge of the logic, and allows you to craft it precisely to your needs.

I'd suggest creating a withPolling high-order resolver to achieve what you described above:

import { rest } from 'msw'

function withPolling(times, resolver) {
  let count = 0

  return (req, res, ctx) => {
    count++
    const isPending = count <= times
    return resolver(req, res, ctx, isPending)
  }
}

server.use(
  rest.get(
    '/poll',
    withPolling(100, (req, res, ctx, isPending) => {
      if (isPending) {
        return res(ctx.json(pendingResponse))
      }

      return res(ctx.json(okResponse))
    })
  )
)

Please correct me if I've misunderstood your expectations towards such logic.

drivasperez commented 3 years ago

Hey, thanks for getting back to me so quickly. I agree that .times() would be slippery-slopeish. The HOF is a nice solution, but I do think it's a case where being able to use a generator would be nicer (especially for cases that are more complicated than this one).

I can see why you wouldn't want to add that complexity to MSW, though, and I can also see an argument that really complicated test cases shouldn't be encouraged in the first place.

Anyway, thanks again.

kettanaito commented 3 years ago

Oh, sorry, I completely forgot to reply regarding the generators! Yes, it sounds like the best fit, and I wanted to ask you if you could give the generator approach a try with the current MSW version? I know it'll most likely fail, but I'm curious as to how and why it'd fail. Supporting generators as response resolvers would indeed be a great addition to the library.

drivasperez commented 3 years ago

Sure. It doesn't error out, but it seems like the mock just returns the generator that's created when you call the generator function. (And obviously it doesn't typecheck.)

I think the change to make would be on this line: https://github.com/mswjs/msw/blob/1562bfc901c930bffc8c5eb150bb855b54020676/src/handlers/RequestHandler.ts#L175

Instead of returning mockedResponse here, we'd first have to check if it is a generator, and if so stash it, and keep calling .next() on it every time a request comes through, returning the result until it returns {done: true}.

kettanaito commented 3 years ago

Thank you for the effort to add generator support to the response resolver! I'll review your pull request and we'll land this great improvement.