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

Support ReadableStream as a mocked response body #1336

Closed
1 task done
kettanaito opened this issue Jul 16, 2022 · 13 comments · Fixed by #1436
Closed
1 task done

Support ReadableStream as a mocked response body #1336

kettanaito opened this issue Jul 16, 2022 · 13 comments · Fixed by #1436
Assignees
Labels

Comments

@kettanaito
Copy link
Member

kettanaito commented Jul 16, 2022

Scope

Adds a new behavior

Compatibility

  • This is a breaking change

Feature description

Context

Proposal

It'd be great to support ReadableStream as a mocked response body.

Challenges

  • No ReadableStream in Node 14-17. Node 17+ has utils like .toWeb() but I have no idea if they return a compatible API.
  • Readable streams cannot be cloned using postMessage() API (client-worker channel). They can be re-constructed but that leads to its own issues.
  • ReadbleStream can't be easily polyfilled (I've tried). Its methods like pipeTo and tea operate on WritableStream as input, so this requires that to be polyfilled as well. Suddenly, you're polyfilling half of the browser.

API

The main challenge for the API is to allow developers to construct readable streams the same way for browsers and Node. This comes from the principle that the same set of handlers can be used across environments, so handlers mustn't rely on environment-specific API like window.ReadableStream.

Here's an example of a ctx.stream() API that uses window.ReadableStream browser API:

rest.get('/resource', (req, res, ctx) => {
  const responseStream = new ReadableStream({
    start(controller) {
      controller.enqueue('hello')
      controller.enqueue('world')
      controller.close()
    },
  })

  return res(ctx.stream(responseStream))
})

This will throw in Node saying ReadableStream is undefined.

Resources

@kettanaito
Copy link
Member Author

kettanaito commented Jul 16, 2022

Trying to find a common ground API for readable streams.

stream.Readable.toWeb()

A converter function that creates a web-compatible readable stream from a Readable instance.

  • Added in 17.0
  • Still experimental as of 18.6.0

stream/web#ReadableStream

A polyfill of window.ReadableStream.

  • Added in 16.5.0
  • Looks stable

Potential usage

// ReadableStream.ts
export const ReadableStream = 
  typeof window === 'undefined'
    ? require('stream/web').ReadableStream
    : window.ReadableStream
// handlers.ts
import { ReadableStream } from './ReadableStream'

The functionality of window.ReadableStream will depend on the DOM-like environments (i.e. JSDOM). Some may not have it at all. As with fetch, you'd have to include a polyfill manually if you're using readable streams in such environments.

@kettanaito
Copy link
Member Author

readable-stream

There's a readable-stream package published to NPM by the Node maintainers group. Its version 4 is cut out from Node 18 and comes with methods like toWeb() that produce web-compatible polyfilled streams. We need to give this a try.

@kettanaito
Copy link
Member Author

Looks like we should be able to construct a readable stream in the handlers with the help of readable-stream package.

The remaining challenge is how to transfer those streams to the worker. With #1288, I've tried constructing a stream on the client and on the worker, and manually piping it via BroadcastChannel (so that the worker associates incoming stream chunks with a proper request). This does look like overkill, having to construct two streams.

Chrome supports sending ReadableStream as transport on postMessage() and it works great. That's not the case for any other browser, however, as their support is still lacking. Without the ability to transfer the stream directly, I'm not sure what options we've got left.

@piotr-cz
Copy link
Contributor

piotr-cz commented Feb 16, 2023

Implementing ReadableStream in a response context could possibly fix the issue with closing Server-sent Events connection, which is described here: #156 (comment)

By the way, at this moment the signature for ctx.body permits ReadableStream:

msw/src/context/body.ts

Lines 10 to 12 in 2633fa9

export const body = <
BodyType extends string | Blob | BufferSource | ReadableStream | FormData,
>(

However using it logs an error with link to this issue.

@jbcpollak
Copy link

This would be a great addition for SSE, this is a constant problem for mocking our frontend and if we could do this it would be much cleaner.

@piotr-cz
Copy link
Contributor

Looks like the only browser that didn't implement Transferable objects yet is Safari.. not surprising

@kettanaito
Copy link
Member Author

Readable steams are already working in the Fetch API branch. Glad to hear it may resolve other issues as well.

@kettanaito kettanaito self-assigned this Feb 16, 2023
@piotr-cz
Copy link
Contributor

For reference, support Fetch API specification is here: #1436

@piotr-cz
Copy link
Contributor

piotr-cz commented Feb 17, 2023

I just tested the v0.0.0-fetch.rc-3 and the ReadableStream works fine, at least for my Sever-sent Events implementation

which is based on mock test and is a bit different than example in this issue description: https://github.com/mswjs/msw/pull/1436/files#diff-cc6c833284918f1602b3c897fb29381fd3803581ae18da15e795e8612360bef9
or than Migration guide > Support ReadableStream mocked responses

Also I guess this is no longer required:

// Bypass server-sent events.
if (accept.includes('text/event-stream')) {
return
}

@kettanaito Thank You for pointing me into right direction.

@kettanaito
Copy link
Member Author

@piotr-cz, I'm glad to hear that. There's a lot more to Fetch API support that it seems. Readable streams support is a fantastic addition to MSW capabilities. I'm working hard on making that release happen sometime this year. As with many things, this also takes longer than expected due to tech debt.

@Yurii-Yanovitsky
Copy link

Yurii-Yanovitsky commented Apr 26, 2023

Hello everyone. Thanks for the Readable streams support. As for now I understand that you pass it as ArrayBuffer to the worker. Novertheless I've encounter some problem on stream cancellation support. Dou you know any workarounds how we can determine from the worker side that the client app closed/cancelled the stream to be able to release access to the stream source? For example, if I have setInterval working to provide stream data and I want to clear it when the client signals that the stream is to be closed.

@kettanaito
Copy link
Member Author

Hi, @Yurii-Yanovitsky.

The current implementation converts the mock response into ArrayBuffer and sends that to the worker thread in a single message. If I understand correctly, your scenario is:

  1. Mock a response with a ReadableStream.
  2. Start streaming (MSW sent the message to the worker).
  3. Cancel the mock readable stream mid-response.
  4. Actual behavior: the stream is still locked because it's being used?

I don't think I see how that is possible, given that .arrayBuffer() will read the entire stream at once and send it to the worker. Maybe the issue is in the client-side reading and has nothing to do with the worker?

In any case, the current behavior is largely irrelevant because once #1404 lands, the library will be using actual streams between the client and the worker (see this). In that scenario, if you cancel the mocked readable stream mid-response, it should free the resources correctly, since that's going to also cancel the stream that's being sent to the worker.

@kettanaito
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants