Skip to content

fix(node/fs): add a createReadStream method to the FileHandle class#27591

Closed
nkaradzhov wants to merge 3 commits intodenoland:mainfrom
nkaradzhov:feat_createreadstream_to_filehandle
Closed

fix(node/fs): add a createReadStream method to the FileHandle class#27591
nkaradzhov wants to merge 3 commits intodenoland:mainfrom
nkaradzhov:feat_createreadstream_to_filehandle

Conversation

@nkaradzhov
Copy link
Copy Markdown
Contributor

Add the createReadStream method to the FileHandle class in node compat as part of #25554

@dsherret dsherret changed the title feat(node/fs): Add a createReadStream method to the FileHandle class feat(node/fs): add a createReadStream method to the FileHandle class Jan 8, 2025
@dsherret dsherret requested a review from kt3k January 8, 2025 21:43
@kt3k kt3k changed the title feat(node/fs): add a createReadStream method to the FileHandle class fix(node/fs): add a createReadStream method to the FileHandle class Jan 14, 2025
@kt3k kt3k mentioned this pull request Jan 14, 2025
24 tasks
createReadStream(options?: CreateReadStreamOptions): ReadStream {
assertNotClosed(this, createReadStream.name);
const opts = options as Omit<CreateReadStreamOptions, "encoding">;
return createReadStream(this.#path, opts);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This opens another file object. We need to reuse #rid of this file handle.

Please follow what Node.js does here https://github.com/nodejs/node/blob/5770972dc6c0458af5458b8056b16d70905ea9d8/lib/internal/fs/promises.js#L361-L364

}

createReadStream(options?: CreateReadStreamOptions): ReadStream {
assertNotClosed(this, createReadStream.name);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@kt3k
Copy link
Copy Markdown
Contributor

kt3k commented Apr 7, 2025

Landed as #28700 e2341c7

@nkaradzhov Thanks for your contribution

@kt3k kt3k closed this Apr 7, 2025
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.

2 participants