mswjs / msw

Seamless REST/GraphQL API mocking library for browser and Node.js.
https://mswjs.io
MIT License
15.6k stars 498 forks source link

Adopt Fetch API primitives #1404

Closed kentcdodds closed 10 months ago

kentcdodds commented 1 year ago

Scope

Improves an existing behavior

Compatibility

Feature description

I think this could be done in a way that is non-breaking. But I think it'd be great to be able to write my handlers similar to how I write my Remix loaders/actions. They receive an object with { request, params, context } and return a Response. Could even use the Remix ponyfill for Request/Response: https://github.com/remix-run/web-std-io

kettanaito commented 1 year ago

Thanks for writing this down, @kentcdodds. I've been thinking about using native Fetch's Request and Response for some time now. I will provide this thread with more thoughts/concerns around this change in the future.

kettanaito commented 1 year ago

First of all, I think this is a good change. Embracing the platform is something MSW has been doing almost since the beginning, both when using the Service Worker API as well as operating with Fetch Response instances in the browser. I think we should also embrace these APIs for public usage (currently, used only internally). Overall, this is the direction that our public API will take sooner or later.

In a very drafty fashion, this is the public API I imagine:

// handlers.js
import { rest } from 'msw'

export const handlers = [
  rest.get('/user', () => {
    const user = JSON.stringify({ name: 'John' })
    return new Response(user)
  })
]

Now, let’s talk about some concerns with such API change.

Insufficient data

Both Request and Response instances are insufficient to describe real-world requests and responses. Some of the examples of the missing data are:

The fetch API also lacks certain convenience features such as responding with a JSON body quickly. I think it’s too verbose to be consumed directly.

Environment support

MSW still supports versions of Node.js that have no native Request or Response classes. I believe those are added in Node.js 18 with the addition of the native fetch but I may be mistaken. This means that our users wouldn’t be able to construct new Response() in handlers because that class does not exist in Node.js.

Even outside of support, Node.js handles requests and responses differently based on the request issuing actor:

This difference was our main motivator to introduce IsomorphicRequest and IsomorphicResponse classes that can describe requests and responses regardless of the request actor.


Next, let’s talk about how we can mitigate some of my concerns above.

Similar to Remix, MSW can provide additional request information like path parameters or cookies alongside the request instance instead of embedded in it:

// This is an approximate API that reflects that of Remix.
rest.get('/resource/:id', ({ request, params, cookies }) => {
  console.log(params.id)
})

I suppose we can help with faster response creation in a similar way:

rest.post('/user', ({ request, json }) => {
  return json({ id: 'Name' })
})

But with such API I’d be careful with what this whole object represents. If we stuff it with all sorts of utilities, it will become hard to use. I also like the functional approach the current API has as you can abstract specific parts of the response to functions and reuse them.

This surfaces rather well if we decide to compose multiple response data at once:

rest.get('/resource', ({ json, set, delay }) => {
  // return ???
})

This brings us to the point that there are 3 kinds of API we should expose:

Okay, what is that “network record” thingie? I’d describe it as the response representation in the “Network” tab. In the addition to the Response data, a network record also controls response timing (and, perhaps, other things in the future). This may also be an API that allows you to return network errors, as those are not responses in the sense of fetch API.

In this regard, the resolver function should really return an instruction on how to respond, and response is just a part of that instruction.

import { json } from 'msw'

rest.get('/resource', ({ request }, respondWith) => {
  return respondWith(
    new Response('hello'), // or "json()"
    { timing: 1200 }
  )
})

You can still return a Response instance straight away. In that case, it’s a delay-less 1-1 response as you specified.

As I think about this API, the composability concern becomes bigger. The response/network record separation feels natural but we need to design it in a way that would allow people to abstract, compose, and reuse certain response/network behaviors with ease.

API draft

To conclude this initial post, this is somewhat the API to consider:

import { compose, json, status, NetworkEntry } from 'msw'

