deno_std icon indicating copy to clipboard operation
deno_std copied to clipboard

`@std/assert/equals` considers `Object.create(null)` and `{}` to be equal

Open lilnasy opened this issue 11 months ago • 4 comments

Describe the bug

  • Since https://github.com/denoland/std/pull/6153, assertEquals checks object prototypes. However, it makes certain exceptions to keep existing tests within the repo from failing.
  • Other assertion libraries have more correct checks: "node:assert/strict", "ava", and "uvu" all make a distinction between null-prototype object and plain objects. "node:assert" and "chai" don't.

Steps to Reproduce The following assertion passes:

import { assertEquals } from "@std/assert"
assertEquals(Object.create(Object.prototype), Object.create(null))

Expected behavior The assertion fails.

Environment Not applicable.

lilnasy avatar Jan 06 '25 03:01 lilnasy

I don't get what this issue is about. The given example seems already failing.

kt3k avatar Jan 06 '25 03:01 kt3k

You're right, my steps to reproduce were incorrect.

- assertEquals(Object.create(Object), Object.create(null))
+ assertEquals(Object.create(Object.prototype), Object.create(null))

lilnasy avatar Jan 06 '25 04:01 lilnasy

If both objects have no properties, it makes sense to distinguish between a prototype of Object.prototype and no prototype. But if the objects do have properties, should we consider those equal? Alternatively, we could have one function that only checks properties and one that also checks prototypes?

A note on the tests: Before #6153, equal checked constructors, not prototypes. That's why an undefined constructor used to equal Object's constructor. As far as I know, there are exactly two tests that could be affected by this change:

  • assert/equals_test.ts, assertEquals() compares objects structurally if one object's constructor is undefined and the other is Object: Compares objects that have properties and are the same aside from prototypes
  • msgpack/encode_test.ts, encode() handles maps: Uses assertEquals(decode(encode(mapNull)), mapNull) when actually decode produces {} here.

WWRS avatar Mar 17 '25 16:03 WWRS

A note on the tests: Before #6153, equal checked constructors, not prototypes. That's why an undefined constructor used to equal Object's constructor.

Not exactly. Before the change, falsy constructors were already special-cased to count as equal to Object constructors. After the change, nullish prototypes are special-cased in the same way, which I just did for consistency with the assumed intention of the old behavior. There's no practical difference between falsy and nullish here, as the only falsy value possible for a prototype is null. constructor, meanwhile, is just a normal property name, so it could be overridden and return anything.

I'm not sure why falsy constructors were special-cased in the first place, but I assume it was intended as an ergonomic thing.

lionel-rowe avatar Mar 18 '25 02:03 lionel-rowe