Skip to content

Conversation

@Rich-Harris
Copy link
Member

I don't know if this fixes #591 because I still don't have a repro. Probably not. But I thought it might be worth a try

@Rich-Harris
Copy link
Member Author

In fact, re-reading some of the linked issues i'm fairly certain this won't change anything. Given that we don't have any control over how the original response is consumed I'm not sure what we can really do here. Maybe instead of cloning the response, we return a proxy that lets us intercept the buffered value? This would also have the nice consequence that we'd know whether it was being consumed as text/json or as an arrayBuffer (which affects how/if we serialize it)

@benmccann
Copy link
Member

It took me quite awhile to wrap my head around the problem (probably a couple hours to be honest). I did a write up over on the issue: #591 (comment)

I agree that I don't think this PR will help unfortunately. I don't think a proxy would work either because it's fundamentally the same as what we're doing now. In either case we'd be trying to read the stream into a buffer. If we reach the limit of that buffer then we're out of luck, so the approach is fundamentally incompatible with streaming. I believe the only way to do make it work is to process the response for rendering and serialize for hydration in parallel.

@stalkerg
Copy link
Contributor

serialize for hydration in parallel

It's not "parallel" but yes, if we will wait for our await result.json() and event loop switch execution to working with await clone.text() it will be solved but we lose stream anyway.
In that case, much easier to do node-fetch/node-fetch#533 (comment)

@Rich-Harris
Copy link
Member Author

closing in favour of #635

@Conduitry Conduitry deleted the gh-591 branch April 1, 2021 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cloning of response leads to issues

4 participants