export const handlers = [
  rest.get('/resource', ({ request }) => {
    // Returning a plain fetch Response.
    return new Response('hello world')
  }),

  rest.post('/user/:id', ({ request, params }) => {
    const { id } = params
    // Returning a plain Response but using built-in
    // utilities to compose it more efficiently.
    return json({ id })
  }),

  rest.post('/movie', async ({ request }) => {
    // Reading request per fetch API.
    const movie = await request.json()
    // Composing multiple response utilities
    // into a single "Response" instance.
    const response = compose(
      status(201),
      json(movie),
    )

    // Responding with a network record (entry)
    // to control response metadata like timing.
    return new NetworkEntry(response, { timing: 1200 })
  })
]

I’d much appreciate a discussion on this so we could find the right API together.

kettanaito commented 1 year ago

While this is a breaking change, there are a couple of things I'd like to keep:

kentcdodds commented 1 year ago

Personally, I'd strongly advise with sticking with the standard objects and use utility functions to help with common use cases. Otherwise you violate the principle of least surprise leading to confusion on the part of users. It's almost worse to make it look like a standard object with slight changes because people will assume that it's standard and when things don't work the same they may get confused. It also means that code isn't as portable. A firm stance on standards is the approach that I took with Testing Library and it was very successful for both the library as well as the users. Same goes for Remix.

Having to construct new URL is a small price to pay for consistency and fewer API surprises.

kentcdodds commented 1 year ago

The API you've shown works well for me. I think it's ok that a handler could return a Response or a special NetworkEntry utility object for special cases. Though I think it may be simpler if we figure out how to make it so it only returns a Response and utilities can be used (via compose) to add special headers that MSW could read/remove. Just to make it more consistent. But I don't think that bit is as important personally.

mattcosta7 commented 1 year ago

We could even expose the request URL, pre wrapped as an additional argument to that callback, if necessary ,which would keep the request object native, but avoid needing to wrap it in new URL manually - although I'd agree it seems like it isn't unreasonable to wrap those where necessary instead of on every request

kentcdodds commented 1 year ago

Yup. I just think it's an important imperative to keep things as standard as possible rather than almost standard. Even in the name of convenience.

kettanaito commented 1 year ago

@kentcdodds, agree on the adhering to standards point.

Though I think it may be simpler if we figure out how to make it so it only returns a Response and utilities can be used (via compose) to add special headers that MSW could read/remove.

As practice shows that headers are not the best place to persist library implementation details. We can try that route, as we're speaking of response header in this context, but I'd leave room open for better API.

@mattcosta7, yeah. That's the route I'd take with params and cookies. The read-only data is rather straightforward to derive from the Request instance. It is when people wish to alter the intercepted requests that troubles begin. People may want to do the following:

  1. Alter query parameters of the intercepted request;
  2. Alter headers/cookies/body of the intercepted request;

In this regard, I'd encourage them to create a new Request instance, use the intercepted instance as the RequestInit to that new instance, and alter it upon construction.

// This is a response patching scenario with extra steps.
rest.get('/resource', async ({ request }) => {
  const proxyRequest = new Request(request)
  proxyRequest.headers.delete('Some-Header') // is this permitted?

  const originalResponse = await fetch(proxyRequest)
  return new Response(...)
})
kettanaito commented 1 year ago

