Skip to content

Conversation

@o-t-w
Copy link
Contributor

@o-t-w o-t-w commented Sep 15, 2021

Adding a simple usage example of Fetch and a very brief explanation of Node Streams vs Web Streams.

#940

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

o-t-w and others added 2 commits September 15, 2021 17:11
fix code formatting

Co-authored-by: Matteo Collina <[email protected]>
fix code indentation

Co-authored-by: Matteo Collina <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Sep 15, 2021

Codecov Report

Merging #1035 (d5b843e) into main (1e1a6db) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1035   +/-   ##
=======================================
  Coverage   94.89%   94.89%           
=======================================
  Files          37       37           
  Lines        3623     3623           
=======================================
  Hits         3438     3438           
  Misses        185      185           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e1a6db...d5b843e. Read the comment docs.

README.md Outdated
}
```

If you require a Node stream, it is possible to convert between the two using `.fromWeb()`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make that a link?

Copy link
Member

@ronag ronag Sep 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's actually .fromWeb() and .toWeb() if you are referring to converting between the two, since that implies both ways. Otherwise please formulate it in a way where it's explicit one way.

README.md Outdated

Basic usage example:

```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, add js after triple ` when you open code blocks to enable syntax highlighting in examples. This will improve readability.

README.md Outdated

Nodejs has two kinds of streams: [web streams](https://nodejs.org/dist/latest-v16.x/docs/api/webstreams.html) which follow the API of the WHATWG web standard found in browsers, and an older Node-specific [streams API](https://nodejs.org/api/stream.html). `response.body` returns a readable web stream. If you would prefer to work with a Node stream you can convert a web stream using `.fromWeb()`.

```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for this example - add js, please.

Add syntax highlighting to codeblock
@ronag ronag merged commit c7cfb0a into nodejs:main Sep 22, 2021
KhafraDev pushed a commit to KhafraDev/undici that referenced this pull request Jun 23, 2022
* docs: add basic usage example of fetch

* remove promise example

* Update README.md

fix code formatting

Co-authored-by: Matteo Collina <[email protected]>

* Update README.md

fix code indentation

Co-authored-by: Matteo Collina <[email protected]>

* make fromWeb clearer

* improve language about fromWeb

* Update README.md

Add syntax highlighting to codeblock

Co-authored-by: Matteo Collina <[email protected]>
crysmags pushed a commit to crysmags/undici that referenced this pull request Feb 27, 2024
* docs: add basic usage example of fetch

* remove promise example

* Update README.md

fix code formatting

Co-authored-by: Matteo Collina <[email protected]>

* Update README.md

fix code indentation

Co-authored-by: Matteo Collina <[email protected]>

* make fromWeb clearer

* improve language about fromWeb

* Update README.md

Add syntax highlighting to codeblock

Co-authored-by: Matteo Collina <[email protected]>
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.

5 participants