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

Please document fs.WriteStream.path #4327

Closed
rogierschouten opened this issue Dec 17, 2015 · 8 comments
Closed

Please document fs.WriteStream.path #4327

rogierschouten opened this issue Dec 17, 2015 · 8 comments
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system.

Comments

@rogierschouten
Copy link

WriteStreams have a .path property with the path of the file they write to. Please document this property so that I can reliably use it without fear of it being an unsupported feature. This will also enable me to put it into node.d.ts and push that to DefinitelyTyped.

@bnoordhuis bnoordhuis added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. labels Dec 17, 2015
@bnoordhuis
Copy link
Member

I don't know, it has a bunch of undocumented properties; the reason they're not underscore-prefixed is mostly an accident of history.

@claudiorodriguez
Copy link
Contributor

Among these, only bytesWritten is documented, the rest are not:

this.path = path;
this.fd = options.fd === undefined ? null : options.fd;
this.flags = options.flags === undefined ? 'w' : options.flags;
this.mode = options.mode === undefined ? 0o666 : options.mode;
this.start = options.start;
this.pos = undefined;
this.bytesWritten = 0;

At this point it would be a serious breaking change to prefix them all with underscores, of course, but exposing them would require some more clarification IMHO. From what I'm seeing, the first implementation of these was by @felixge
cc @nodejs/fs ?

@rogierschouten
Copy link
Author

The reason I use the path property is that I get a WriteStream from an NPM module that makes temp files with random names. I need to get that random name and WriteStream#path is the only way to do it. I don't see why these couldn't be made public, apparently people are relying on this.

@claudiorodriguez
Copy link
Contributor

Well, it -has- been there since Mar 2010 at least.

@mscdex
Copy link
Contributor

mscdex commented Dec 17, 2015

@rogierschouten Seems like that module could just return the name it used?

@felixge
Copy link
Contributor

felixge commented Dec 19, 2015

To provide some historical context. I implemented WriteStream over 5 years ago. At this time node core didn't have much coding conventions and objects often exposed "private" properties without any indication (see 1, 2). IIRC Ryan disliked the '_' prefix convention back then, possibly out of resentment for Python.

As far as the path property is concerned, I don't recall if I intended it to be private or not, but at this point I'd just document it as an official property. Having a mechanism for getting the path seems very useful, and I can only imagine how much code in the wild already depends on the existence of the path property as-is.

claudiorodriguez added a commit to claudiorodriguez/node that referenced this issue Dec 20, 2015
Documents the "path" property on fs.WriteStream
and fs.ReadStream. See nodejs#4327
@aredridel
Copy link
Contributor

I've written code that depends on this for sure.

jasnell pushed a commit that referenced this issue Jan 15, 2016
Documents the "path" property on fs.WriteStream
and fs.ReadStream. See #4327

PR-URL: #4368
Reviewed-By: James M Snell <[email protected]>
@jasnell
Copy link
Member

jasnell commented Jan 15, 2016

Landed

@jasnell jasnell closed this as completed Jan 15, 2016
evanlucas pushed a commit that referenced this issue Jan 18, 2016
Documents the "path" property on fs.WriteStream
and fs.ReadStream. See #4327

PR-URL: #4368
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Jan 28, 2016
Documents the "path" property on fs.WriteStream
and fs.ReadStream. See #4327

PR-URL: #4368
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Feb 11, 2016
Documents the "path" property on fs.WriteStream
and fs.ReadStream. See #4327

PR-URL: #4368
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit to MylesBorins/node that referenced this issue Feb 11, 2016
Documents the "path" property on fs.WriteStream
and fs.ReadStream. See nodejs#4327

PR-URL: nodejs#4368
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit to MylesBorins/node that referenced this issue Feb 15, 2016
Documents the "path" property on fs.WriteStream
and fs.ReadStream. See nodejs#4327

PR-URL: nodejs#4368
Reviewed-By: James M Snell <[email protected]>
scovetta pushed a commit to scovetta/node that referenced this issue Apr 2, 2016
Documents the "path" property on fs.WriteStream
and fs.ReadStream. See nodejs#4327

PR-URL: nodejs#4368
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

No branches or pull requests

7 participants