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

Fix diffing object contain readonly symbol key object #10414

Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Fixes

- `[jest-matcher-utils]` Fix diffing object contain readonly symbol key object ([#10414](https://github.com/facebook/jest/pull/10414))
- `[jest-reporters]` Fixes notify reporter on Linux (using notify-send) ([#10393](https://github.com/facebook/jest/pull/10400))
- `[jest-snapshot]` Correctly handles arrays and property matchers in snapshots ([#10404](https://github.com/facebook/jest/pull/10404))

Expand Down
51 changes: 51 additions & 0 deletions e2e/__tests__/__snapshots__/domDiffing.test.ts.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`should work without error 1`] = `
"FAIL __tests__/dom.test.js
✕ use toBe compare two div (<<REPLACED>>)
✕ compare span and div (<<REPLACED>>)

● use toBe compare two div

expect(received).toBe(expected) // Object.is equality

If it should pass with deep equality, replace \\"toBe\\" with \\"toStrictEqual\\"

Expected: <div />
Received: serializes to the same string

12 | const div1 = document.createElement('div');
13 | const div2 = document.createElement('div');
> 14 | expect(div1).toBe(div2);
| ^
15 | });
16 |
17 | test('compare span and div', () => {

at Object.toBe (__tests__/dom.test.js:14:16)

● compare span and div

expect(received).toBe(expected) // Object.is equality

- Expected - 1
+ Received + 1

- <span />
+ <div />

16 |
17 | test('compare span and div', () => {
> 18 | expect(document.createElement('div')).toBe(document.createElement('span'));
| ^
19 | });
20 |

at Object.toBe (__tests__/dom.test.js:18:41)

Test Suites: 1 failed, 1 total
Tests: 2 failed, 2 total
Snapshots: 0 total
Time: <<REPLACED>>
Ran all test suites."
`;
16 changes: 16 additions & 0 deletions e2e/__tests__/domDiffing.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
*/

import runJest from '../runJest';
import {replaceTime} from '../Utils';

test('should work without error', () => {
const output = runJest('dom-diffing');
expect(output.failed).toBe(true);
expect(replaceTime(output.stderr)).toMatchSnapshot();
});
19 changes: 19 additions & 0 deletions e2e/dom-diffing/__tests__/dom.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
*/

/* eslint-env browser */

test('use toBe compare two div', () => {
const div1 = document.createElement('div');
const div2 = document.createElement('div');
expect(div1).toBe(div2);
});

test('compare span and div', () => {
expect(document.createElement('div')).toBe(document.createElement('span'));
});
3 changes: 3 additions & 0 deletions e2e/dom-diffing/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"jest": {}
}
17 changes: 11 additions & 6 deletions packages/jest-matcher-utils/src/Replaceable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,17 @@ export default class Replaceable {

forEach(cb: ReplaceableForEachCallBack): void {
if (this.type === 'object') {
Object.entries(this.object).forEach(([key, value]) => {
cb(value, key, this.object);
});
Object.getOwnPropertySymbols(this.object).forEach(key => {
cb(this.object[key], key, this.object);
});
const descriptors = Object.getOwnPropertyDescriptors(this.object);
[
...Object.keys(descriptors),
...Object.getOwnPropertySymbols(descriptors),
]
//@ts-expect-error because typescript do not support symbol key in object
//https://github.com/microsoft/TypeScript/issues/1863
.filter(key => descriptors[key].enumerable)
.forEach(key => {
cb(this.object[key], key, this.object);
});
} else {
this.object.forEach(cb);
}
Expand Down
24 changes: 24 additions & 0 deletions packages/jest-matcher-utils/src/__tests__/Replaceable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,30 @@ describe('Replaceable', () => {
expect(cb.mock.calls[0]).toEqual([1, 'a', map]);
expect(cb.mock.calls[1]).toEqual([2, 'b', map]);
});

test('forEach should ignore nonenumerable property', () => {
const symbolKey = Symbol('jest');
const symbolKey2 = Symbol('awesome');
const object = {a: 1, [symbolKey]: 3};
Object.defineProperty(object, 'b', {
configurable: true,
enumerable: false,
value: 2,
writable: true,
});
Object.defineProperty(object, symbolKey2, {
configurable: true,
enumerable: false,
value: 4,
writable: true,
});
const replaceable = new Replaceable(object);
const cb = jest.fn();
replaceable.forEach(cb);
expect(cb).toHaveBeenCalledTimes(2);
expect(cb.mock.calls[0]).toEqual([1, 'a', object]);
expect(cb.mock.calls[1]).toEqual([3, symbolKey, object]);
});
});

describe('isReplaceable', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,3 +198,16 @@ exports[`printDiffOrStringify has no common after clean up chaff one-line 1`] =
Expected: <g>"delete"</>
Received: <r>"insert"</>
`;

exports[`printDiffOrStringify object contain readonly symbol key object 1`] = `
<g>- Expected - 1</>
<r>+ Received + 1</>

<d> Object {</>
<g>- "b": 2,</>
<r>+ "b": 1,</>
<d> Symbol(key): Object {</>
<d> "a": 1,</>
<d> },</>
<d> }</>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,20 @@ test('convert accessor descriptor into value descriptor', () => {
});
});

test('skips non-enumerables', () => {
test('shuold not skips non-enumerables', () => {
const obj = {};
Object.defineProperty(obj, 'foo', {enumerable: false, value: 'bar'});

const copy = deepCyclicCopyReplaceable(obj);

expect(Object.getOwnPropertyDescriptors(copy)).toEqual({});
expect(Object.getOwnPropertyDescriptors(copy)).toEqual({
foo: {
configurable: true,
enumerable: false,
value: 'bar',
writable: true,
},
});
});

test('copies symbols', () => {
Expand Down Expand Up @@ -113,3 +120,22 @@ test('return same value for built-in object type except array, map and object',
expect(deepCyclicCopyReplaceable(regexp)).toBe(regexp);
expect(deepCyclicCopyReplaceable(set)).toBe(set);
});

test('should copy object symbol key property', () => {
const symbolKey = Symbol.for('key');
expect(deepCyclicCopyReplaceable({[symbolKey]: 1})).toEqual({[symbolKey]: 1});
});

test('should set writable, configurable to true', () => {
const a = {};
Object.defineProperty(a, 'key', {
configurable: false,
enumerable: true,
value: 1,
writable: false,
});
const copied = deepCyclicCopyReplaceable(a);
expect(Object.getOwnPropertyDescriptors(copied)).toEqual({
key: {configurable: true, enumerable: true, value: 1, writable: true},
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @jest-environment jsdom
*/
/* eslint-env browser*/
import deepCyclicCopyReplaceable from '../deepCyclicCopyReplaceable';

test('should copy dom element', () => {
const div = document.createElement('div');
const copied = deepCyclicCopyReplaceable(div);
expect(copied).toEqual(div);
expect(div === copied).toBe(false); //assert reference is not the same
});

test('should copy complex element', () => {
const div = document.createElement('div');
const span = document.createElement('span');
div.setAttribute('id', 'div');
div.innerText = 'this is div';
div.appendChild(span);
const copied = deepCyclicCopyReplaceable(div);
expect(copied).toEqual(div);
expect(div === copied).toBe(false); //assert reference is not the same
expect(div.children[0] === copied.children[0]).toBe(false); //assert reference is not the same
});
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,29 @@ describe('printDiffOrStringify', () => {
expect(testDiffOrStringify(expected, received)).toMatchSnapshot();
});

test('object contain readonly symbol key object', () => {
const expected = {b: 2};
const received = {b: 1};
const symbolKey = Symbol.for('key');
Object.defineProperty(expected, symbolKey, {
configurable: true,
enumerable: true,
value: {
a: 1,
},
writable: false,
});
Object.defineProperty(received, symbolKey, {
configurable: true,
enumerable: true,
value: {
a: 1,
},
writable: false,
});
expect(testDiffOrStringify(expected, received)).toMatchSnapshot();
});

describe('MAX_DIFF_STRING_LENGTH', () => {
const lessChange = INVERTED_COLOR('single ');
const less = 'single line';
Expand Down
34 changes: 23 additions & 11 deletions packages/jest-matcher-utils/src/deepCyclicCopyReplaceable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* LICENSE file in the root directory of this source tree.
*/

import {plugins} from 'pretty-format';

const builtInObject = [
Array,
Buffer,
Expand Down Expand Up @@ -42,6 +44,8 @@ export default function deepCyclicCopyReplaceable<T>(
return deepCyclicCopyMap(value, cycles);
} else if (isBuiltInObject(value)) {
return value;
} else if (plugins.DOMElement.test(value)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} else if (plugins.DOMElement.test(value)) {
} else if (typeof value.cloneNode === 'function') {

maybe? not sure if it's better or not

return (((value as unknown) as Element).cloneNode(true) as unknown) as T;
} else {
return deepCyclicCopyObject(value, cycles);
}
Expand All @@ -55,25 +59,33 @@ function deepCyclicCopyObject<T>(object: T, cycles: WeakMap<any, any>): T {

cycles.set(object, newObject);

Object.keys(descriptors).forEach(key => {
if (descriptors[key].enumerable) {
descriptors[key] = {
const newDescriptors = [
...Object.keys(descriptors),
...Object.getOwnPropertySymbols(descriptors),
].reduce(
//@ts-expect-error because typescript do not support symbol key in object
//https://github.com/microsoft/TypeScript/issues/1863
(newDescriptors: {[x: string]: PropertyDescriptor}, key: string) => {
const enumerable = descriptors[key].enumerable;

newDescriptors[key] = {
configurable: true,
enumerable: true,
enumerable,
value: deepCyclicCopyReplaceable(
// this accesses the value or getter, depending. We just care about the value anyways, and this allows us to not mess with accessors
// it has the side effect of invoking the getter here though, rather than copying it over
(object as Record<string, unknown>)[key],
(object as Record<string | symbol, unknown>)[key],
cycles,
),
writable: true,
};
} else {
delete descriptors[key];
}
});

return Object.defineProperties(newObject, descriptors);
return newDescriptors;
},
{},
);
//@ts-expect-error because typescript do not support symbol key in object
//https://github.com/microsoft/TypeScript/issues/1863
return Object.defineProperties(newObject, newDescriptors);
}

function deepCyclicCopyArray<T>(array: Array<T>, cycles: WeakMap<any, any>): T {
Expand Down