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: missing stream.Readable.ReadableState and stream.Writable.WritableState #38678

Closed
1 task done
pd4d10 opened this issue May 13, 2021 · 9 comments
Closed
1 task done
Labels
doc Issues and PRs related to the documentations.

Comments

@pd4d10
Copy link
Contributor

pd4d10 commented May 13, 2021

📗 API Reference Docs Problem

  • Version: 16
  • Platform: All
  • Subsystem: stream

Location

Description

stream.Readable.ReadableState and stream.Writable.WritableState are user accessible, but there seems to be no doc of them.


  • I would like to work on this issue and
    submit a pull request.
@pd4d10 pd4d10 added the doc Issues and PRs related to the documentations. label May 13, 2021
@Ayase-252
Copy link
Member

IIRC, the only way user could access them is this._readableState or this._writableState. I think the leading _ marks them as internal use only.

@Trott
Copy link
Member

Trott commented May 13, 2021

Related: #445

@mcollina
Copy link
Member

Thanks for reporting. I'll close this: those are not documented because they are private.

@mcollina
Copy link
Member

cc @nodejs/streams

@ronag
Copy link
Member

ronag commented May 16, 2021

IMO. They are and should remain private (or as private as we can make them).

@pd4d10
Copy link
Contributor Author

pd4d10 commented May 16, 2021

Get it. Shall we keep it private, for example, remove this line?

Readable.ReadableState = ReadableState;

If so I guess it also requires CITGM tests.

@mcollina
Copy link
Member

I'm not sure what you'd try to accomplish by removing them.

Doing so would require including a deprecation notice and a semver-major cycle.

@pd4d10
Copy link
Contributor Author

pd4d10 commented May 16, 2021

what you'd try to accomplish by removing them

None actually, just noticed that they weren't in the documentation but were exposed to the user

If this is a costly change, I'm also in favor of keeping it as is.

@mcollina
Copy link
Member

Unfortunately it is :(. The general wisdom is to avoid disruption if it could be avoided.

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.
Projects
None yet
Development

No branches or pull requests

5 participants