chore(suite): use structuredClone for deep copying #6071
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| export const deepClone = <T>(value: T): T => { | ||
|
There was a problem hiding this comment. I would suggest little bit restrict values that can be passed, because you can pass anything now I think, but I am not sure how JSON will behave if you will pass object with BigInt or other special types. It will work for structuredClone but JSON could mess it up, so we should limit it only types that will work 100% for both solutions. There was a problem hiding this comment. And now we are back to square 1 from which the conversation has started hehe :D Yeah, makes sense. There was a problem hiding this comment. I agree that we should consider that objects are going to be stringified when using const appointment = {
location: 'Prague',
day: new Date(),
}
const whenStructuredClone = structuredClone(appointment)
whenStructuredClone.day.getDate()
// result -> 25
const whenStringified = JSON.parse(JSON.stringify(appointment))
whenStringified.day.getDate()
// result -> Uncaught TypeError: whenStringified.day.getDate is not a functionAnd the same for nested objects. And the same behavior can be expected from other objects like Restricting the use to types that will work 100% for both could be a solution but if I am not mistaken I think it should iterate over the whole object and nested potential objects to make sure there is no nested There was a problem hiding this comment. I think type like this should do the job: import { JsonValue } from 'type-fest' |
||
| if (value === undefined) { | ||
| return value; | ||
| } | ||
|
|
||
| let clone; | ||
|
|
||
| if (typeof structuredClone === 'function') { | ||
| clone = structuredClone(value); | ||
| } else { | ||
| clone = JSON.parse(JSON.stringify(value)); | ||
|
There was a problem hiding this comment. We should probably use some better polyfill than this. There are already available. There was a problem hiding this comment. Never heard of it before, core-js has a polyfill. (I know we are cautious about included new dependencies, but just sharing). |
||
| } | ||
|
|
||
| return clone; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| import { deepClone } from '../src/deepClone'; | ||
|
|
||
| describe('deepClone', () => { | ||
| it('deepClone works', () => { | ||
| const original = { | ||
| a: 1, | ||
| b: '2', | ||
| c: { | ||
| a: 1, | ||
| b: '2', | ||
| c: { | ||
| a: [1, 2, 3], | ||
| b: '2', | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| const copy = deepClone(original); | ||
| const shallowCopy = { ...original }; | ||
|
|
||
| expect(copy.c.c.a === original.c.c.a).toBeFalsy(); | ||
| expect(shallowCopy.c.c.a === original.c.c.a).toBeTruthy(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are ~ 12 more places where
JSON.parse(JSON.stringify...is used, I guess it would be nice to use this util on all those places?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
da91863