Skip to content

Conversation

SebastienGllmt
Copy link
Contributor

This PR implements the missing support for createReadStream and createWriteStream on the FileHandler class (as outlined in #25554)

Notably, it does so by building on top of #27591 which was presumably abandoned

);

Deno.test(
"[node/fs filehandle.createWriteStream] Create a write stream",
Copy link
Contributor Author

@SebastienGllmt SebastienGllmt Apr 1, 2025

Choose a reason for hiding this comment

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

I'm not sure if these tests are meant to be taken from somewhere

The definition of the createReadStream test I took from #27591
The definition of the createWriteStream test I wrote myself trying to follow the same style

@SebastienGllmt SebastienGllmt force-pushed the feat_createreadstream_to_filehandle branch from 93b2e2b to 9337f3e Compare April 2, 2025 04:11
@kt3k kt3k changed the title feat(node/fs): Add a createReadStream & createWriteStream methods to the FileHandle class fix(node/fs): Add a createReadStream & createWriteStream methods to the FileHandle class Apr 7, 2025
Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

Simplified the createWriteStream test case to avoid the flakiness in CI

@kt3k kt3k changed the title fix(node/fs): Add a createReadStream & createWriteStream methods to the FileHandle class fix(ext/node): add a createReadStream & createWriteStream methods to the FileHandle class Apr 7, 2025
@kt3k kt3k merged commit e2341c7 into denoland:main Apr 7, 2025
18 checks passed
@SebastienGllmt SebastienGllmt deleted the feat_createreadstream_to_filehandle branch April 7, 2025 11:37
nayeemrmn pushed a commit that referenced this pull request Apr 11, 2025
…e FileHandle class (#28700)

Co-authored-by: Nikolay Karadzhov <nkaradzhov89@gmail.com>
Co-authored-by: Yoshiya Hinosawa <stibium121@gmail.com>
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.

3 participants