Skip to content

Conversation

@nikdo
Copy link

@nikdo nikdo commented Jan 6, 2019

Fixes #386.

I've spent enormous time investigating the conditions when backpressure occurs, yet I still have a lot of unanswered questions. Although I hate uncertainties, the formula I introduced is a verified hypothesis. I'm well prepared and open to any discussions about this topic. :)

I would appreciate any feedback on the code, especially tests.

@bitinn
Copy link
Collaborator

bitinn commented Jan 8, 2019 via email

@bitinn
Copy link
Collaborator

bitinn commented Jan 16, 2019

Hey @nikdo, I looked into it and while your documentation is very good. I am thinking about 2 things:

  • Personally, I don't see a strong reason to expose clone(highWaterMark) option. The reason being, it's not spec-compliant, hence not isomorphic. It seems like people facing this clone problem are using node-fetch in an isomorphic sense, which is why they run into highWaterMark issue, because they expect nodejs to have the same limit as browsers.

  • I would prefer 2 other solutions:

    (a) Can we pass such value via Fetch option, which means at least the browsers can ignore it and not cause a problem?

    (b) Better yet, can we simply provide a helper library that takes a node-fetch response and clone them in a way that allow for large response? And still provide the same API like text() and json()? Basically a wrapper that's isomorphic.

@gutenye
Copy link

gutenye commented Jul 10, 2019

What's the status of this PR? What can I do to help advance it? It's a pretty serious problem considering a lot of users using node-fetch in an isomorphic sense. Yet no fix in almost 3 years.

@xxczaki
Copy link
Member

xxczaki commented Sep 9, 2019

@gutenye We are currently in process of preparing the v3 release of node-fetch and this issue is in our roadmap.

I've just submitted a pull request, that fixes this problem, but in a preferred way 😄

@nikdo
Copy link
Author

nikdo commented Sep 10, 2019

Let's close this then.

@nikdo nikdo closed this Sep 10, 2019
@nikdo nikdo deleted the 386-clone-with-highwatermark branch September 10, 2019 11: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.

When users clone(), automatically create streams with custom highWaterMark

4 participants