From 5c6480b95dd2dc8d21be6bfe01187bf4935c87a2 Mon Sep 17 00:00:00 2001 From: souldzin <4908127+souldzin@users.noreply.github.com> Date: Sat, 22 Aug 2020 05:18:51 -0500 Subject: [PATCH] fix: toMatchObject works with super getters (#10381) --- CHANGELOG.md | 2 + .../__snapshots__/matchers.test.js.snap | 33 +++++++++++++++ .../expect/src/__tests__/matchers.test.js | 23 +++++++++++ packages/expect/src/__tests__/utils.test.ts | 41 ------------------- packages/expect/src/utils.ts | 36 ++++++---------- 5 files changed, 71 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d7334b849147..9cc0427b6aa3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### Fixes +- `[expect]` Fix `toMatchObject` to work with inherited class getters ([#10381](https://github.com/facebook/jest/pull/10381)) + ### Chore & Maintenance ### Performance diff --git a/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap b/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap index aa0d250d4cb4..829838a271b1 100644 --- a/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap +++ b/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap @@ -3988,6 +3988,18 @@ exports[`toMatchObject() {pass: false} expect({"a": "b"}).toMatchObject({"c": "d } `; +exports[`toMatchObject() {pass: false} expect({"a": "b"}).toMatchObject({"toString": Any}) 1`] = ` +expect(received).toMatchObject(expected) + +- Expected - 0 ++ Received + 1 + + Object { ++ "a": "b", + "toString": Any, + } +`; + exports[`toMatchObject() {pass: false} expect({"a": [{"a": "a", "b": "b"}]}).toMatchObject({"a": [{"a": "c"}]}) 1`] = ` expect(received).toMatchObject(expected) @@ -4246,6 +4258,13 @@ Expected: not {"t": {"x": {"r": "r"}}} Received: {"a": "b", "t": {"x": {"r": "r"}, "z": "z"}} `; +exports[`toMatchObject() {pass: true} expect({"a": "b", "toString": [Function toString]}).toMatchObject({"toString": Any}) 1`] = ` +expect(received).not.toMatchObject(expected) + +Expected: not {"toString": Any} +Received: {"a": "b", "toString": [Function toString]} +`; + exports[`toMatchObject() {pass: true} expect({"a": "b"}).toMatchObject({"a": "b"}) 1`] = ` expect(received).not.toMatchObject(expected) @@ -4314,6 +4333,13 @@ exports[`toMatchObject() {pass: true} expect({"a": undefined}).toMatchObject({"a Expected: not {"a": undefined} `; +exports[`toMatchObject() {pass: true} expect({}).toMatchObject({"a": undefined, "b": "b", "c": "c"}) 1`] = ` +expect(received).not.toMatchObject(expected) + +Expected: not {"a": undefined, "b": "b", "c": "c"} +Received: {} +`; + exports[`toMatchObject() {pass: true} expect({}).toMatchObject({"a": undefined, "b": "b"}) 1`] = ` expect(received).not.toMatchObject(expected) @@ -4321,6 +4347,13 @@ Expected: not {"a": undefined, "b": "b"} Received: {} `; +exports[`toMatchObject() {pass: true} expect({}).toMatchObject({"d": 4}) 1`] = ` +expect(received).not.toMatchObject(expected) + +Expected: not {"d": 4} +Received: {} +`; + exports[`toMatchObject() {pass: true} expect(2015-11-30T00:00:00.000Z).toMatchObject(2015-11-30T00:00:00.000Z) 1`] = ` expect(received).not.toMatchObject(expected) diff --git a/packages/expect/src/__tests__/matchers.test.js b/packages/expect/src/__tests__/matchers.test.js index 4223f4e53475..232ca8e06ab7 100644 --- a/packages/expect/src/__tests__/matchers.test.js +++ b/packages/expect/src/__tests__/matchers.test.js @@ -1958,6 +1958,22 @@ describe('toMatchObject()', () => { } } + class Sub extends Foo { + get c() { + return 'c'; + } + } + + const withDefineProperty = (obj, key, val) => { + Object.defineProperty(obj, key, { + get() { + return val; + }, + }); + + return obj; + }; + const testNotToMatchSnapshots = tuples => { tuples.forEach(([n1, n2]) => { it(`{pass: true} expect(${stringify(n1)}).toMatchObject(${stringify( @@ -2082,6 +2098,12 @@ describe('toMatchObject()', () => { {a: 'b', c: 'd', [Symbol.for('jest')]: 'jest'}, {a: 'b', c: 'd', [Symbol.for('jest')]: 'jest'}, ], + // These snapshots will show {} as the object because the properties + // are not enumerable. We will need to somehow make the serialization of + // these keys a little smarter before reporting accurately. + [new Sub(), {a: undefined, b: 'b', c: 'c'}], + [withDefineProperty(new Sub(), 'd', 4), {d: 4}], + [{a: 'b', toString() {}}, {toString: jestExpect.any(Function)}], ]); testToMatchSnapshots([ @@ -2129,6 +2151,7 @@ describe('toMatchObject()', () => { {a: 'b', c: 'd', [Symbol.for('jest')]: 'jest'}, {a: 'c', [Symbol.for('jest')]: expect.any(String)}, ], + [{a: 'b'}, {toString: jestExpect.any(Function)}], ]); [ diff --git a/packages/expect/src/__tests__/utils.test.ts b/packages/expect/src/__tests__/utils.test.ts index d5b31b84a16b..c210abf9fb34 100644 --- a/packages/expect/src/__tests__/utils.test.ts +++ b/packages/expect/src/__tests__/utils.test.ts @@ -11,7 +11,6 @@ import { emptyObject, getObjectSubset, getPath, - hasOwnProperty, iterableEquality, subsetEquality, } from '../utils'; @@ -107,46 +106,6 @@ describe('getPath()', () => { }); }); -describe('hasOwnProperty', () => { - it('does inherit getter from class', () => { - class MyClass { - get key() { - return 'value'; - } - } - expect(hasOwnProperty(new MyClass(), 'key')).toBe(true); - }); - - it('does not inherit setter from class', () => { - class MyClass { - set key(_value: unknown) {} - } - expect(hasOwnProperty(new MyClass(), 'key')).toBe(false); - }); - - it('does not inherit method from class', () => { - class MyClass { - key() {} - } - expect(hasOwnProperty(new MyClass(), 'key')).toBe(false); - }); - - it('does not inherit property from constructor prototype', () => { - function MyClass() {} - MyClass.prototype.key = 'value'; - // @ts-expect-error - expect(hasOwnProperty(new MyClass(), 'key')).toBe(false); - }); - - it('does not inherit __proto__ getter from Object', () => { - expect(hasOwnProperty({}, '__proto__')).toBe(false); - }); - - it('does not inherit toString method from Object', () => { - expect(hasOwnProperty({}, 'toString')).toBe(false); - }); -}); - describe('getObjectSubset', () => { [ [{a: 'b', c: 'd'}, {a: 'd'}, {a: 'b'}], diff --git a/packages/expect/src/utils.ts b/packages/expect/src/utils.ts index d386c8ad8edf..e4b60c31c835 100644 --- a/packages/expect/src/utils.ts +++ b/packages/expect/src/utils.ts @@ -21,33 +21,23 @@ type GetPath = { value?: unknown; }; -// Return whether object instance inherits getter from its class. -const hasGetterFromConstructor = (object: object, key: string) => { - const constructor = object.constructor; - if (constructor === Object) { - // A literal object has Object as constructor. - // Therefore, it cannot inherit application-specific getters. - // Furthermore, Object has __proto__ getter which is not relevant. - // Array, Boolean, Number, String constructors don’t have any getters. - return false; - } - if (typeof constructor !== 'function') { - // Object.create(null) constructs object with no constructor nor prototype. - // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/create#Custom_and_Null_objects +/** + * Checks if `hasOwnProperty(object, key)` up the prototype chain, stopping at `Object.prototype`. + */ +const hasPropertyInObject = (object: object, key: string): boolean => { + const shouldTerminate = + !object || typeof object !== 'object' || object === Object.prototype; + + if (shouldTerminate) { return false; } - const descriptor = Object.getOwnPropertyDescriptor( - constructor.prototype, - key, + return ( + Object.prototype.hasOwnProperty.call(object, key) || + hasPropertyInObject(Object.getPrototypeOf(object), key) ); - return descriptor !== undefined && typeof descriptor.get === 'function'; }; -export const hasOwnProperty = (object: object, key: string): boolean => - Object.prototype.hasOwnProperty.call(object, key) || - hasGetterFromConstructor(object, key); - export const getPath = ( object: Record, propertyPath: string | Array, @@ -129,7 +119,7 @@ export const getObjectSubset = ( seenReferences.set(object, trimmed); Object.keys(object) - .filter(key => hasOwnProperty(subset, key)) + .filter(key => hasPropertyInObject(subset, key)) .forEach(key => { trimmed[key] = seenReferences.has(object[key]) ? seenReferences.get(object[key]) @@ -299,7 +289,7 @@ export const subsetEquality = ( } const result = object != null && - hasOwnProperty(object, key) && + hasPropertyInObject(object, key) && equals(object[key], subset[key], [ iterableEquality, subsetEqualityWithContext(seenReferences),