ext/node: `assert.deepStrictEqual({}, Object.create(null))` should throw

[kt3k]

The following script throws in Node, but not in Deno:

import assert from "node:assert"
assert.deepStrictEqual({}, Object.create(null))

[edilson258]

Hello, @kt3k

Please assign me this one.

[bartlomieju]

@edilson258 feel free to work on this. We only assign issues to Deno team members.

[edilson258]

There are some tests failing because they expect the old behavior where deepStrictEqual({}, Object.create(null)) is true, like:

https://github.com/denoland/deno/blob/ff078dcfabfe916c63db800d2d91ca0dd77da5c7/tests/node_compat/test/parallel/test-sqlite-transactions.js#L32-L43

All the above assertions now fails because db.prepare('BEGIN').run() returns an object whose prototype is null __proto__: null, whereas the expected object { changes: 0, lastInsertRowid: 0 } has the default Object.prototype. Since assert.deepStrictEqual NOW checks both property values and the prototype chain, the difference in prototypes causes the assertion to fail.

I think that setting __proto__: null in the expected object is the best solution because it ensures that the object has no prototype and is an approach that is already being used in similar cases, like:

https://github.com/denoland/deno/blob/ff078dcfabfe916c63db800d2d91ca0dd77da5c7/tests/node_compat/test/parallel/test-sqlite-transactions.js#L44-L47

There are lots of assertions in the code base that rely on that old behavior, this patch will touch lots of files in the source tree.

Is that proposed change acceptable?

@kt3k @bartlomieju

[kt3k]

@edilson258 Thanks for working on deepStrictEqual. The change in deepStrictEqual looks good to me.

The test files under tests/node_compat/test/parallel are copied from Node.js repository. Those tests are passing with Node.js. I think the issue here is the implementation of node:sqlite in Deno

[edilson258]

I think the issue here is the implementation of node:sqlite in Deno

True, but the issue isn't limited to node:sqlite, as the patch has caused multiple deepStrictEqual assertions to fail across a range of tests, including many that are unrelated to node:sqlite.