PR closed due to unresolved review loop #8021

nitin-is-me · 2025-07-25T02:42:16Z

Description

This PR improves the fetch.md documentation by fixing and clarifying key code examples related to streaming, async usage, and response handling. These are the changes I made:

Correctly buffers and parses the full streaming response in the Undici example, avoiding parsing incomplete chunks.
Adds proper try/catch handling in the stream’s final block to handle JSON parse errors.
Adds an inline explanation of the TextDecoder stream option to reduce confusion.

Related Issues

N/A

Check List

I have read the Contributing Guidelines and made commit messages that follow the guideline.
I have run pnpm format to ensure the code follows the style guide.
I have run pnpm test to check if all tests are passing. (N/A)
I have run pnpm build to check if the website builds without errors. (N/A)
I've covered new added functionality with unit tests if necessary. (N/A)

vercel · 2025-07-25T02:42:23Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
nodejs-org	✅ Ready (Inspect)	Visit Preview	Jul 26, 2025 0:40am

avivkeller · 2025-07-25T10:22:13Z

I don't see how this is an improvement to our previous page

nitin-is-me · 2025-07-25T11:12:57Z

@avivkeller Let me explain why these changes are improvements. Each commit addresses a concrete issue in the examples or improves clarity where it previously led to confusion:

fix(docs): correct response structure comment in GET example:

Original comment incorrectly showed a single object as the response, while the actual response from the API is an array of objects.
I updated the comment to wrap the object in square brackets to reflect the actual return structure:
```
// [
//   { userId: 1, id: 1, ... }
// ]
```
fix(docs): explain top-level await usage and fallback in fetch example:

The example uses top-level await, but without context, this can cause a SyntaxError for users in CommonJS environments.
I clarified that top-level await requires ES modules (.mjs or "type": "module") and also added a fallback using an async IIFE:
```
(async () => {
await ...
})();
```
fix(docs): handle streaming JSON parsing properly in fetch example:

The original stream handling attempted to parse chunks directly or didn’t account for chunk boundaries, so
I introduced buffering and moved JSON.parse into the .final() block of the Writable stream, where the full body is guaranteed to be available:
```
final(callback) {
const json = JSON.parse(buffer);
}
```
fix(docs): add comment explaining TextDecoder stream option

The line used decoder.decode(chunk, { stream: true }) without explanation which could be confusing to devs or readers, so I added a short but critical comment clarifying that the stream: true option may buffer incomplete UTF-8 sequences, which affects chunk-wise console logging or string building:
```
// Note: decoder.decode() with { stream: true } may buffer incomplete UTF-8 chunks.
```
This helps readers understand why a chunk might appear missing or cut off when logged directly.

Each of these changes is small but meaningful, aimed at making the examples clearer, more accurate, and more beginner friendly. Happy to revise anything further if needed! :D

AugustinMauroy · 2025-07-25T11:37:47Z

fix(docs): explain top-level await usage and fallback in fetch example:

You can use code tabs for CJS/ESM

nitin-is-me · 2025-07-25T11:50:07Z

You can use code tabs for CJS/ESM

Am I mistaken, or there are no code tabs to switch between CJS and ESM in the fetch documentation?:

This is the code I'm changing, the default one will give SyntaxError:

nitin-is-me · 2025-07-25T12:52:39Z

Hello everyone! I'm going to update the fetch.md file so that it contains tabs for both CJS and ESM, keeping the rest changes as it is. This will not take much!

nitin-is-me · 2025-07-25T13:44:45Z

Update: Added missing dual tab support (CommonJS and ES Modules) for the streaming examples in fetch documentation.

AugustinMauroy

LGMT !

avivkeller

This is still doing several things. For example, If we want to add CJS/ESM interop, it should be it's own PR.

avivkeller · 2025-07-25T22:17:25Z

Let's do the fixes in one PR, and the addition/features in another

nitin-is-me · 2025-07-26T00:13:00Z

@avivkeller Acknowledged, working on it!

…voiding parsing incomplete chunks mid-stream

Signed-off-by: Nitin <nitinjha2609@gmail.com>

nitin-is-me · 2025-07-26T00:42:23Z

Tab changes were reverted, it will be submitted separately in another PR. This one now only includes small fixes and clarification changes to the fetch example. Changes were made on top of the latest main branch.

nitin-is-me · 2025-07-26T01:24:07Z

Tab additions have been separated out into a new PR (#8035) to keep changes focused. That PR builds on top of the updates made here. In case any conflicts occur, then I'm happy to rebase the branch of that PR (fetch-tabs) on top of main once this PR is merged, tag me anytime!

nitin-is-me · 2025-07-29T00:57:49Z

Hey @avivkeller @AugustinMauroy 👋
Just wanted to bump this PR, and #8035 (Separated for adding feature) in case it slipped under the radar. Let me know if any changes are needed. Appreciate your time!

avivkeller · 2025-07-29T01:48:28Z

apps/site/pages/en/learn/getting-started/fetch.md

@@ -181,4 +181,4 @@ async function fetchGitHubRepos() {
 }

 fetchGitHubRepos().catch(console.error);
-```
+```


Suggested change

```

```

avivkeller · 2025-07-29T01:49:09Z

apps/site/pages/en/learn/getting-started/fetch.md

          } catch (error) {
            console.error('Error parsing JSON:', error);
+          } finally {
+            console.log('Stream processing completed.');
+            callback();
          }
