Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Purpose of fs ReadStream.open #29050

Closed
ronag opened this issue Aug 8, 2019 · 4 comments
Closed

Purpose of fs ReadStream.open #29050

ronag opened this issue Aug 8, 2019 · 4 comments
Labels
fs Issues and PRs related to the fs subsystem / file system. question Issues that look for answers.

Comments

@ronag
Copy link
Member

ronag commented Aug 8, 2019

I'm not quite sure I understand the purpose of ReadStream.open in fs? Why would I use this?

Opening the stream after closing it doesn't quite make sense and if passing an existing fd then it should already be opened.
@ronag ronag changed the title fs ReadStream.open Purpose of fs ReadStream.open Aug 8, 2019
@bnoordhuis bnoordhuis added fs Issues and PRs related to the fs subsystem / file system. question Issues that look for answers. labels Aug 9, 2019
@bnoordhuis
Copy link
Member

The reason is lost in the mist of time. It was introduced in the exquisitely well documented commit 44b308b from 2012.

I had some pretty strong misgivings about streams2 back then, among them that it had a lot of loose ends, and this was one of them. Isaac basically merged it without consulting anyone.

@ronag
Copy link
Member Author

ronag commented Aug 9, 2019 •
edited
Loading

Is it something we can deprecate? Remove from documentation?

@bnoordhuis
Copy link
Member

I don't think it's documented. Removal would need to start with run-time deprecation, i.e, printing a deprecation notice on first use.

@ronag ronag mentioned this issue Aug 9, 2019
Closed
4 tasks
@coreyfarrell coreyfarrell mentioned this issue Aug 14, 2019
Closed
4 tasks
@ronag ronag mentioned this issue Aug 24, 2019
Closed
4 tasks
@ronag
Copy link
Member Author

ronag commented Oct 6, 2019

Multiple PR's related to this.

@ronag ronag closed this as completed Oct 6, 2019
Trott pushed a commit that referenced this issue Dec 18, 2019
@ronag @Trott
fs: allow overriding fs for streams
2b9847c 
Allow overriding open, write, and close when using createReadStream()
and createWriteStream().

PR-URL: #29083
Refs: #29050
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
BridgeAR pushed a commit that referenced this issue Jan 3, 2020
@ronag @BridgeAR
fs: allow overriding fs for streams
7f6a0ed 
Allow overriding open, write, and close when using createReadStream()
and createWriteStream().

PR-URL: #29083
Refs: #29050
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
targos pushed a commit to targos/node that referenced this issue Apr 25, 2020
@ronag @targos
fs: allow overriding fs for streams
43379cf 
Allow overriding open, write, and close when using createReadStream()
and createWriteStream().

PR-URL: nodejs#29083
Refs: nodejs#29050
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
targos pushed a commit that referenced this issue Apr 28, 2020
@ronag @targos
fs: allow overriding fs for streams
a15e712 
Allow overriding open, write, and close when using createReadStream()
and createWriteStream().

PR-URL: #29083
Refs: #29050
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
fs Issues and PRs related to the fs subsystem / file system. question Issues that look for answers.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bnoordhuis @ronag