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: <integer> is misleading #33585

Open
1 task
seishun opened this issue May 27, 2020 · 5 comments
Open
1 task

doc: <integer> is misleading #33585

seishun opened this issue May 27, 2020 · 5 comments
Labels
doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project.

Comments

@seishun
Copy link
Contributor

seishun commented May 27, 2020

📗 API Reference Docs Problem

  • Version: master

Location

Affected URL(s):

Problem description

Many methods are documented to take <integer> or <integer[]>. Examples:
Buffer.from(array)
emitter.setMaxListeners(n)
It seems to always mean "an integer Number". However, clicking the type link leads to an MDN section that starts with "ECMAScript has two built-in numeric types: Number and BigInt (see below).". One might erroneously think that BigInt, which is also an "integer", works too.


  • I would like to work on this issue and submit a pull request.
@seishun seishun added the doc Issues and PRs related to the documentations. label May 27, 2020
@DerekNonGeneric
Copy link
Contributor

Any clues on what needs to be done to correct this? It seems as though C++ types are being mixed into the JS docs.

/cc @addaleax

@BridgeAR
Copy link
Member

BridgeAR commented Jun 5, 2020

We declare a variable as integer in case it should be an integer, not a decimal number. It's still a number as actual JS type but it should still be useful for the reader to know that we likely validate that it's in fact an integer (or that the API will only work as expected with integers).

We might want to stop referencing to JS numbers on MDN. Besides that, I would keep it as is.

@DerekNonGeneric
Copy link
Contributor

I wonder if it would be more helpful to link to specification in this regard.

https://www.ecma-international.org/ecma-262/6.0/#sec-number.isinteger

@DerekNonGeneric
Copy link
Contributor

DerekNonGeneric commented Jun 17, 2020

I found a nice sentence from the V8 docs supporting my initial thought about them
being C++ integers, so alternatives may exist.

These accessor functions convert a C++ integer to a JavaScript integer using
Integer::New, and convert a JavaScript integer to a C++ integer using
Int32Value. — https://v8.dev/docs/embed#accessing-static-global-variables

Since @BridgeAR gave me the lead on this, it would be nice to have his approval
on #33766 too, but no pressure.

@DerekNonGeneric
Copy link
Contributor

I would have liked to link to something like the following.

https://docs.oracle.com/javase/10/docs/api/java/lang/Integer.html

Unsure if linking to the class from the V8 docs would be out of the question.

https://v8docs.nodesource.com/node-14.1/df/d84/classv8_1_1_integer.html

Or is it too late to create an Integer class, implement it, and document it?

I'm not really sure what needs to be done here… (Sorry I couldn't be of more help on this.)

@targos targos added the meta Issues and PRs related to the general management of the project. label Aug 9, 2021
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. meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants