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.

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

doc: add a note about readStream.path if fd is specified #40252

Closed
wants to merge 1 commit into from

Conversation

ylemkimon
Copy link
Contributor

#40013 changed the behavior of readStream.path, it'll be undefined if fd is specified in fs.createReadStream(). This is not a breaking change as docs state:

If fd is specified, ReadStream will ignore the path argument and will use the specified file descriptor.

However, this would be confusing for those who relied on readStream.path and this PR adds a note about it.

nodejs#40013 changed the behavior of `readStream.path`, it'll be `undefined` if `fd` is specified.
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. labels Sep 29, 2021
Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

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

Optionally it'd be great to have a test for this.

@jasnell jasnell added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 30, 2021
Ayase-252 added a commit to Ayase-252/node that referenced this pull request Oct 7, 2021
Ayase-252 pushed a commit that referenced this pull request Oct 7, 2021
it'll be `undefined` if `fd` is specified.

Refs: #40013

PR-URL: #40252
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Ayase-252
Copy link
Member

Landed in b77e36c.

Thanks for contribution.

@Ayase-252 Ayase-252 closed this Oct 7, 2021
danielleadams pushed a commit to danielleadams/node that referenced this pull request Oct 8, 2021
it'll be `undefined` if `fd` is specified.

Refs: nodejs#40013

PR-URL: nodejs#40252
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Oct 9, 2021
Refs: #40252 (review)

PR-URL: #40359
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Oct 13, 2021
Refs: #40252 (review)

PR-URL: #40359
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants