[vagos]

I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.

[okuryu]

it might be better to combine this if statement with the number if statement.

serialize-javascript/index.js

Lines 132 to 134 in 5838b91

	if (type === 'number' && !isNaN(origValue) && !isFinite(origValue)) {
	return '@__I-' + UID + '-' + (infinities.push(origValue) - 1) + '__@';
	}

[hlovdal]

This test can be simplified to if (Number.isNaN(value)).

Number.isNaN automatically returns false for non-number values.

[redonkulus]

Thanks for this, and sorry for the slow review. The NaN support is good and still missing on main (serialize(NaN) currently gives null). A few things before we can merge:

1. Please rebase onto main. The branch conflicts in index.js, and main no longer uses randombytes.
2. Port the tests to node:test. The suite moved from mocha/chai to the built-in runner, so the new tests currently fail with ReferenceError: expect is not defined. Use strictEqual from node:assert like the surrounding tests.
3. Please drop the Error support from this PR (or fix it in a separate one). As written, new Error({"name":…,"message":…}) passes an object as the message, so it deserializes with message "[object Object]", and subclasses like TypeError come back as plain Error. It also has no tests. If you'd like to tackle Errors separately, it would need to emit something like new Error(<serialized message>), restore name, and include round-trip tests.

With the NaN part on its own and the tests ported, this should be ready to go.