Skip to content
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

How to handle query params #71

Closed
kentcdodds opened this issue Mar 20, 2020 · 7 comments · Fixed by #76
Closed

How to handle query params #71

kentcdodds opened this issue Mar 20, 2020 · 7 comments · Fixed by #76

Comments

@kentcdodds
Copy link
Contributor

I'm pretty sure this is a bug in pathToRegExp (why is this project not using path-to-regexp like many other path parsers?). But if I provide the following URL: http://localhost:8989/api/book?query=:query this gets parsed to the following regex: http:\/\/localhost:8989\/api\/book?query=(?<query>.+?(?=\/|$))\/?

You may notice that the ? for the search string is not escaped. I believe that to be a bug in the parser. If I escape it myself, then things work just fine.

@kentcdodds
Copy link
Contributor Author

Oh, also, is there a way to make the :query param optional?

@kettanaito
Copy link
Member

Why is this project not using path-to-regexp?

Historically, when this project started to use path-to-regexp@3.x it required to have about this amount of abstraction to achieve what MSW needs:

https://github.com/open-draft/msw/blob/ebf2f0c37519db7a420c5ebf6ddbfcda52dcd24f/src/utils/matchPath.ts#L62-L107

With the latest version of path-to-regexp it may make sense to re-introduce it to the library, although I'd like to have some reasons to do so. I'm using a tiny in-house alternative called node-match-path, which may not catch all the use cases, but does what I need.

How to handle query params?

I wouldn't advise going with http://localhost:8989/api/book?query=:query as it makes query position in the URI sensitive. For example, what if you have the following query: ?a=2&query=:query. I'd still expect such URI to match, as it has the query URL query parameter in place, but it won't match the provided RegExp. Generally, I don't think this is a scenario to be handled using path parameters.

Instead, you should specify which URL you would like to mock (without query) and decide on the mocking within the response resolver, basing your logic around req.query:

import { composeMocks, rest } from 'msw'

composeMocks(
  rest.get('http://localhost:8989/api/book', (req, res, ctx) => {
    const { query } = req.query

    // In case there is no such query parameter return the original response
    if (!query) {
      return ctx.fetch(req)
    }

    return res(
      ctx.json({
        title: 'The Magician',
        author: 'Raymond E. Feist'
      })
    )
  })
)

At the moment MSW performs no URL query parsing, which I think we should add. Handling query parameters was on my mind for some time, but I never got the moment to do it. This may be the right moment.

Please, would the solution suggested above satisfy your usage scenario? If not, elaborate why, I'd like to make this into the library in a proper way.

@kentcdodds
Copy link
Contributor Author

I tried the above suggestion and that didn't work for me. I don't have time to investigate why (will try later) but I thought I'd let you know.

@kentcdodds
Copy link
Contributor Author

In case it's helpful, here's what I have tried:

  rest.get(`${apiUrl}/books`, async (req, res, ctx) => {
    console.log('here we go')
    if (!req.query.hasOwnProperty('query')) {
      return ctx.fetch(req)
    }
    const {query} = req.query

    let matchingBooks
    if (query) {
      matchingBooks = matchSorter(allBooks, query, {
        keys: [
          'title',
          'author',
          'publisher',
          {threshold: matchSorter.rankings.CONTAINS, key: 'synopsis'},
        ],
      })
    } else {
      // return a random assortment of 10 books not already in the user's list
      matchingBooks = getBooksNotInUsersList(getUser(req).id).slice(0, 10)
    }
    await sleep()

    return res(ctx.json({books: matchingBooks}))
  }),

@kettanaito
Copy link
Member

Yes, accessing req.query won't work at the moment, as it's not implemented. I'm tacking this in #76.

I've decided to parse the URL using new URL(req.url) and provide the req.query = parsedUrl.searchParams. Standard URLSearchParams interface is handy when getting the query parameters and I think it should be favored over keeping req.query as an object.

The usage of req.query would be as follows:

res.get('/api/books', (req, res, ctx) => {
  const bookId = req.query.get('id')

  return res(ctx.json({ bookId })
})

@kettanaito
Copy link
Member

Released req.query support in 0.10.0. Published the Query parameters documentation to showcase how to access query parameters in response resolvers.

Internally, I made query params and hash to be dropped when request URL are compared.

@kentcdodds, could you please try it? I hope it works well for your use case.

@kentcdodds
Copy link
Contributor Author

Yup, this works:

  rest.get(`${apiUrl}/books`, async (req, res, ctx) => {
    if (!req.query.has('query')) {
      return ctx.fetch(req)
    }
    const query = req.query.get('query')

    let matchingBooks
    if (query) {
      matchingBooks = matchSorter(allBooks, query, {
        keys: [
          'title',
          'author',
          'publisher',
          {threshold: matchSorter.rankings.CONTAINS, key: 'synopsis'},
        ],
      })
    } else {
      // return a random assortment of 10 books not already in the user's list
      matchingBooks = getBooksNotInUsersList(getUser(req).id).slice(0, 10)
    }

    return res(ctx.json({books: matchingBooks}))
  }),

Thanks!

lim-jiwoo added a commit to team-coderder/Frontend-CRA that referenced this issue Jun 11, 2023
URI에 쿼리를 포함하지 않는 게 맞은듯 해서(mswjs/msw#71) 수정함
또한 스케쥴의 요일이 대문자가 아니면 제대로 변환되지 않아서 수정함
또한 추천 핸들러만 작동시킬 수 있도록 따로 분리함
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants