Confusing and inconsistent comparison of NaN

[aradzie]

Consider the following example:

import { test } from "node:test";
import { assert, expect } from "chai";

test("pass", () => {
  expect([NaN]).to.deep.equal([NaN]);
  expect({ a: NaN }).to.deep.equal({ a: NaN });

  assert.deepStrictEqual([NaN], [NaN]);
  assert.deepStrictEqual({ a: NaN }, { a: NaN });
});

test("fail/expect", () => {
  expect(NaN).to.equal(NaN);
});

test("fail/assert", () => {
  assert.strictEqual(NaN, NaN);
});

✖ fail/expect
  Error [AssertionError]: expected NaN to equal NaN
✖ fail/assert
  Error [AssertionError]: expected NaN to equal NaN

The comparison algorithm behaves differently depending on whether NaN values are compared directly or as deep property/member values. This inconsistency creates confusion and diminishes the developer experience.

We suggest to change the behavior of expect(...).to.equal(...) and assert.strictEqual(..., ...) to match their deep counterparts.

[aradzie]

And for the sake of comparison, a similar example, this time using the Node.js assert module:

import assert from "node:assert/strict";
import { test } from "node:test";

test("pass", () => {
  assert.strictEqual(NaN, NaN);
  assert.deepStrictEqual([NaN], [NaN]);
  assert.deepStrictEqual({ a: NaN }, { a: NaN });
});

The test above successfully passes.

[43081j]

Makes sense to me

We should do the same as node I suspect

@koddsson thoughts?

[koddsson]

equals is essentially just a short-hand for === and in JavaScript NaN !== NaN so to me chai is doing the correct thing here.

Node was special casing their implementation to specifically handle NaN === NaN which to me is confusing. It seems like they're moving away from this by marking assert.equals as deprecated and pointing users towards assert.strictEqual which uses Object.is.

I think our strictEquals should probably assert correctly that NaN === NaN but the equals is working correctly to me.

I'm not 100% sure on the .deep bits. deep-eql uses Object.is for equality so it makes sense that it says that NaN === NaN like the node implementation.

---

With all this being said this has been working like this for a while so I think this feedback is good for how we change the API for Chai@v6 as it seems like most of these changes warrant a breaking change.