Skip to content

Commit 54aad99

Browse files
author
Robert Jackson
committed
Core: Ensure inherited getters are not invoked by assert.deepEqual.
Previously, given the following: ```js class Foo { constructor(a = 1) { this.a = a; } } Object.defineProperty(Foo.prototype, 'b', { enumerable: true, get() { return new Foo(this.a + 1); } }) QUnit.test( "hello test", function( assert ) { assert.deepEqual(new Foo(), new Foo()); }); ``` The `assert.deepEqual` invocation would never complete (ultimately crashing the tab or node process). The changes here ensure that inherited descriptors (getter / setter _or_ value only) are compared without invoking the getter therefore preventing the issue mentioned above.
1 parent 112d782 commit 54aad99

File tree

2 files changed

+59
-5
lines changed

2 files changed

+59
-5
lines changed

src/equiv.js

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,31 @@ export default ( function() {
1313
return obj.__proto__;
1414
};
1515

16+
function lookupDescriptor( obj, keyName ) {
17+
let current = obj;
18+
do {
19+
const descriptor = Object.getOwnPropertyDescriptor( current, keyName );
20+
if ( descriptor !== undefined ) {
21+
return descriptor;
22+
}
23+
current = getProto( current );
24+
} while ( current !== null );
25+
return null;
26+
}
27+
28+
function hasSharedDescriptor( a, b, keyName ) {
29+
var aDescriptor = lookupDescriptor( a, keyName );
30+
var bDescriptor = lookupDescriptor( b, keyName );
31+
32+
return ( aDescriptor !== null && bDescriptor !== null ) &&
33+
aDescriptor.value === bDescriptor.value &&
34+
aDescriptor.get === bDescriptor.get &&
35+
aDescriptor.set === bDescriptor.set &&
36+
aDescriptor.writable === bDescriptor.writable &&
37+
aDescriptor.configurable === bDescriptor.configurable &&
38+
aDescriptor.enumerable === bDescriptor.enumerable;
39+
}
40+
1641
function useStrictEquality( a, b ) {
1742

1843
// This only gets called if a and b are not strict equal, and is used to compare on
@@ -265,16 +290,30 @@ export default ( function() {
265290
aProperties.push( i );
266291

267292
// Skip OOP methods that look the same
293+
var hasValidConstructor = a.constructor !== Object &&
294+
typeof a.constructor !== "undefined";
295+
268296
if (
269-
a.constructor !== Object &&
270-
typeof a.constructor !== "undefined" &&
271-
typeof a[ i ] === "function" &&
272-
typeof b[ i ] === "function" &&
273-
a[ i ].toString() === b[ i ].toString()
297+
hasValidConstructor &&
298+
(
299+
(
300+
301+
// Skip own functions with same definition
302+
hasOwnProperty.call( a, i ) &&
303+
typeof a[ i ] === "function" &&
304+
typeof b[ i ] === "function" &&
305+
a[ i ].toString() === b[ i ].toString()
306+
) || (
307+
308+
// Skip shared inherited functions
309+
hasSharedDescriptor( a, b, i )
310+
)
311+
)
274312
) {
275313
continue;
276314
}
277315

316+
278317
// Compare non-containers; queue non-reference-equal containers
279318
if ( !breadthFirstCompareChild( a[ i ], b[ i ] ) ) {
280319
return false;

test/main/deepEqual.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1664,6 +1664,21 @@ QUnit.test( "Compare structures with multiple references to the same containers"
16641664
assert.equal( QUnit.equiv( { big: x, z: [ true ] }, { big: y, z: [ false ] } ), false, "Nonequivalent structures" );
16651665
} );
16661666

1667+
QUnit.test( "Compare instances with getters", function( assert ) {
1668+
function Foo( a ) {
1669+
this.a = a === undefined ? 1 : a;
1670+
}
1671+
1672+
Object.defineProperty( Foo.prototype, "b", {
1673+
enumerable: true,
1674+
get() {
1675+
return new Foo( this.a + 1 );
1676+
}
1677+
} );
1678+
1679+
assert.deepEqual( new Foo(), new Foo(), "inherited getters are not included in computation" );
1680+
} );
1681+
16671682
QUnit.test( "Test that must be done at the end because they extend some primitive's prototype",
16681683
function( assert ) {
16691684

0 commit comments

Comments
 (0)