From 1a3b2803b75dbdda2d3b315a9531f8a66be77fa2 Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Wed, 1 May 2019 17:15:57 -0400 Subject: [PATCH 1/5] Reapply #7089 --- .../jest-snapshot/src/__tests__/utils.test.ts | 80 ++++++++++++++++++- packages/jest-snapshot/src/utils.ts | 17 ++++ 2 files changed, 93 insertions(+), 4 deletions(-) diff --git a/packages/jest-snapshot/src/__tests__/utils.test.ts b/packages/jest-snapshot/src/__tests__/utils.test.ts index 93193ff5977c..f28efd12128d 100644 --- a/packages/jest-snapshot/src/__tests__/utils.test.ts +++ b/packages/jest-snapshot/src/__tests__/utils.test.ts @@ -9,6 +9,7 @@ jest.mock('fs'); import fs from 'fs'; import path from 'path'; +import assert from 'assert'; import chalk from 'chalk'; import { @@ -201,14 +202,85 @@ test('serialize handles \\r\\n', () => { describe('DeepMerge', () => { it('Correctly merges objects with property matchers', () => { - const target = {data: {bar: 'bar', foo: 'foo'}}; + /* eslint-disable sort-keys */ + // to keep keys in numerical order rather than alphabetical + const target = { + data: { + one: 'one', + two: 'two', + three: [ + { + four: 'four', + five: 'five', + }, + // Include an array element not present in the propertyMatchers + { + six: 'six', + seven: 'seven', + }, + ], + eight: [{nine: 'nine'}], + }, + }; const matcher = expect.any(String); - const propertyMatchers = {data: {foo: matcher}}; + const propertyMatchers = { + data: { + two: matcher, + three: [ + { + four: matcher, + }, + ], + eight: [ + {nine: matcher}, + // Include an array element not present in the target + {ten: matcher}, + ], + }, + }; const mergedOutput = deepMerge(target, propertyMatchers); - expect(mergedOutput).toStrictEqual({data: {bar: 'bar', foo: matcher}}); - expect(target).toStrictEqual({data: {bar: 'bar', foo: 'foo'}}); + // Use assert.deepStrictEqual() instead of expect().toStrictEqual() + // since we want to actually validate that we got the matcher + // rather than treat it specially the way that expect() does + assert.deepStrictEqual(mergedOutput, { + data: { + one: 'one', + two: matcher, + three: [ + { + four: matcher, + five: 'five', + }, + { + six: 'six', + seven: 'seven', + }, + ], + eight: [{nine: matcher}, {ten: matcher}], + }, + }); + + // Ensure original target is not modified + expect(target).toStrictEqual({ + data: { + one: 'one', + two: 'two', + three: [ + { + four: 'four', + five: 'five', + }, + { + six: 'six', + seven: 'seven', + }, + ], + eight: [{nine: 'nine'}], + }, + }); + /* eslint-enable sort-keys */ }); }); diff --git a/packages/jest-snapshot/src/utils.ts b/packages/jest-snapshot/src/utils.ts index 992fa72926d0..d0e4b20eb049 100644 --- a/packages/jest-snapshot/src/utils.ts +++ b/packages/jest-snapshot/src/utils.ts @@ -178,6 +178,21 @@ export const saveSnapshotFile = ( ); }; +const deepMergeArray = (target: Array, source: Array) => { + // Clone target + const mergedOutput = target.slice(); + + source.forEach((element, index) => { + if (typeof mergedOutput[index] === 'undefined') { + mergedOutput[index] = element; + } else { + mergedOutput[index] = deepMerge(target[index], element); + } + }); + + return mergedOutput; +}; + export const deepMerge = (target: any, source: any) => { const mergedOutput = {...target}; if (isObject(target) && isObject(source)) { @@ -185,6 +200,8 @@ export const deepMerge = (target: any, source: any) => { if (isObject(source[key]) && !source[key].$$typeof) { if (!(key in target)) Object.assign(mergedOutput, {[key]: source[key]}); else mergedOutput[key] = deepMerge(target[key], source[key]); + } else if (Array.isArray(source[key])) { + mergedOutput[key] = deepMergeArray(target[key], source[key]); } else { Object.assign(mergedOutput, {[key]: source[key]}); } From af7261b0717e971e5997a1a659787e3d6e918ef9 Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Wed, 1 May 2019 17:34:00 -0400 Subject: [PATCH 2/5] handle simple arrays and arrays of arrays --- .../jest-snapshot/src/__tests__/utils.test.ts | 208 +++++++++++------- packages/jest-snapshot/src/utils.ts | 16 +- 2 files changed, 141 insertions(+), 83 deletions(-) diff --git a/packages/jest-snapshot/src/__tests__/utils.test.ts b/packages/jest-snapshot/src/__tests__/utils.test.ts index f28efd12128d..9283eaaf9838 100644 --- a/packages/jest-snapshot/src/__tests__/utils.test.ts +++ b/packages/jest-snapshot/src/__tests__/utils.test.ts @@ -200,87 +200,137 @@ test('serialize handles \\r\\n', () => { expect(serializedData).toBe('\n"
\n
"\n'); }); -describe('DeepMerge', () => { - it('Correctly merges objects with property matchers', () => { - /* eslint-disable sort-keys */ - // to keep keys in numerical order rather than alphabetical - const target = { - data: { - one: 'one', - two: 'two', - three: [ - { - four: 'four', - five: 'five', - }, - // Include an array element not present in the propertyMatchers - { - six: 'six', - seven: 'seven', - }, - ], - eight: [{nine: 'nine'}], +describe('DeepMerge with property matchers', () => { + const matcher = expect.any(String); + + /* eslint-disable sort-keys */ + // to keep keys in numerical order rather than alphabetical + const cases = [ + [ + 'a nested object', + // Target + { + data: { + one: 'one', + two: 'two', + }, + }, + // Matchers + { + data: { + two: matcher, + }, + }, + // Expected + { + data: { + one: 'one', + two: matcher, + }, + }, + ], + + [ + 'an object with an array of objects', + // Target + { + data: { + one: [ + { + two: 'two', + three: 'three', + }, + // Include an array element not present in the propertyMatchers + { + four: 'four', + five: 'five', + }, + ], + six: [{seven: 'seven'}], + nine: [[{ten: 'ten'}]], + }, + }, + // Matchers + { + data: { + one: [ + { + two: matcher, + }, + ], + six: [ + {seven: matcher}, + // Include an array element not present in the target + {eight: matcher}, + ], + nine: [[{ten: matcher}]], + }, }, - }; - - const matcher = expect.any(String); - const propertyMatchers = { - data: { - two: matcher, - three: [ - { - four: matcher, - }, - ], - eight: [ - {nine: matcher}, - // Include an array element not present in the target - {ten: matcher}, - ], + // Expected + { + data: { + one: [ + { + two: matcher, + three: 'three', + }, + { + four: 'four', + five: 'five', + }, + ], + six: [{seven: matcher}, {eight: matcher}], + nine: [[{ten: matcher}]], + }, }, - }; - - const mergedOutput = deepMerge(target, propertyMatchers); - - // Use assert.deepStrictEqual() instead of expect().toStrictEqual() - // since we want to actually validate that we got the matcher - // rather than treat it specially the way that expect() does - assert.deepStrictEqual(mergedOutput, { - data: { - one: 'one', - two: matcher, - three: [ - { - four: matcher, - five: 'five', - }, - { - six: 'six', - seven: 'seven', - }, - ], - eight: [{nine: matcher}, {ten: matcher}], + ], + + [ + 'an object with an array of strings', + // Target + { + data: { + one: ['one'], + two: ['two'], + three: ['three', 'four'], + five: ['five'], + }, + }, + // Matchers + { + data: { + one: [matcher], + two: ['two'], + three: [matcher], + five: 'five', + }, }, - }); - - // Ensure original target is not modified - expect(target).toStrictEqual({ - data: { - one: 'one', - two: 'two', - three: [ - { - four: 'four', - five: 'five', - }, - { - six: 'six', - seven: 'seven', - }, - ], - eight: [{nine: 'nine'}], + // Expected + { + data: { + one: [matcher], + two: ['two'], + three: [matcher, 'four'], + five: 'five', + }, }, - }); - /* eslint-enable sort-keys */ - }); + ], + ]; + /* eslint-enable sort-keys */ + + it.each(cases)( + 'Correctly merges %s', + (_case, target, propertyMatchers, expected) => { + const originalTarget = JSON.parse(JSON.stringify(target)); + const mergedOutput = deepMerge(target, propertyMatchers); + + // Use assert.deepStrictEqual() instead of expect().toStrictEqual() + // since we want to actually validate that we got the matcher + // rather than treat it specially the way that expect() does + assert.deepStrictEqual(mergedOutput, expected); + + // Ensure original target is not modified + expect(target).toStrictEqual(originalTarget); + }, + ); }); diff --git a/packages/jest-snapshot/src/utils.ts b/packages/jest-snapshot/src/utils.ts index d0e4b20eb049..1ac37a1f4c4f 100644 --- a/packages/jest-snapshot/src/utils.ts +++ b/packages/jest-snapshot/src/utils.ts @@ -182,11 +182,19 @@ const deepMergeArray = (target: Array, source: Array) => { // Clone target const mergedOutput = target.slice(); - source.forEach((element, index) => { - if (typeof mergedOutput[index] === 'undefined') { - mergedOutput[index] = element; + source.forEach((sourceElement, index) => { + const targetElement = mergedOutput[index]; + const targetElementType = typeof targetElement; + + if (targetElementType === 'undefined' || targetElementType !== 'object') { + // Source does not exist in target or target is primitive and cannot be deep merged + mergedOutput[index] = sourceElement; + } else if (Array.isArray(target[index])) { + // Target is an array + mergedOutput[index] = deepMergeArray(target[index], sourceElement); } else { - mergedOutput[index] = deepMerge(target[index], element); + // Target is an object - recursively merge + mergedOutput[index] = deepMerge(target[index], sourceElement); } }); From c9d1aff271948f2a2e33956bda50821e1d099b06 Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Wed, 1 May 2019 18:03:06 -0400 Subject: [PATCH 3/5] remove unecessary check and reorder for positive checks instead of negative ones --- packages/jest-snapshot/src/utils.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/jest-snapshot/src/utils.ts b/packages/jest-snapshot/src/utils.ts index 1ac37a1f4c4f..b3500a59bd47 100644 --- a/packages/jest-snapshot/src/utils.ts +++ b/packages/jest-snapshot/src/utils.ts @@ -186,15 +186,15 @@ const deepMergeArray = (target: Array, source: Array) => { const targetElement = mergedOutput[index]; const targetElementType = typeof targetElement; - if (targetElementType === 'undefined' || targetElementType !== 'object') { - // Source does not exist in target or target is primitive and cannot be deep merged - mergedOutput[index] = sourceElement; - } else if (Array.isArray(target[index])) { + if (Array.isArray(target[index])) { // Target is an array mergedOutput[index] = deepMergeArray(target[index], sourceElement); - } else { - // Target is an object - recursively merge + } else if (targetElementType === 'object') { + // Target is a (non-array) object - recursively merge mergedOutput[index] = deepMerge(target[index], sourceElement); + } else { + // Source does not exist in target or target is primitive and cannot be deep merged + mergedOutput[index] = sourceElement; } }); From 4e296346765a86dd51a0ff6758c65082584595d1 Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Tue, 11 Jun 2019 15:06:12 -0400 Subject: [PATCH 4/5] use isObject --- packages/jest-snapshot/src/utils.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/jest-snapshot/src/utils.ts b/packages/jest-snapshot/src/utils.ts index b3500a59bd47..184e24f737f0 100644 --- a/packages/jest-snapshot/src/utils.ts +++ b/packages/jest-snapshot/src/utils.ts @@ -184,12 +184,11 @@ const deepMergeArray = (target: Array, source: Array) => { source.forEach((sourceElement, index) => { const targetElement = mergedOutput[index]; - const targetElementType = typeof targetElement; if (Array.isArray(target[index])) { // Target is an array mergedOutput[index] = deepMergeArray(target[index], sourceElement); - } else if (targetElementType === 'object') { + } else if (isObject(targetElement)) { // Target is a (non-array) object - recursively merge mergedOutput[index] = deepMerge(target[index], sourceElement); } else { From 9dbc98fc7f2366b881470fecae55eedc0f9bb7a8 Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Thu, 13 Jun 2019 11:20:25 -0400 Subject: [PATCH 5/5] PR comments - remove frivolous comments, use Array.from(target) --- packages/jest-snapshot/src/utils.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/jest-snapshot/src/utils.ts b/packages/jest-snapshot/src/utils.ts index b68aa9a5c863..1a1524d2e039 100644 --- a/packages/jest-snapshot/src/utils.ts +++ b/packages/jest-snapshot/src/utils.ts @@ -179,17 +179,14 @@ export const saveSnapshotFile = ( }; const deepMergeArray = (target: Array, source: Array) => { - // Clone target - const mergedOutput = target.slice(); + const mergedOutput = Array.from(target); source.forEach((sourceElement, index) => { const targetElement = mergedOutput[index]; if (Array.isArray(target[index])) { - // Target is an array mergedOutput[index] = deepMergeArray(target[index], sourceElement); } else if (isObject(targetElement)) { - // Target is a (non-array) object - recursively merge mergedOutput[index] = deepMerge(target[index], sourceElement); } else { // Source does not exist in target or target is primitive and cannot be deep merged