deno icon indicating copy to clipboard operation
deno copied to clipboard

fix(node/assert): throw on deepStrictEqual({}, Object.create(null))

Open edilson258 opened this issue 7 months ago • 2 comments

assert.deepStrictEqual now differentiates {} from Object.create(null)

This PR updates the object comparison logic to throw when comparing a plain object ({}) with an object created via Object.create(null), matching Node.js behavior.

Closes #27565

edilson258 avatar May 23 '25 19:05 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

edilson258 avatar May 24 '25 04:05 edilson258

@edilson258 Thanks for the PR. The maintainers will work on the fixes for the existing test cases (they should be passing as is). Please wait for a moment.

kt3k avatar May 26 '25 06:05 kt3k

Note to self (and the team):

  • [x] In generateKeyPair callback, publicKey.asymmetricKeyDetails needs to be have undefined prototype. #29576
  • [x] generateKeyPairSync returns publicKey which needs to have undefined prototype. #29576
  • [x] StatementSync.prototype.run() needs to return object with undefined prototype. #29404

bartlomieju avatar Jun 02 '25 13:06 bartlomieju

Thanks!

edilson258 avatar Jun 04 '25 05:06 edilson258