-
-          callback();
-        },
-        final(callback) {
-          console.log('Stream processing completed.');
-          callback();


The point of this demo is to showcase the callback, let's not change it to a try final

agree with aviv here - sometimes we choose to show code concepts in isolation even if a production app would have far more rigor

avivkeller · 2025-07-29T01:49:32Z

apps/site/pages/en/learn/getting-started/fetch.md

@@ -155,23 +157,21 @@ async function fetchGitHubRepos() {
      return new Writable({
        write(chunk, encoding, callback) {
          buffer += chunk.toString();
-
+          callback();


Why was this added?

avivkeller · 2025-07-29T01:50:03Z

apps/site/pages/en/learn/getting-started/fetch.md

    partial += decoder.decode(chunk, { stream: true });
+    // Note: decoder.decode() with { stream: true } may buffer incomplete UTF-8 chunks.


Is this relevant to the code snippet?

comments go above the block

nitin-is-me · 2025-07-29T02:20:50Z

@avivkeller Alright, I’ve already explained everything above and responded to all the points, but it’s starting to feel it’s just going in circles or not really being read. So I’m gonna close this PR, along with the other one that depended on it.
No hard feelings, just don’t wanna keep chasing something that’s clearly not moving forward.

bmuenzenmeyer · 2025-07-29T03:32:45Z

i hope you find ways to continue contributing, I felt as if review feedback was responsive here.

nitin-is-me requested a review from a team as a code owner July 25, 2025 02:42

vercel bot deployed to Preview July 25, 2025 02:43 View deployment

vercel bot deployed to Preview July 25, 2025 03:11 View deployment

vercel bot deployed to Preview July 25, 2025 13:38 View deployment

nitin-is-me changed the title ~~fix(docs): improve fetch examples with accurate usage, error handling, and streaming clarity~~ fix(docs): improve fetch examples with proper streaming, error handling, and dual CJS/ESM usage Jul 25, 2025

AugustinMauroy approved these changes Jul 25, 2025

View reviewed changes

avivkeller requested changes Jul 25, 2025

View reviewed changes

nitin-is-me added 7 commits July 26, 2025 05:53

fix(docs): correct response structure comment in GET example

56d4079

fix(docs): explain top-level await usage and fallback in fetch example

1144797

fix(docs): handle streaming JSON parsing properly in fetch example, a…

e85f008

…voiding parsing incomplete chunks mid-stream

fix(docs): add comment explaining TextDecoder stream option

09bc47f

chore: fixed bracket and spacing in comment example

bb28953

Signed-off-by: Nitin <nitinjha2609@gmail.com>

fix(docs): include dual cjs/esm tabs in fetch streaming examples

d5354de

fix(docs): revert unintended tab changes for separate PR

3ec643c

nitin-is-me force-pushed the fetch-fix branch from 0f553c0 to 3ec643c Compare July 26, 2025 00:38

vercel bot deployed to Preview July 26, 2025 00:40 View deployment

nitin-is-me changed the title ~~fix(docs): improve fetch examples with proper streaming, error handling, and dual CJS/ESM usage~~ fix(docs): improve fetch examples with proper streaming and error handling Jul 26, 2025

nitin-is-me mentioned this pull request Jul 26, 2025

feat(docs): add dual ESM/CJS tabs to fetch documentation #8035

Closed

5 tasks

avivkeller reviewed Jul 29, 2025

View reviewed changes

nitin-is-me closed this Jul 29, 2025

nitin-is-me deleted the fetch-fix branch July 29, 2025 02:21

nitin-is-me changed the title ~~fix(docs): improve fetch examples with proper streaming and error handling~~ PR closed due to unresolved review loop Jul 29, 2025

		partial += decoder.decode(chunk, { stream: true });
		// Note: decoder.decode() with { stream: true } may buffer incomplete UTF-8 chunks.

Uh oh!

PR closed due to unresolved review loop #8021

PR closed due to unresolved review loop #8021

Uh oh!

Conversation

nitin-is-me commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

vercel bot commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

avivkeller commented Jul 25, 2025

Uh oh!

nitin-is-me commented Jul 25, 2025

Uh oh!

AugustinMauroy commented Jul 25, 2025

Uh oh!

nitin-is-me commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nitin-is-me commented Jul 25, 2025

Uh oh!

nitin-is-me commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AugustinMauroy left a comment

Choose a reason for hiding this comment

Uh oh!

avivkeller left a comment

Choose a reason for hiding this comment

Uh oh!

avivkeller commented Jul 25, 2025

Uh oh!

nitin-is-me commented Jul 26, 2025

Uh oh!

nitin-is-me commented Jul 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nitin-is-me commented Jul 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nitin-is-me commented Jul 29, 2025

Uh oh!

avivkeller Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

avivkeller Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

bmuenzenmeyer Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

avivkeller Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

avivkeller Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

nitin-is-me commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bmuenzenmeyer commented Jul 29, 2025

Uh oh!

Uh oh!

nitin-is-me commented Jul 25, 2025 •

edited

Loading

vercel bot commented Jul 25, 2025 •

edited

Loading

nitin-is-me commented Jul 25, 2025 •

edited

Loading

nitin-is-me commented Jul 25, 2025 •

edited

Loading

nitin-is-me commented Jul 26, 2025 •

edited

Loading

nitin-is-me commented Jul 26, 2025 •

edited

Loading

nitin-is-me commented Jul 29, 2025 •

edited

Loading