One of the things I wanted to do for some time is separate mocked response from the resolver instructions (see #1207). Mocked response is not the only thing that the resolver may return (i.e. may also return a network error). I think it makes sense for the resolver to always return an instruction (internally):

This is not a user-facing API but rather how I'd like to implement things under the hood. Something like a set of possible instructions with their data:

type ResolverInstruction =
  { type: 'USE_MOCK', mockedResponse: Response } |
  { type: 'USER_NETWORK_ERROR', networkError: NetworkError } |
  { type: 'PASSTHROUGH }

Separating instructions (intentions) from the mocked payload should bring clarity to the API, which may eventually make the public API easier as well. For example, there's no way to respond with a network error once at the moment (#413). It's because we couple the "once" capabilities into the response composition (res.once()), mixing instruction and its payload. By returning the instruction object, we may embed behaviors like once into the instructions themselves, sitting alongside the mocked response or network error or anything else.

kettanaito commented 1 year ago

One of the great side-effects on adhering to the spec is that we could then minimize issues like #1327.

kettanaito commented 1 year ago

Using Fetch API Response

MSW already uses Response as internal representation of a mocked response. We will aim to support it being returned from the response resolver directly.

That being said, I find Fetch API Response to be unsuitable to declare responses. It's great for describing existing responses but it lacks a lot of flexibility for error-prone response declaration:

// Currently (MSW)
res(ctx.json({ mocked: true }))

// Fetch API Response
new Response(
  JSON.stringify({ mocked: true }),
  {
    headers: {
      'Content-Type': 'application/json',
    },
  },
)

With this in mind, I'd consider a custom abstraction that would construct responses according to the spec, return the Fetch API Response, but provide the user with a flexible API that helps declare HTTP responses quickly and concisely.

Proposed API: HttpResponse

class HttpResponse {
  static text(body?: string, init?: ResponseInit): Response {
    return new Response(body, init)
  }

  static json(body?: object, init?: ResponseInit): Response {
    const headers = new Headers(init?.headers)
    headers.set('Content-Type', 'application/json')

    return new Response(JSON.stringify(body), {
      ...init,
      headers,
    })
  }

  static arrayBuffer(body?: ArrayBuffer, init?: ResponseInit): Response {
    return new Response(body, init)
  }

  static formData(body?: FormData, init?: ResponseInit): Response {
    return new Response(body, init)
  }
}

// Usage
HttpResponse.text('hello', { status: 301 })
HttpResponse.json({ a: 1 })
HttpResponse.arrayBuffer(new Uint8Array())
HttpResponse.formData(new FormData())

Such HttpResponse API would:

mattcosta7 commented 1 year ago

Using Fetch API Response

MSW already uses Response as internal representation of a mocked response. We will aim to support it being returned from the response resolver directly.

That being said, I find Fetch API Response to be unsuitable to declare responses. It's great for describing existing responses but it lacks a lot of flexibility for error-prone response declaration:

  • Totally possible to set the response body with the wrong/missing Content-Type header (generally, no way to collocate response body value and its Content-Type);
  • No way to control response.type;
  • No way to set Set-Cookie response header (a security consideration but still a reason why would need an abstraction to allow this just as we do right now with ctx.cookie());
  • response.statusText is not inferred from response.status, which, in itself is against the spec (all status codes have "" as statusText value);
  • No way to assert response body type on the type-level to provide compatibility with response structures described in TypeScript;
  • Response declarations via Response are extremely lengthy. Compare these:
// Currently (MSW)
res(ctx.json({ mocked: true }))

// Fetch API Response
new Response(
  JSON.stringify({ mocked: true }),
  {
    headers: {
      'Content-Type': 'application/json',
    },
  },
)

With this in mind, I'd consider a custom abstraction that would construct responses according to the spec, return the Fetch API Response, but provide the user with a flexible API that helps declare HTTP responses quickly and concisely.

Proposed API: HttpResponse

class HttpResponse {
  static text(body?: string, init?: ResponseInit): Response {
    return new Response(body, init)
  }

  static json(body?: object, init?: ResponseInit): Response {
    const headers = new Headers(init?.headers)
    headers.set('Content-Type', 'application/json')

    return new Response(JSON.stringify(body), {
      ...init,
      headers,
    })
  }

  static arrayBuffer(body?: ArrayBuffer, init?: ResponseInit): Response {
    return new Response(body, init)
  }

  static formData(body?: FormData, init?: ResponseInit): Response {
    return new Response(body, init)
  }
}

// Usage
HttpResponse.text('hello', { status: 301 })
HttpResponse.json({ a: 1 })
HttpResponse.arrayBuffer(new Uint8Array())
HttpResponse.formData(new FormData())

Such HttpResponse API would:

  • Collocate response body and its Content-Type via static methods like .text() or .json().
  • It would throw an exception if you provide an unsuitable Content-Type for the body type you've given (i.e. setting Content-Type: plain/text for a JSON response body).
  • It would still allow for customization via the init?: ResponseInit argument to set custom status, status text, and headers.
  • It would return a plain Fetch API Response instance so you could pass its output to anything that's compatible with it (e.g. a fetch() call or MSW's response resolver).
  • Response methods can have BodyType generics to validate response body compatibility on the type-level (like MSW has now).

I like this conceptually. Exposing helps to build ergonomic responses seems like a good way to go (and I believe remix does similar for loaders?)

I'm not sure we'd need to have the class construct, instead of just the methods themselves?

kettanaito commented 1 year ago

Yeah, the class construct is pseudocode. I ended up having HttpResponse as a plain object (capitalized so it feels like a namespace). It works great, it's concise, and it's type-safe. I will move forward with HttpResponse for now.

and I believe remix does similar for loaders?

Yes! Remix exposes json, redirect, and perhaps some other methods to construct a Response instance easier. Let us see what HttpResponse brings to the table.

kettanaito commented 1 year ago

Do you think convenience shorthands have a place to be? Something like HttpResponse.redirect()?

vasilii-kovalev commented 1 year ago

One thing I find useful (among others) when working with MSW is the ability to provide types for request and response, like this:

rest.get<
  RequestBody,
  PathParams,
  ResponseBody
>(
  URL,
  (request, response, context) => {
    // Some code.
  }
)

How would it look like when the changes are applied? Do request, response and context share the types, or they are completely separated? Because if they are connected, importing json from the library rather than receiving it from the handler would break the connection.

vasilii-kovalev commented 1 year ago

IMO, from the library's perspective, it would be beneficial to use the platform under the hood. But from the consumer's perspective, a convenient API would be more useful rather than a compliant one. It is the same as using fetch and axios: fetch is a native instrument, but axios has a more convenient API and types.

kentcdodds commented 1 year ago

@vasilii-kovalev I disagree. The benefit of using the platform here is compatibility with other tools and knowledge sharing. If the API is "just the platform" then people who know the platform already know the API. If they don't know the platform, they'll learn it as they learn the API which is transferable knowledge for them which is a big win.

As for the namespace object, I don't like it because it is unnecessarily verbose and forces me to first auto-complete the object, and then auto-complete the method I want. If instead these were simple methods exposed by a module within MSW, I could have the option of importing them as a namespace if I wanted to, but I wouldn't have to do that. The fact that people get to choose the name of the namespace import (so there's no enforced consistency) is not a big deal in my mind.

vasilii-kovalev commented 1 year ago

@kentcdodds, I suppose we just have different preferences 🙂 I don't have any problems with libraries/frameworks introducing a new API/concepts, if they solve the problem I have, speed up my work and keep the code's understanding/correctness at an acceptable level (= gets along with TypeScript in this case). React, TypeScript, MSW and other different libraries' API were new for me once, but I used to it.

When talking about that namespace vs. handler function's parameter options, I only care about types connection between different parts of the handler. If it will be possible to add types separately for request and response (if they are not connected somehow under the hood via context, for example), and the response would be imported from the library, it would still work for me 🙂

mattcosta7 commented 1 year ago

As for the namespace object, I don't like it because it is unnecessarily verbose and forces me to first auto-complete the object, and then auto-complete the method I want. If instead these were simple methods exposed by a module within MSW, I could have the option of importing them as a namespace if I wanted to, but I wouldn't have to do that. The fact that people get to choose the name of the namespace import (so there's no enforced consistency) is not a big deal in my mind.

That was kind of the same thinking I was trying to express earlier

I'm not sure we'd need to have the class construct, instead of just the methods themselves?

// 'msw/response' 

function toResponseInit(init: ResponseInit | number): ResponseInit {
  if (typeof responseInit === "number") {
    return { status: responseInit } satisfies ResponseInit
  } 
  return init
}

// I don't think we actually have a better way to type this here, since techically a `jsonable` is anything that implements a `toJSON` method, or anything that's type Scalar = string|null|undefined|number|boolean type JSON = Scalar | Record<string, scalar> | Array<JSON> which is effectively almost anything anway?

export function json(body: any, init: ResponseInit | number = 200): Response {
   const response = new Response(JSON.stringify(body), toResponseInit(init))
   response.headers.set('Content-Type', 'application/json; charset=utf-8')
   return response
}

export function text(body: string|null, init: ResponseInit | number = 200): Response {
   return new Response(body, toResponseInit(init))
}

export function formData(body: FormData|null, init: ResponseInit | number = 200): Response {
   return new Response(body, toResponseInit(init))
}

export function arrayBuffer(body: ArrayBuffer|null, init: ResponseInit | number = 200): Response {
   return new Response(body, toResponseInit(init))
}

export function redirect(url: string, init: ResponseInit | number = 302): Response {
  const responseInit = toResponseInit(init)
   if (!responseInit.status) {
    responseInit.status = 302;
  }
  const response = new Response(null, responseInit);
  response.headers.set("Location", url);
  return response
};

// in mocks/handlers (or wherever)
import {json, formData, arrayBuffer, redirect, text} from 'msw/response'

these also should benefit from a little treeshaking (even though it's very light wrappers) where a single object would not allow that (in addition to the easier editor inference).

I'm not sure what the path to type-safety in practice here is, since response objects aren't generic - although given the mention earlier, for instance, that toJSON is already not going to be typesafe implicitly when used to form json, I think that's a reasonable tradeoff for keeping a consistent, platform based response api?

kettanaito commented 1 year ago

Shipping fewer library-specific abstractions is, generally, a sign of good API design. It means a much leaner learning curve for anyone wishing to interact with the API. That's why I want to migrate to Fetch API primitives internally to the fullest extent.

But we cannot ignore the fact that the platform is often lacking behind. It's the reason why we have tools like Babel, PostCSS, and many others. And API mocking is one of the areas where the web standards offer no solution at all. You're simply not meant to declare responses in your code, only receive and handle their representations (Response instances) that were declared elsewhere (most likely on the server).

In this light, I think the best direction to go is a mix of the two:

But also:

So, to summarise:

On HttpResponse

One thing I need to highlight is that HttpResponse.* methods are fully compatible with Response's constructor signature. So you're not learning any new API at this point. This is a huge win for the user, as they may swap HttpResponse.json() with Response, which would give them a more verbose API but the same input/output.

On HttpResponse import

To give people some context, we've discussed on Twitter the best way to export the HttpResponse object. I've originally proposed this:

import { HttpResponse } from 'msw'

I believe @kentcdodds has suggested exporting individual helper methods of that object from a different module, so you could use wildcard imports and name that end object as you wish:

import * as myName from 'msw/response'

I have a few concerns with this suggestion:

import { rest } from 'msw'
import * as HttpResponse from 'msw/response'

I find this unnecessarily verbose with no particular benefits apart from type suggestions for all the downsides we also get.

Now, on type suggestions. I can't say that's a substantial benefit either. The mental journey when mocking a response is:

I don't expect people to ever think of the words "json" or "xml" when they think of what they want to do in the response resolver. Arguably, standalone json and xml exports make zero sense because they are too vague.

Here's an example of what I'd expect a json function exported by a library may do:

These shorthand names only make sense when accessed through a parent context that adds that sense, like HttpResponse.json(). I also believe that once people try using this API, they will see how trivial it becomes to declare any sort of mocked response. Discussing it hypothetically is one thing but once you try it, I'm certain some of my points will click with you.

kettanaito commented 1 year ago

@mattcosta7, thanks for writing that up! I'm sharing some of my concerns about structuring API around the assumption of wildcard imports above. That's not a good idea, imo.

hese also should benefit from a little treeshaking (

Not really applicable for a devtool, which MSW is.

I'm not sure what the path to type-safety in practice here is

You're right, Fetch API primitives are not type-safe. But we can make HttpResponse to be type-safe by accepting generics in its methods. I will write a more detailed take on the new approach to type-safety with this API once I've used it enough myself.

kettanaito commented 1 year ago

Proposal: replace ctx.fetch() with bypass()

I'm proposing we replace ctx.fetch() with a bypass() function and change how bypassed requests are constructed.

Right now, MSW asks you to use a totally different version of fetch, which is ctx.fetch(), just so the library could append an internal header to tell the worker to completely bypass an intercepted request.

Instead, the new API should be this:

import { bypass } from 'msw'

rest.get('/user', ({ request) => {
  const originalResponse = await fetch(bypass(request))
})
mattcosta7 commented 1 year ago

@mattcosta7, thanks for writing that up! I'm sharing some of my concerns about structuring API around the assumption of wildcard imports above. That's not a good idea, imo.

these also should benefit from a little treeshaking (

Not really applicable for a devtool, which MSW is.

Maybe not really necessary for a dev tool, and these are all generally small, but fewer bytes of code shipped to the browser does shave little bits of time here/there for things like playwright tests (where the scripts are fetched/parsed/evaled for every test)

I'm not sure I see the value in adding a lot of extra text to namespace them all in an object, but I don't feel strongly on that, so happy to try it and see how it feels too

I'm not sure what the path to type-safety in practice here is

You're right, Fetch API primitives are not type-safe. But we can make HttpResponse to be type-safe by accepting generics in its methods. I will write a more detailed take on the new approach to type-safety with this API once I've used it enough myself.

Of course we can always do something like

export const HttpResponse = {
    json<T>(body: T, init: ResponseInit | number = 200): Response {
      const response = new Response(JSON.stringify(body), toResponseInit(init))
      response.headers.set('Content-Type', 'application/json; charset=utf-8')
      return response
   },
   ...otherMethods
}

Where json can accept a generic argument, but inferring that from the route handler wouldn't be likely, unless this was an argument to the callback somewhere? Not necessarily suggesting this as the api, but mentioning it for discussion around passing type safety from the method generics to a json call

rest.get<
   RequestBody, 
   PathParams, 
   ResponseType
>(url, (request, HttpResponse) => { 
   const body = {} satisfies ResponseType
   return HttpResponse.json<ResponseType>(body) 
})

One thing tough with typings here are that a Response is always the return so typesafety might be possible for the call to create the response but returning Response objects themselves won't necessarily have that same safety. I think that's ok, but might lead to some friction there?

Importing HttpResponse directly like import {HttpResponse} from 'msw' would likely make the following code a bit less safe, since the generic parameter is not easily comparable to the ReponseType since (a very trivial set of types for example)

const rest = {
   get(
       url: string, 
       // keeping the resolve
       responseResolver: (
          // maybe arguments here look more like request/params. maybe not.  the resolver function and context objects propably make less sense here in that case? (also outside the scope of this change for now, but just for discussion)
           request: Request, 
           // maybe determinable from the url string directly with path parameter extraction types like https://github.com/ghoullier/awesome-template-literal-types#router-params-parsing
           params
       ) => Response) {...}
}

rest.get<
   // with http requests, json can be munged, by providing a custom Request instead of a fetch native request.  there's no guarantee the request actually gets _made_ with that type, so maybe better to _not_ type it directly and rely on 
   RequestBody,  
   // json could be handled like below, but so would incorrect types
   ResponseType
>(url, (request) => { 
   const body = {} satisfies ResponseType
  const res = HttpResponse.json<ResponseType>(body) 

   // works
   return res

   return new Response(null) // also works (assuming the previous return isn't there
})

Since the return here is actually a Response object, it's very possible to return an invalid response object (using a native browser response).

Overriding types for Response here could be doable Response<Body> but then we're away from the native types here, and now the 'msw' Response becomes the only type-checkable response here?

kettanaito commented 1 year ago

Since the return here is actually a Response object, it's very possible to return an invalid response object (using a native browser response).

Yep. And that's why we will generally recommend using HttpResponse for that, and a few other reasons. I recall some TypeScript magic of function generics inferring from the parental context (the way we validate ctx.json<T> right now, which gets inferred from the handler itself (rest.get<T>). We can do that same for HttpResponse.json<T>() and then either return an augmented Response<T> instance to have type validation or, even better, see if we can rely on generics compatibility in some way to now alter the output type.

kentcdodds commented 1 year ago

I yield on the namespace thing. It's not that important to me.

I stand very firm on the web standards thing though. Please do not use anything other than web standard Request/Response, otherwise the entire goal of the original issue is left unsatisfied for reasons I've already explained.

Also, I'm a fan of bypass 👍

kettanaito commented 1 year ago

@kentcdodds, agree. Yes, as I've said, with this change we will fully support Fetch API primitives: the request you get will be a Request instance, and the response you return from the resolver should be a plain Response instance. That being said, we will introduce custom, spec-compatible abstractions to declare responses easier and support real-world practices. Think of them as the json() utility that Remix exposes, only spec-compliant 😉

kentcdodds commented 1 year ago

Perfect ❤️

kettanaito commented 1 year ago

Looks like the Request class from @remix-run/web-fetch disregards any custom credentials value from request init. Opened a fix at https://github.com/remix-run/web-std-io/pull/21.

dbritto-dev commented 1 year ago

Shipping fewer library-specific abstractions is, generally, a sign of good API design. It means a much leaner learning curve for anyone wishing to interact with the API. That's why I want to migrate to Fetch API primitives internally to the fullest extent.

But we cannot ignore the fact that the platform is often lacking behind. It's the reason why we have tools like Babel, PostCSS, and many others. And API mocking is one of the areas where the web standards offer no solution at all. You're simply not meant to declare responses in your code, only receive and handle their representations (Response instances) that were declared elsewhere (most likely on the server).

In this light, I think the best direction to go is a mix of the two:

  • Fully respect Fetch API Request and Response internally. This also makes our internals much lighter and easier to work with.
  • Expose intercepted request as Fetch API Request so its API is familiar to our users.
  • Expose additional info like params and cookies alongside the request but never backed in its object.
  • Support response resolvers returning plain Fetch API Response instances. This is currently not possible as res() abstracts the Response declaration away from the consumer, making it feel like magic.

But also:

  • Expose abstractions for type-safe mocking. Neither Request nor Response is type-safe, just as almost all web standard APIs (MessageChannel, EventTarget, EventEmitter, etc). Compromising the resilience of the end-user code in favor of just respecting the standard would be hurting the users.
  • Expose abstractions to declare a mocked response. I've written at length why Response was never meant to declare responses. This is the gap that something like HttpResponse should fill.

So, to summarise:

  • You always operate with Fetch API primitives;
  • You can return Fetch API Response straight from the resolver;
  • But you can also use custom API that abstracts some of the verbosity and the lack of support from you, like HttpResponse.

On HttpResponse

One thing I need to highlight is that HttpResponse.* methods are fully compatible with Response's constructor signature. So you're not learning any new API at this point. This is a huge win for the user, as they may swap HttpResponse.json() with Response, which would give them a more verbose API but the same input/output.

On HttpResponse import

To give people some context, we've discussed on Twitter the best way to export the HttpResponse object. I've originally proposed this:

import { HttpResponse } from 'msw'
  • You keep a single msw import when intercepting (rest/graphql) and mocking.

I believe @kentcdodds has suggested exporting individual helper methods of that object from a different module, so you could use wildcard imports and name that end object as you wish:

import * as myName from 'msw/response'

I have a few concerns with this suggestion:

  • No standardized and, thus, no predictable way to tell people how to name this import. If everyone will be calling this object as they wish, it'd get harder to debug reported scenarios, it'd be harder for users to read the docs, and, in general, it feels wrong to push the responsibility of collocating utilities to the consumer.
  • People will be using HttpResponse 99% of the time. This is what we are going to recommend in the docs and in any related tutorials. Just as I've said above: we respect the standards but we're not going to let people pay the price of those standards lacking behind. This means that everyone will have two imports all the time:
import { rest } from 'msw'
import * as HttpResponse from 'msw/response'

I find this unnecessarily verbose with no particular benefits apart from type suggestions for all the downsides we also get.

Now, on type suggestions. I can't say that's a substantial benefit either. The mental journey when mocking a response is:

  • What do I want? To mock a response.
  • How do I want to mock it? As a JSON.

I don't expect people to ever think of the words "json" or "xml" when they think of what they want to do in the response resolver. Arguably, standalone json and xml exports make zero sense because they are too vague.

Here's an example of what I'd expect a json function exported by a library may do:

  • Parse a string into a JSON (ala JSON.parse()).
  • Stringify a given object to a string (ala JSON.stringify()).
  • Do literally anything JSON-related. Maybe form a response.

These shorthand names only make sense when accessed through a parent context that adds that sense, like HttpResponse.json(). I also believe that once people try using this API, they will see how trivial it becomes to declare any sort of mocked response. Discussing it hypothetically is one thing but once you try it, I'm certain some of my points will click with you.

Actually importing from 'msw/response' it's the context that you are talking about so there's no need to have an object. I'm thinking in these functions/methods as just utilities that you can use in order to get a Response instance but totally up to you to use it. So you can use either a Response instance directly or one of these utilities.

kettanaito commented 1 year ago

At this point it's decided that HttpResponse, among some other utilities, will be exported from the root of msw. You will be using those all the time, and we need to account for the developer experience when it comes to managing imports.

So you can use either a Response instance directly or one of these utilities.

Technically, you can. We will support this and you can use global Response anytime you want. But in practice, if you wish to write isomorphic handlers (a fancy term I will adapt that means "handlers that can run in both browser and Node"), you will be adviced to use either a Response mirror or HttpResponse:

import { rest, Response, FormData } from 'msw'

// This handler can be executed in both browser and Node.js
// due to the mirror exports of "Response" and "FormData".
rest.get('/user', () => {
  const data = new FormData()
  data.set('name', 'Alice')

  return new Response(data)
})
dbritto-dev commented 1 year ago

At this point it's decided that HttpResponse, among some other utilities, will be exported from the root of msw. You will be using those all the time, and we need to account for the developer experience when it comes to managing imports.

So you can use either a Response instance directly or one of these utilities.

Technically, you can. We will support this and you can use global Response anytime you want. But in practice, if you wish to write isomorphic handlers (a fancy term I will adapt that means "handlers that can run in both browser and Node"), you will be advices to use either a Response mirror or HttpResponse:

import { rest, Response, FormData } from 'msw'

// This handler can be executed in both browser and Node.js
// due to the mirror exports of "Response" and "FormData".
rest.get('/user', () => {
  const data = new FormData()
  data.set('name', 'Alice')

  return new Response(data)
})

Amazing, let me know if I can help with something even if it's just docs. I would like to help with this.

kettanaito commented 1 year ago

@dbritto-dev, I know I could certainly use your help! Here are a couple of ways you can help me make this change a reality faster:

dbritto-dev commented 1 year ago

@kettanaito cool, I'll work on that

cdimitroulas commented 1 year ago

Edit: Deleted my comment as I see it's already been discussed here

kettanaito commented 10 months ago

Released: v2.0.0 🎉

This has been released in v2.0.0!

Make sure to always update to the latest version (npm i msw@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.