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: Buffer UInt -> Uint aliases are not documented #36751

Closed
1 task
adroitwhiz opened this issue Jan 3, 2021 · 6 comments
Closed
1 task

doc: Buffer UInt -> Uint aliases are not documented #36751

adroitwhiz opened this issue Jan 3, 2021 · 6 comments
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors.

Comments

@adroitwhiz
Copy link

📗 API Reference Docs Problem

  • Version: v12.19.0 and up, v14.9.0 and up, v15 and up
  • Platform: All
  • Subsystem: Buffer

Location

Affected URL(s):

Description

The Buffer.[read/write]UInt -> Buffer.[read/write]Uint aliases added in #34729 do not appear in the rendered documentation, despite appearing in the YAML sections.


  • I would like to work on this issue and
    submit a pull request.
@adroitwhiz adroitwhiz added the doc Issues and PRs related to the documentations. label Jan 3, 2021
@aduh95 aduh95 added the good first issue Issues that are suitable for first-time contributors. label Jan 4, 2021
@jasnell
Copy link
Member

jasnell commented Jan 4, 2021

cc'ing @addaleax who added those aliases in case there was a reason those weren't documented.

@addaleax
Copy link
Member

addaleax commented Jan 4, 2021

Yeah, I intended them to be typo-catchers and they’re there just for consistency with the typed array naming convention. Adding separate documentation entries to the (already heavily redundant) page seemed relatively pointless.

@adroitwhiz
Copy link
Author

I ran into some confusion when someone's code (calling a lowercase version of the method) threw an error on my machine because I was running on a version of Node where the lowercase methods did not exist, and because there's no documentation on this, couldn't figure out why it was even working on their machine.

While the change has been backported to the current supported LTS releases, it's incredibly confusing to have undocumented method aliases that exist in some versions of Node but not others, with no indication that e.g. TypeError: [...].readUint8 is not a function is fixed by upgrading Node.

@addaleax
Copy link
Member

addaleax commented Jan 4, 2021

I guess my point isn’t so much that this should not documented per se, but that it seems hard to do so in a meaningful way that doesn’t clutter the documentation unnecessarily. I guess, my main question is, if you run into this problem, where would you look it up? If you go to the docs for https://nodejs.org/api/buffer.html#buffer_buf_readuint8_offset – which is what I would probably do in a case like this, but that’s just me – you’ll find that this is part of the changelog for the function.

If we add a paragraph like “The following aliases are provided: readUint8 for readUInt8, writeUint8 for writeUInt8”, etc., that would seem just fine to me, but how useful is that given that most text searches (both web search and in-page search in browsers) are usually case-insensitive by default?

@adroitwhiz
Copy link
Author

I think that would be useful--when I was trying to figure out why readUint8 was working, I Ctrl+F'd for readuint8 (case-insensitive) and checked if any of the matches mentioned something about an alias.

I didn't think to check the History section; I assumed that if I was on the latest version of the docs, everything relating to the current state of the function (aliases included) would be outside the History section.

@targos
Copy link
Member

targos commented Jan 5, 2021

I suggest an update to the docs here: #36796

@targos targos closed this as completed in e68299a Jan 9, 2021
danielleadams pushed a commit that referenced this issue Jan 12, 2021
Fixes: #36751

PR-URL: #36796
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Harshitha K P <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
targos added a commit that referenced this issue May 1, 2021
Fixes: #36751

PR-URL: #36796
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Harshitha K P <[email protected]>
Reviewed-By: Gireesh Punathil <[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. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants