Repository: nodejs/node
Pull request: assert,util: fail promise comparison in deep equal checks

It is impossible to look into the content of a promise and it's state. This aligns the comparison with WeakMaps and WeakSets. Only reference equal promises will pass the check in the future.

Fixes https://github.com/nodejs/node/issues/55198

Changed files:
filename: doc/api/assert.md
patch: @@ -278,6 +278,10 @@ An alias of [`assert.ok()`][].
 <!-- YAML
 added: v0.1.21
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/59448
+    description: Promises are not considered equal anymore if they are not of
+                 the same instance.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/57627
     description: Invalid dates are now considered equal.
@@ -366,8 +370,10 @@ are also recursively evaluated by the following rules.
 * Implementation does not test the [`[[Prototype]]`][prototype-spec] of
   objects.
 * {Symbol} properties are not compared.
-* {WeakMap} and {WeakSet} comparison does not rely on their values
-  but only on their instances.
+* {WeakMap}, {WeakSet} and {Promise} instances are **not** compared
+  structurally. They are only equal if they reference the same object. Any
+  comparison between different `WeakMap`, `WeakSet`, or `Promise` instances
+  will result in inequality, even if they contain the same content.
 * {RegExp} lastIndex, flags, and source are always compared, even if these
   are not enumerable properties.
 
@@ -472,6 +478,10 @@ parameter is an instance of {Error} then it will be thrown instead of the
 <!-- YAML
 added: v1.2.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/59448
+    description: Promises are not considered equal anymore if they are not of
+                 the same instance.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/57627
     description: Invalid dates are now considered equal.
@@ -540,10 +550,10 @@ are recursively evaluated also by the following rules.
 * {Map} keys and {Set} items are compared unordered.
 * Recursion stops when both sides differ or either side encounters a circular
   reference.
-* {WeakMap} and {WeakSet} instances are **not** compared structurally.
-  They are only equal if they reference the same object. Any comparison between
-  different `WeakMap` or `WeakSet` instances will result in inequality,
-  even if they contain the same entries.
+* {WeakMap}, {WeakSet} and {Promise} instances are **not** compared
+  structurally. They are only equal if they reference the same object. Any
+  comparison between different `WeakMap`, `WeakSet`, or `Promise` instances
+  will result in inequality, even if they contain the same content.
 * {RegExp} lastIndex, flags, and source are always compared, even if these
   are not enumerable properties.
 
@@ -2230,6 +2240,10 @@ added:
   - v23.4.0
   - v22.13.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/59448
+    description: Promises are not considered equal anymore if they are not of
+                 the same instance.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/57627
     description: Invalid dates are now considered equal.
@@ -2268,10 +2282,10 @@ behaving as a super set of it.
 * {Map} keys and {Set} items are compared unordered.
 * Recursion stops when both sides differ or both sides encounter a circular
   reference.
-* {WeakMap} and {WeakSet} instances are **not** compared structurally.
-  They are only equal if they reference the same object. Any comparison between
-  different `WeakMap` or `WeakSet` instances will result in inequality,
-  even if they contain the same entries.
+* {WeakMap}, {WeakSet} and {Promise} instances are **not** compared
+  structurally. They are only equal if they reference the same object. Any
+  comparison between different `WeakMap`, `WeakSet`, or `Promise` instances
+  will result in inequality, even if they contain the same content.
 * {RegExp} lastIndex, flags, and source are always compared, even if these
   are not enumerable properties.
 * Holes in sparse arrays are ignored.

filename: lib/internal/util/comparisons.js
patch: @@ -99,22 +99,23 @@ const types = require('internal/util/types');
 const {
   isAnyArrayBuffer,
   isArrayBufferView,
+  isBigIntObject,
+  isBooleanObject,
+  isBoxedPrimitive,
+  isCryptoKey,
   isDate,
+  isFloat16Array,
+  isFloat32Array,
+  isFloat64Array,
+  isKeyObject,
   isMap,
-  isRegExp,
-  isSet,
   isNativeError,
-  isBoxedPrimitive,
   isNumberObject,
+  isPromise,
+  isRegExp,
+  isSet,
   isStringObject,
-  isBooleanObject,
-  isBigIntObject,
   isSymbolObject,
-  isFloat16Array,
-  isFloat32Array,
-  isFloat64Array,
-  isKeyObject,
-  isCryptoKey,
   isWeakMap,
   isWeakSet,
 } = types;
@@ -409,7 +410,7 @@ function objectComparisonStart(val1, val2, mode, memos) {
     ) {
       return false;
     }
-  } else if (isWeakMap(val1) || isWeakSet(val1)) {
+  } else if (isWeakMap(val1) || isWeakSet(val1) || isPromise(val1)) {
     return false;
   }
 

filename: test/parallel/test-assert-deep.js
patch: @@ -1633,3 +1633,10 @@ test('Inherited null prototype without own constructor properties should check t
   assert.deepEqual(a, b);
   assert.deepEqual(b, a);
 });
+
+test('Promises should fail deepEqual', () => {
+  const a = Promise.resolve(1);
+  const b = Promise.resolve(1);
+  assertDeepAndStrictEqual(a, a);
+  assertNotDeepOrStrict(a, b);
+});