doc: correctly use integer vs number

[avivkeller]

The integer type is an alias of number in our documentation, and isn't a valid JavaScript type. So, as to minimize confusion, the correct type (number) should be used instead.

[nodejs-github-bot]

Review requested:

• @nodejs/crypto
• @nodejs/http
• @nodejs/http2
• @nodejs/loaders
• @nodejs/net
• @nodejs/test_runner
• @nodejs/wasi

[JakobJingleheimer]

If integer is allowed, I think it's better / more informative because it's more specific (for instance, don't pass a float) even if it ultimately resolves to number.

[avivkeller]

If integer is allowed, I think it's better / more informative because it's more specific (for instance, don't pass a float) even if it ultimately resolves to number.

Firstly, a user may not know there is a difference, if it ultimately resolves to number.

Secondly, we don't use it that way. For example,

node/doc/api/tty.md

Line 110 in a73b575

* `fd` {number} A file descriptor associated with a TTY.

won't accept floats, but the number type is used.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.91%. Comparing base (a73b575) to head (c6e963e).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #59421   +/-   ##
=======================================
  Coverage   89.91%   89.91%           
=======================================
  Files         655      655           
  Lines      192866   192866           
  Branches    37806    37803    -3     
=======================================
+ Hits       173412   173420    +8     
+ Misses      12015    12006    -9     
- Partials     7439     7440    +1     

Files with missing lines	Coverage Δ
lib/internal/readline/promises.js	100.00% <100.00%> (ø)

... and 32 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[gurgunday]

lgtm

[aduh95]

I disagree that this minimize confusion, I'd say the contrary. Adding a request for changes so this doesn't land without some data to corroborate that affirmation.

[avivkeller]

data to corroborate that affirmation

The way I see it, this is not ideal for a few reasons:

1. In our new documentation generator, one will be able to search for "number" when looking for numeric parameters. Why should they also need to look at "integer"? We don't separate "string" and "char", for example.
2. For beginners, seeing integer listed separately may lead to the assumption that an integer type exists or that integer has special behavior, neither of which is true.
3. We aren't consistent about the use of integer over number. As I mentioned, there are cases where number is used to define something, and integer is used to define it elsewhere.

[ovflowd]

data to corroborate that affirmation

The way I see it, this is not ideal for a few reasons:

1. In our new documentation generator, one will be able to search for "number" when looking for numeric parameters. Why should they also need to look at "integer"? We don't separate "string" and "char", for example.
2. For beginners, seeing integer listed separately may lead to the assumption that an integer type exists or that integer has special behavior, neither of which is true.
3. We aren't consistent about the use of integer over number. As I mentioned, there are cases where number is used to define something, and integer is used to define it elsewhere.

The thing is that JavaScript doesn't have the "notion" of char as a struct. You could argue it doesn't either for an integer and a double and a float, but technically speaking, JavaScript keeps a reference of what that Number is supposed to be, based on how it gets defined https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number#description

Although the term Integer is never used, nor it is Float and Double, as JavaScript treats them equally, it's interesting that 1_000 === 1000 and 1.55 === 155/100 although there's an implict conversion to float happening there. But JavaScript will treat them equally.

I do agree that referencing to {number} for what are actually Numbers makes sense. Node itself doesn't have a barrier that in fields marked as "integer" only actual integers are allowed, but I might be wrong.

[aduh95]

Node itself doesn't have a barrier that in fields marked as "integer" only actual integers are allowed, but I might be wrong.

We do have validators for integers specifically, also we interact with C++ code for which integer and number are two different types.

node/lib/internal/validators.js

Lines 94 to 128 in 0fd1ecd

	const validateInteger = hideStackFrames(
	(value, name, min = NumberMIN_SAFE_INTEGER, max = NumberMAX_SAFE_INTEGER) => {
	if (typeof value !== 'number')
	throw new ERR_INVALID_ARG_TYPE(name, 'number', value);
	if (!NumberIsInteger(value))
	throw new ERR_OUT_OF_RANGE(name, 'an integer', value);
	if (value < min || value > max)
	throw new ERR_OUT_OF_RANGE(name, `>= ${min} && <= ${max}`, value);
	},
	);
	/**
	* @callback validateInt32
	* @param {*} value
	* @param {string} name
	* @param {number} [min]
	* @param {number} [max]
	* @returns {asserts value is number}
	*/
	/** @type {validateInt32} */
	const validateInt32 = hideStackFrames(
	(value, name, min = -2147483648, max = 2147483647) => {
	// The defaults for min and max correspond to the limits of 32-bit integers.
	if (typeof value !== 'number') {
	throw new ERR_INVALID_ARG_TYPE(name, 'number', value);
	}
	if (!NumberIsInteger(value)) {
	throw new ERR_OUT_OF_RANGE(name, 'an integer', value);
	}
	if (value < min || value > max) {
	throw new ERR_OUT_OF_RANGE(name, `>= ${min} && <= ${max}`, value);
	}
	},
	);

[ovflowd]

Node itself doesn't have a barrier that in fields marked as "integer" only actual integers are allowed, but I might be wrong.

We do have validators for integers specifically, also we interact with C++ code for which integer and number are two different types.

node/lib/internal/validators.js

Lines 94 to 128 in 0fd1ecd

	const validateInteger = hideStackFrames(
	(value, name, min = NumberMIN_SAFE_INTEGER, max = NumberMAX_SAFE_INTEGER) => {
	if (typeof value !== 'number')
	throw new ERR_INVALID_ARG_TYPE(name, 'number', value);
	if (!NumberIsInteger(value))
	throw new ERR_OUT_OF_RANGE(name, 'an integer', value);
	if (value < min || value > max)
	throw new ERR_OUT_OF_RANGE(name, `>= ${min} && <= ${max}`, value);
	},
	);
	/**
	* @callback validateInt32
	* @param {*} value
	* @param {string} name
	* @param {number} [min]
	* @param {number} [max]
	* @returns {asserts value is number}
	*/
	/** @type {validateInt32} */
	const validateInt32 = hideStackFrames(
	(value, name, min = -2147483648, max = 2147483647) => {
	// The defaults for min and max correspond to the limits of 32-bit integers.
	if (typeof value !== 'number') {
	throw new ERR_INVALID_ARG_TYPE(name, 'number', value);
	}
	if (!NumberIsInteger(value)) {
	throw new ERR_OUT_OF_RANGE(name, 'an integer', value);
	}
	if (value < min || value > max) {
	throw new ERR_OUT_OF_RANGE(name, `>= ${min} && <= ${max}`, value);
	}
	},
	);

Duly noted. I was referring to arguments of functions that I've used before and passed non-integers or what I assuked were non-integers, and it accepted them well.

Thanks for correcting me, tho. I bet that the functions that interact with the C++ counterparts either had casting or validation.

[mcollina]

Saying an integer is very different than a number, and even the JS internal representation would be different. In some cases (JS only APIs), ai think we handle both and likely it’s possible for changing it, but not in the general case.

[ovflowd]

@avivkeller would be open to change this PR to only change the integer to number on places where actually any number is allowed? Makes sense to keep integers for places that are actually only integers as @aduh95 and @mcollina mentioned 👀

[legendecas]

For beginners, seeing integer listed separately may lead to the assumption that an integer type exists or that integer has special behavior, neither of which is true.

FWIW, there is no integer JS primitive type, but an integer number has its observable behavior, in APIs like Number.isInteger or Number.isSafeInteger. Integer numbers can also be optimized by the JS engine with better performance in various cases, like a PACKED_SMI_ELEMENTS array. For reference, WebIDL even distinguishes more integer types: https://webidl.spec.whatwg.org/#dfn-integer-type

[joyeecheung]

The current documentation tool would link integer in the types to https://webidl.spec.whatwg.org/#dfn-integer-type by the way. For example see https://nodejs.org/api/process.html#event-exit - I think beginners would be able to figure out if they click the link.

[legendecas]

Not sure why the JSDoc is brought up here. The documents in /doc/api are not processed by the JSDoc.

[avivkeller]

Okay, I'll adjust the PR to make the distinction between the primitives more accurate (that is, using integer when an integer is needed, and number when a float is needed)

[aduh95]

Suggested change

	The `port` will be coerced to a integer. If it is not a legal port, a `TypeError`
	The `port` will be coerced to an integer. If it is not a legal port, a `TypeError`

[aduh95]

Suggested change

	The `port` will be coerced to a integer. If it is not a legal port, a `TypeError`
	The `port` will be coerced to an integer. If it is not a legal port, a `TypeError`

[aduh95]

Is that true? It's at least incomplete given you can set it to Infinity which doesn't have a nearest integer. Let's not talk about coerce, let's just say it's a maximum

Suggested change

	The `Error.stackTraceLimit` property specifies the number of stack frames,
	coerced to the nearest integer, collected by a stack trace (whether generated
	by `new Error().stack` or `Error.captureStackTrace(obj)`).
	The `Error.stackTraceLimit` property specifies the maximum number of stack
	frames collected by a stack trace (whether generated by `new Error().stack` or
	`Error.captureStackTrace(obj)`).

[aduh95]

Suggested change

	* `n` {integer} A non-negative number. The maximum number of listeners per
	* `n` {number} A non-negative number. The maximum number of listeners per

[aduh95]

Suggested change

	* Returns: {integer}
	* Returns: {number}

[aduh95]

Suggested change

	* `n` {integer}
	* `n` {number}

[aduh95]

Suggested change

	* Returns: {integer}
	* Returns: {number}

[aduh95]

Suggested change

	* Type: {integer}
	* Type: {number}

[aduh95]

Suggested change

	* Type: {integer}
	* Type: {number}

[aduh95]

Suggested change

	* Type: {integer}
	* Type: {number}

[aduh95]

Suggested change

	* Type: {integer} **Default:** `2000`
	* Type: {number} **Default:** `2000`

[aduh95]

Suggested change

	* Type: {integer} **Default:** `2000`
	* Type: {number} **Default:** `2000`

[aduh95]

Suggested change

	* Type: {integer} Requests per socket. **Default:** 0 (no limit)
	* Type: {number} Requests per socket. **Default:** 0 (no limit)

[aduh95]

Suggested change

	* Type: {integer} Timeout in milliseconds. **Default:** 0 (no timeout)
	* Type: {number} Timeout in milliseconds. **Default:** 0 (no timeout)

[aduh95]

There are way too many changes to properly review them, please split it into several PRs.