mswjs / msw

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

Rest PUT handler should not need search parameters #638

Closed juanca closed 3 years ago

juanca commented 3 years ago

Environment

Name Version
msw 0.26.0
browser Chrome Version 88.0.4324.192 (Official Build) (x86_64)
OS macOS Version 11.1

Request handlers

import { setupWorker } from 'msw';

const mockServiceWorker = setupWorker();

mockServiceWorker.setup = (handlers) => {
  beforeAll(() => {
    mockServiceWorker.start({
      onUnhandledRequest: (request) => {
        // Without overriding the default console error, Jasmine can't catch this and won't fail the test
        console.error(`Received request ${request.method} ${request.url.href}, which has no handler.`); // eslint-disable-line no-console
      },
      waitUntilReady: true,
    });
  });
  beforeEach(() => {
    mockServiceWorker.resetHandlers();
    mockServiceWorker.use(...handlers);
  });
  afterAll(() => {
    mockServiceWorker.stop();
  });
};

export default mockServiceWorker;

Then the actual test:

describe('timer', () => {
  mockServiceWorker.setup([
    rest.put('/api/v1/timer/start', (req, res, context) => {
      const params = {
        accumulated_time: req.url.searchParams.get('accumulated_time'),
      };

      if (isEqualWith(params, { accumulated_time: 0 })) {
        return res(
          context.status(200),
          context.json({ c: true }),
        );
      }
    }),
    rest.put('/api/v1/timer/start?accumulated_time=0', (req, res, context) => {
      return res(
        context.status(200),
        context.json({ b: true }),
      );
    }),
    rest.put('http://localhost:9876/api/v1/timer/start?accumulated_time=0', (req, res, context) => {
      return res(
        context.status(200),
        context.json({ a: true }),
      );
    }),
  ]);

  beforeEach(function () {
    mockServiceWorker.printHandlers();
  });

  it(...);
});

Actual request

export const startTimer = (durationInSeconds = 0) => {
  const url = `/api/v1/timer/start?accumulated_time=${durationInSeconds}`;
  const fetchOptions = {
    method: 'put',
    credentials: 'same-origin',
    headers: { 'X-CSRF-Token': window.authenticity_token },
  };

  return fetch(url, fetchOptions);
};

Current behavior

I am having trouble figuring out how to properly mock a PUT request.

As you can see in the handlers, I am testing out which of the 3 possible ways to specify the handler URL. The test warns about it not using the ideal pattern. See screenshot of dev tools:

image

And here is the preview of the response:

image

In addition, I am not sure why the worker is 404ing -- almost like it is forwarding the request? Probably not related but here is a screenshot of the details:

image

Expected behavior

I expect the service worker to intercept the PUT request with the proper URL and without any parameters specified. Just like a GET request.

kettanaito commented 3 years ago

Hey, @juanca.

As the warning rightfully stands, the ?accumulated_time=0 is a redundant part of the request handler's URL. See the reason why.

Resource path vs parameters

Query parameters are parameters—arguments sent to a resource on the server. They don't represent a path to the resource:

When you construct a REST request handler, the first argument you give to it is a resource path:

rest.put('/api/v1/timer/start', resolver)

But I need the value of that parameter!

You can access all query parameters in req.url.searchParams:

rest('/api/v1/timer/start', (req, res, ctx) => {
  const accumulatedTime = req.url.searchParams.get('accumulated_time')
})

But I only want to mock a request with a specific query parameter value!

  1. Capture a query-less resource.
  2. Access the value of the needed query parameter.
  3. Mock the response conditionally based on the query parameter.
rest('/api/v1/timer/start', (req, res, ctx) => {

  if (req.url.searchParams.has('accumulated_time')) {
    // No "?accumulated_time" query parameter present—bypass this request.
    return
  }

  // Query parameter is present! Mocking the response...

  return res(ctx.json({ time: req.uql.searchParams.get('accumulated_time') })
})

I highly suggest reading about Path matching, which illustrates different ways to capture a request and their difference.

juanca commented 3 years ago

If that's the case then I think there's a bug. I have a handler that follows your suggestions and it does not "match". I have to specify the search params in the path in order for MSW to "match" it.

Or am I misunderstanding the order of precedence for handlers? Is it FIFO or LIFO?

kettanaito commented 3 years ago

The first matching handler that returns a mocked response is used:

https://github.com/mswjs/msw/blob/48f401a7425ed720d757e372fa9aaaba57030da7/src/utils/getResponse.ts#L36-L56

If you have the handlers set up literally as in the code example above, then:

  1. The rest.put('/api/v1/timer/start') handler is used if the request satisfies its if (isEqualWith(params, { accumulated_time: 0 })) { condition.
  2. If not, lookup jumps to the next matching handler: rest.put('/api/v1/timer/start?accumulated_time=0', which always returns a mocked response, so it will be used.

Query parameters are dropped entirely by MSW when comparing matching URLs. All three of your request handlers will match as they are identical in terms of the request path. The first one that produces a mocked response will be used.

I see little value in having three handlers set up for the same request URL. I suggest merging them into a single handler if you need some complex logic, like conditional responses.

This is the handler that will match PUT /api/v1/timer/start:

rest.put('/api/v1/timer/start', (req, res, ctx) => {
  return res(ctx.json({ matches: true })
})

If something isn't mocked when it should be, always follow these guidelines.

juanca commented 3 years ago
  1. The rest.put('/api/v1/timer/start') handler is used if the request satisfies its if (isEqualWith(params, { accumulated_time: 0 })) { condition.
  2. If not, lookup jumps to the next matching handler: rest.put('/api/v1/timer/start?accumulated_time=0', which always returns a mocked response, so it will be used.

D:

This is good to know! I've been stuffing my handlers with a bunch of if(...)s.

But I digress.

Okay, I think perhaps there is something wrong with my isEqualWith. I'll look into it.

juanca commented 3 years ago

Okay, the lesson here is:

The values of the search parameters are always strings. This was a lesson I learned after making this issue. Comparing the integer 0 and string 0 was the crux of the issue here.

@kettanaito Thanks for the explanation, it helped pinpoint the problem.