Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deepStrictEqual not commutative by not accounting for non-enumerable properties #37710

Closed
nmalyschkin opened this issue Mar 11, 2021 · 0 comments · Fixed by #37711
Closed

deepStrictEqual not commutative by not accounting for non-enumerable properties #37710

nmalyschkin opened this issue Mar 11, 2021 · 0 comments · Fixed by #37711
Assignees
Labels
assert Issues and PRs related to the assert subsystem. confirmed-bug Issues with confirmed bugs.

Comments

@nmalyschkin
Copy link

  • Version:
    • 14.x (but probably also others)
  • Platform:
    • all

What steps will reproduce the bug?

const { deepStrictEqual } = require("assert");

const a = {};
const b = {};

a.x = 1;

Object.defineProperties(b, {
  x: {
    value: 1,
    writable: true,
  },
  y: {
    value: 5,
    writable: true,
    enumerable: true,
    configurable: true,
  },
});

console.log({ a, b });
deepStrictEqual(a, b); // does not assert
deepStrictEqual(b, a); // does assert

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior?

I expect deepStrictEqual to be at least commutative. (Whether it should handle non-enumerable properties may be up for discussion)

What do you see instead?

deepStrictEqual(b, a) does throw an assertion while deepStrictEqual(a, b) does not.

Additional information

If you point me in the right direction I'm willing to fix it myself.

@BridgeAR BridgeAR self-assigned this Mar 11, 2021
@BridgeAR BridgeAR added assert Issues and PRs related to the assert subsystem. confirmed-bug Issues with confirmed bugs. labels Mar 11, 2021
BridgeAR added a commit to BridgeAR/node that referenced this issue Mar 11, 2021
Verify that both objects property keys are enumerable.

Fixes: nodejs#37710
@aduh95 aduh95 closed this as completed in 5ff2e58 Mar 13, 2021
danielleadams pushed a commit that referenced this issue Mar 16, 2021
Verify that both objects property keys are enumerable.

Fixes: #37710

PR-URL: #37711
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
danielleadams pushed a commit that referenced this issue Mar 16, 2021
Verify that both objects property keys are enumerable.

Fixes: #37710

PR-URL: #37711
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this issue May 1, 2021
Verify that both objects property keys are enumerable.

Fixes: #37710

PR-URL: #37711
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert Issues and PRs related to the assert subsystem. confirmed-bug Issues with confirmed bugs.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants