Skip to content

Commit 7c9fbc1

Browse files
authored
assert,util: fail promise comparison in deep equal checks
It is impossible to look into the content of a promise and its state. This aligns the comparison with WeakMaps and WeakSets. Only reference equal promises will pass the check in the future. Fixes #55198 PR-URL: #59448 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
1 parent 8973589 commit 7c9fbc1

File tree

3 files changed

+43
-21
lines changed

3 files changed

+43
-21
lines changed

doc/api/assert.md

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,10 @@ An alias of [`assert.ok()`][].
278278
<!-- YAML
279279
added: v0.1.21
280280
changes:
281+
- version: REPLACEME
282+
pr-url: https://github.com/nodejs/node/pull/59448
283+
description: Promises are not considered equal anymore if they are not of
284+
the same instance.
281285
- version: REPLACEME
282286
pr-url: https://github.com/nodejs/node/pull/57627
283287
description: Invalid dates are now considered equal.
@@ -366,8 +370,10 @@ are also recursively evaluated by the following rules.
366370
* Implementation does not test the [`[[Prototype]]`][prototype-spec] of
367371
objects.
368372
* {Symbol} properties are not compared.
369-
* {WeakMap} and {WeakSet} comparison does not rely on their values
370-
but only on their instances.
373+
* {WeakMap}, {WeakSet} and {Promise} instances are **not** compared
374+
structurally. They are only equal if they reference the same object. Any
375+
comparison between different `WeakMap`, `WeakSet`, or `Promise` instances
376+
will result in inequality, even if they contain the same content.
371377
* {RegExp} lastIndex, flags, and source are always compared, even if these
372378
are not enumerable properties.
373379

@@ -472,6 +478,10 @@ parameter is an instance of {Error} then it will be thrown instead of the
472478
<!-- YAML
473479
added: v1.2.0
474480
changes:
481+
- version: REPLACEME
482+
pr-url: https://github.com/nodejs/node/pull/59448
483+
description: Promises are not considered equal anymore if they are not of
484+
the same instance.
475485
- version: REPLACEME
476486
pr-url: https://github.com/nodejs/node/pull/57627
477487
description: Invalid dates are now considered equal.
@@ -540,10 +550,10 @@ are recursively evaluated also by the following rules.
540550
* {Map} keys and {Set} items are compared unordered.
541551
* Recursion stops when both sides differ or either side encounters a circular
542552
reference.
543-
* {WeakMap} and {WeakSet} instances are **not** compared structurally.
544-
They are only equal if they reference the same object. Any comparison between
545-
different `WeakMap` or `WeakSet` instances will result in inequality,
546-
even if they contain the same entries.
553+
* {WeakMap}, {WeakSet} and {Promise} instances are **not** compared
554+
structurally. They are only equal if they reference the same object. Any
555+
comparison between different `WeakMap`, `WeakSet`, or `Promise` instances
556+
will result in inequality, even if they contain the same content.
547557
* {RegExp} lastIndex, flags, and source are always compared, even if these
548558
are not enumerable properties.
549559

@@ -2230,6 +2240,10 @@ added:
22302240
- v23.4.0
22312241
- v22.13.0
22322242
changes:
2243+
- version: REPLACEME
2244+
pr-url: https://github.com/nodejs/node/pull/59448
2245+
description: Promises are not considered equal anymore if they are not of
2246+
the same instance.
22332247
- version: REPLACEME
22342248
pr-url: https://github.com/nodejs/node/pull/57627
22352249
description: Invalid dates are now considered equal.
@@ -2268,10 +2282,10 @@ behaving as a super set of it.
22682282
* {Map} keys and {Set} items are compared unordered.
22692283
* Recursion stops when both sides differ or both sides encounter a circular
22702284
reference.
2271-
* {WeakMap} and {WeakSet} instances are **not** compared structurally.
2272-
They are only equal if they reference the same object. Any comparison between
2273-
different `WeakMap` or `WeakSet` instances will result in inequality,
2274-
even if they contain the same entries.
2285+
* {WeakMap}, {WeakSet} and {Promise} instances are **not** compared
2286+
structurally. They are only equal if they reference the same object. Any
2287+
comparison between different `WeakMap`, `WeakSet`, or `Promise` instances
2288+
will result in inequality, even if they contain the same content.
22752289
* {RegExp} lastIndex, flags, and source are always compared, even if these
22762290
are not enumerable properties.
22772291
* Holes in sparse arrays are ignored.

lib/internal/util/comparisons.js

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -99,22 +99,23 @@ const types = require('internal/util/types');
9999
const {
100100
isAnyArrayBuffer,
101101
isArrayBufferView,
102+
isBigIntObject,
103+
isBooleanObject,
104+
isBoxedPrimitive,
105+
isCryptoKey,
102106
isDate,
107+
isFloat16Array,
108+
isFloat32Array,
109+
isFloat64Array,
110+
isKeyObject,
103111
isMap,
104-
isRegExp,
105-
isSet,
106112
isNativeError,
107-
isBoxedPrimitive,
108113
isNumberObject,
114+
isPromise,
115+
isRegExp,
116+
isSet,
109117
isStringObject,
110-
isBooleanObject,
111-
isBigIntObject,
112118
isSymbolObject,
113-
isFloat16Array,
114-
isFloat32Array,
115-
isFloat64Array,
116-
isKeyObject,
117-
isCryptoKey,
118119
isWeakMap,
119120
isWeakSet,
120121
} = types;
@@ -409,7 +410,7 @@ function objectComparisonStart(val1, val2, mode, memos) {
409410
) {
410411
return false;
411412
}
412-
} else if (isWeakMap(val1) || isWeakSet(val1)) {
413+
} else if (isWeakMap(val1) || isWeakSet(val1) || isPromise(val1)) {
413414
return false;
414415
}
415416

test/parallel/test-assert-deep.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1633,3 +1633,10 @@ test('Inherited null prototype without own constructor properties should check t
16331633
assert.deepEqual(a, b);
16341634
assert.deepEqual(b, a);
16351635
});
1636+
1637+
test('Promises should fail deepEqual', () => {
1638+
const a = Promise.resolve(1);
1639+
const b = Promise.resolve(1);
1640+
assertDeepAndStrictEqual(a, a);
1641+
assertNotDeepOrStrict(a, b);
1642+
});

0 commit comments

Comments
 (0)