Node.js 12.0.0 changes how arrays with symbol properties are compared using deepEqual()

[vkarpov15]

Hi, it looks like Node 12 changed how arrays are compared with deepEqual(), presumably due to https://github.com/nodejs/node/pull/25008/files . Here's an example of how deepEqual() handles arrays with symbol properties:

const assert = require('assert');

console.log(process.version);

const s = Symbol('foo');

const arr = [];
arr[s] = 42;

assert.deepEqual(arr, []);

Running this script with Node.js 11.9.0 succeeds:

$ ~/Workspace/libs/node-v11.9.0-linux-x64/bin/node test.js 
v11.9.0
$

Running with Node.js 12 fails:

$ ~/Workspace/libs/node-v12.0.0-linux-x64/bin/node test.js 
v12.0.0
assert.js:89
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

[
  [Symbol(foo)]: 42
]

should equal

[]
$ 

Is this a bug or expected behavior? If so, you should update the legacy mode deepEqual() docs, which explicitly state that symbol properties are ignored.

Related to Automattic/mongoose#7784.

[targos]

/cc @nodejs/assert @BridgeAR

[BridgeAR]

This was indeed an unintentional change and I missed that the array optimization currently always gets the symbol properties even though the loose equal comparison should not compare symbol properties. I'll open a PR to fix that.