From 343bdb16cb66c9fae4070ea5fdd194c2979dad83 Mon Sep 17 00:00:00 2001
From: Zaytsev Kirill <zaytsev.cmath10@gmail.com>
Date: Sun, 30 May 2021 19:18:37 +0300
Subject: [PATCH] refactor: Improved logic of clone utility

---
 src/utils/clone.ts            | 25 +++++-----
 test/unit/utils/clone.test.js | 86 +++++++++++++++++++++--------------
 2 files changed, 65 insertions(+), 46 deletions(-)

diff --git a/src/utils/clone.ts b/src/utils/clone.ts
index a66afca..88d83ef 100644
--- a/src/utils/clone.ts
+++ b/src/utils/clone.ts
@@ -1,9 +1,12 @@
 import has from '@/utils/has'
-import { RecordLike, Scalar, isScalar } from '@/types'
+import {
+    RecordLike,
+    Scalar,
+    isRecordLike,
+    isScalar,
+} from '@/types'
 
-type Cloneable = Scalar|Date|RecordLike<unknown>
-
-export const cloneInstance = <T>(original: T): T => {
+const cloneInstance = <T>(original: T): T => {
     return Object.assign(Object.create(Object.getPrototypeOf(original)), original)
 }
 
@@ -11,7 +14,7 @@ export const cloneInstance = <T>(original: T): T => {
  * A simple (somewhat non-comprehensive) clone function, valid for our use
  * case of needing to unbind reactive watchers.
  */
-export default function clone (value: Cloneable): Cloneable {
+export default function clone (value: unknown): unknown {
     if (isScalar(value)) {
         return value as Scalar
     }
@@ -20,18 +23,16 @@ export default function clone (value: Cloneable): Cloneable {
         return new Date(value)
     }
 
+    if (!isRecordLike(value)) {
+        return cloneInstance(value)
+    }
+
     const source: RecordLike<unknown> = value as RecordLike<unknown>
     const copy: RecordLike<unknown> = Array.isArray(source) ? [] : {}
 
     for (const key in source) {
         if (has(source, key)) {
-            if (isScalar(source[key])) {
-                copy[key] = source[key]
-            } else if (source[key] instanceof Date) {
-                copy[key] = new Date(source[key] as Date)
-            } else {
-                copy[key] = clone(source[key] as Cloneable)
-            }
+            copy[key] = clone(source[key])
         }
     }
 
diff --git a/test/unit/utils/clone.test.js b/test/unit/utils/clone.test.js
index e37567c..bccaa08 100644
--- a/test/unit/utils/clone.test.js
+++ b/test/unit/utils/clone.test.js
@@ -1,45 +1,59 @@
-import clone, { cloneInstance } from '@/utils/clone'
+import clone from '@/utils/clone'
+
+class Sample {
+    constructor() {
+        this.fieldA = 'fieldA'
+        this.fieldB = 'fieldB'
+    }
+
+    doSomething () {}
+}
 
 describe('clone', () => {
-    test('Basic objects stay the same', () => {
-        const obj = { a: 123, b: 'hello' }
-        expect(clone(obj)).toEqual(obj)
+    test.each([
+        [{ a: 123, b: 'hello' }],
+        [{ a: 123, b: { c: 'hello-world' } }],
+        [{
+            id: 123,
+            addresses: [{
+                street: 'Baker Street',
+                building: '221b',
+            }],
+        }],
+    ])('recreates object, preserving its structure', state => {
+        expect(clone(state)).toEqual(state)
+        expect(clone({ ref: state }).ref === state).toBe(false)
     })
 
-    test('Basic nested objects stay the same', () => {
-        const obj = { a: 123, b: { c: 'hello-world' } }
-        expect(clone(obj)).toEqual(obj)
-    })
-
-    test('Simple pojo reference types are re-created', () => {
-        const c = { c: 'hello-world' }
-        expect(clone({ a: 123, b: c }).b === c).toBe(false)
-    })
-
-    test('Retains array structures inside of a pojo', () => {
+    test('retains array structures inside of a pojo', () => {
         const obj = { a: 'abc', d: ['first', 'second'] }
         expect(Array.isArray(clone(obj).d)).toBe(true)
     })
 
-    test('Removes references inside array structures', () => {
+    test('removes references inside array structures', () => {
         const obj = { a: 'abc', d: ['first', { foo: 'bar' }] }
         expect(clone(obj).d[1] === obj.d[1]).toBe(false)
     })
-})
 
-describe('cloneInstance', () => {
+    test('creates a copy of a date', () => {
+        const date = new Date()
+        const copy = clone(date)
+
+        expect(date === copy).toBeFalsy()
+        expect(copy.toISOString()).toStrictEqual(date.toISOString())
+    })
+
+    test('creates a copy of a nested date', () => {
+        const date = new Date()
+        const copy = clone({ date })
+
+        expect(date === copy.date).toBeFalsy()
+        expect(copy.date.toISOString()).toStrictEqual(date.toISOString())
+    })
+
     test('creates a copy of a class instance', () => {
-        class Sample {
-            constructor() {
-                this.fieldA = 'fieldA'
-                this.fieldB = 'fieldB'
-            }
-
-            doSomething () {}
-        }
-
         const sample = new Sample()
-        const copy = cloneInstance(sample)
+        const copy = clone(sample)
 
         expect(sample === copy).toBeFalsy()
 
@@ -50,12 +64,16 @@ describe('cloneInstance', () => {
         expect(copy.doSomething).not.toThrow()
     })
 
-    test('creates a broken copy of builtins', () => {
-        const sample = new Date()
-        const copy = cloneInstance(sample)
+    test('creates a copy of a nested class instance', () => {
+        const sample = new Sample()
+        const copy = clone({ sample })
 
-        expect(sample === copy).toBeFalsy()
-        expect(copy).toBeInstanceOf(Date)
-        expect(() => copy.toISOString()).toThrow()
+        expect(sample === copy.sample).toBeFalsy()
+
+        expect(copy.sample).toBeInstanceOf(Sample)
+        expect(copy.sample.fieldA).toEqual('fieldA')
+        expect(copy.sample.fieldB).toEqual('fieldB')
+        expect(copy.sample.doSomething).toBeTruthy()
+        expect(copy.sample.doSomething).not.toThrow()
     })
 })