fs: make FileHandle.readableWebStream always create byte streams

[isker]

The original implementation of the experimental
FileHandle.readableWebStream API created non-type: 'bytes' streams, which prevented callers from creating mode: 'byob' readers from the returned stream, which means they could not achieve the associated "zero-copy" performance characteristics.

Then, #46933 added a parameter allowing callers to pass the type parameter down to the ReadableStream constructor, exposing the same semantics to callers of FileHandle.readableWebStream.

But there is no point to giving callers this choice: FileHandle-derived streams are by their very nature byte streams. We should not require callers to explicitly opt in to having byte stream semantics. Moreover, I do not see a situation in which callers would ever want to have a non-bytes stream: bytes-streams only do anything differently than normal ones if mode: 'byob' is passed to getReader.

So, remove the options parameter and always create a ReadableStream with type: 'bytes'.

Fixes #54041.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.41%. Comparing base (5e5af29) to head (41c448c).
Report is 800 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #55461      +/-   ##
==========================================
- Coverage   88.41%   88.41%   -0.01%     
==========================================
  Files         652      654       +2     
  Lines      186918   187855     +937     
  Branches    36077    36145      +68     
==========================================
+ Hits       165271   166095     +824     
- Misses      14901    15000      +99     
- Partials     6746     6760      +14     

Files with missing lines	Coverage Δ
lib/internal/fs/promises.js	98.23% <100.00%> (-0.01%)	⬇️

... and 179 files with indirect coverage changes

[avivkeller]

Sorry again for the butchering of my last comment.

Here's my idea to gracefully make users aware of this change. We start by warning that this option has no effect, and then in the next semver, we remove the warning entirely.

[avivkeller]

Assuming this lands in 23.X.0, we can remove the warning in 23.(X + 1).0

Although, because this is experimental, a warning isn't technically required, so you have the final say (IMO) whether there is one to add

[isker]

@redyetidev can you take another look?

[isker]

@nodejs/fs please take a look. Thanks!

[isker]

Maybe someone from @nodejs/streams could review?

[isker]

Thanks @jasnell. @avivkeller stated on slack that they don't think they're qualified to approve but I don't know how to withdraw the review request here. Besides that, I think there is nothing else here blocking a merge.

[isker]

Thanks!

[isker]

Hi @jasnell, is there anything else I need to do to land this PR?

[mcollina]

lgtm

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/64669/ 💛

[isker]

Hi @mcollina or @jasnell, could you take another look at landing this? I'm not sure whether the orange stuff in Jenkins means CI failed or not.

[nodejs-github-bot]

Landed in 1781f63

[isker]

Thanks @jasnell!

[karlhorky]

@isker @jasnell @mcollina @avivkeller Thanks for the PR, reviews and merge!

Excited to see this released - seems it's planned in Node.js 23.8.0:

• 2025-02-13, Version 23.8.0 (Current) #57005

Will this also be backported to v22?

[isker]

I don't have any plans to attempt a backport myself.

[karlhorky]

Now that this PR has been released in Node.js v23.8.0:

I can confirm that Node.js v23.8.0 FileHandle.readableWebStream() can be used without configuration to easily create Web Streams 🎉

import fs from "node:fs/promises";
import http from "node:http";
import path from "node:path";
import { Readable } from "node:stream";

const filePath = "./image.jpg";

const server = http.createServer(async (nodeRequest, nodeResponse) => {
  const stats = await fs.stat(filePath);
  const fileHandle = await fs.open(filePath);
  const webStream = fileHandle.readableWebStream();

  nodeResponse.writeHead(200, {
    "Content-Type": "image/jpeg",
    "Content-Length": String(stats.size),
  });

  const nodeStream = Readable.fromWeb(webStream);

  nodeStream.pipe(nodeResponse).on("finish", async () => {
    await fileHandle.close();
  });
});

const port = 3000;
server.listen(port, () => {
  console.log(`Server running at http://localhost:${port}/`);
});

CodeSandbox demo: https://codesandbox.io/p/devbox/filehandle-readablewebstream-with-new-response-forked-rly47y?file=%2Findex.js%3A9%2C35&workspaceId=ws_GfAuHrswXyA1DoeSwsjjjz