feat!(NODE-4850): serialize negative zero to double

[nbbeeken]

Description

What is changing?

When negative zero is detected it will be serialized to an 8 byte double instead of an Int32 to preserve the sign information.

Is there new documentation needed for these changes?

What is the motivation for this change?

Remove lossy serialization

Double check the following

• Ran npm run lint script
• Self-review completed using the steps outlined here
• PR title follows the correct format: <type>(NODE-xxxx)<!>: <description>
• Changes are covered by tests
• Migration guide updated
• New TODOs have a related JIRA ticket

[nbbeeken]

NODE-4850

[baileympearson]

Suggested change

	title: `fractional arithmetic (1.2 + 0.8) that sums to and int is serialized to int32`,
	title: `fractional arithmetic (1.2 + 0.8) that sums to an int is serialized to int32`,

[baileympearson]

Can you clarify why these tests are necessary?

Here are my thoughts. You did refactor how we serialize numbers, so we need to make sure that the refactor is adequately tested. But either our existing test coverage is adequate and no reason for new tests here (except for new negative zero tests) or our existing test coverage is inadequate. If our existing test coverage is inadequate, then we should either revert the refactor in our number parsing so we're not making untested refactors, or add more testing here so our coverage is complete and leave the refactor.

[nbbeeken]

they aren't necessary

[baileympearson]

Suggested change

	While use cases for negative zero may be rare it is important that the value is preserved on round trips through the BSON serialization layer. Prior to this change negative zero was possible to preserve with the BSON `new Double(-0)` type, such code can be updated to use `-0` directly.
	4x versions of BSON incorrectly serialized the literal -0 as an Int32, which does not support -0. This results in the following behavior:
	```typescript
	deserialize(serialize({ number: -0 })); // { number: 0 }
	deserialize(serialize({ number: new Double(-0) })) // { number: Double(-0) }

BSON now automatically serializes -0 to a Double.

deserialize(serialize({ number: -0 })); // { number: Double(-0) }

Any code previously serializing -0 as a double can now use the literal -0 syntax safely.

[nbbeeken]

updated, I kept it short and simple

[baileympearson]

Maybe we should highlight the difference in behavior here?

Suggested change

	Previously it was required to use the `Double` type class to preserve `-0`:
	```ts
	BSON.serialize({ d: new Double(-0) })
	```
	Now `-0` can be used directly
	```ts
	BSON.serialize({ d: -0 })
	```
	Previously it was required to use the `Double` type class to preserve `-0`:
	```ts
	BSON.deserialize(BSON.serialize({ d: new Double(-0) })); // { d: Double(-0) }
	BSON.deserialize(BSON.serialize({ d: new -0})); // { d: 0 }

Now -0 can be used directly

BSON.deserialize(BSON.serialize({ d: new -0})); // { d: Double(-0) }

[nbbeeken]

The syntax here is a little broken, did you intend new -0? and deserialize does not return a Double instance unless you disable promoteValues. There were no changes to deserialize because it will return a negative zero js number if one is encoded in the BSON bytes so there's nothing to showcase with deserialize here

[baileympearson]

Yeah that's just a copy-paste error, no new was necessary

I want the example to illustrate the difference between v4 and v5 and without deserializing, the difference is not apparent. That's why I wrapped them in deserialize calls

[nbbeeken]

updated