Add 'cloneData' option to buildMemoryStorage

[bhallionOhbibi]

See issue 580 for more details

[codecov]

Codecov Report

Merging #581 (2e2f9cd) into main (c91e6a0) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #581   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines          576       578    +2     
  Branches       188       190    +2     
=========================================
+ Hits           576       578    +2     

Impacted Files	Coverage Δ
src/storage/memory.ts	100.00% <100.00%> (ø)

[bhallionOhbibi]

Looks like there is some issue with the CI

[arthurfiorette]

The CI failed because of linting, please run pnpm lint -- --fix and pnpm format before push to lint your code.

[arthurfiorette]

I added a review to update the cloneData property instead of creating a new one. Overall, suuper thanks for your time in this PR :)

[bhallionOhbibi]

Thanks for your review. This is the first time writing typescript code. As yo ucan expect, my local configuration is not exactly mature right now 😅

I updated the test, but I'm not sure if this is enough, please let me know what you think.

About storeClone:
I removed it. It was an attempt to be a bit more explicit, as I find 'double' on cloneData less explicit. In the other hand, 'double' on cloneData limit amount of parameters and allow us to use the new option without adding cleanupInterval and maxEntries to false every time.
You decide what's best for this library and adding 'double' to cloneData seems like a fine typescript way of adding functionnality.

Another note about this whole clone thing. I really think that the double options should be the default value. I would argue that anyone trying this library for the first time might have a hard time understanding why their axios.get calls doesnt returns the same data every time. And suddenly having to check for cached property or having to provide additionnal configuration might block some users from using the library altogether.

[arthurfiorette]

adding 'double' to cloneData seems like a fine typescript way

Yeah! I'm just not satisfied enough with the double name, any synonyms comes to your mind?

I really think that the double options should be the default value.

They may be, but i'd only make them default on the next major version as it will be a breaking change... Also, I'm not sure how the GC will behave on data being cloned everywhere.

[bhallionOhbibi]

Instead of double I suggested all.

But it would be more explicit and also more configurable to use an enum.
Perhaps something like:

• onSet
• onGet

Where the user can specify one, both or none of the options (so an array of string maybe ?).
I think this aproach is the best, it wont break future compatibility if you want to add clone to another method.
Let the user specify: ['onSet', 'onGet']
And you can leave the default behaviour where true means onGet for retro-compatiblity.

I dont know how one would neatly implement this in typescript tho.

[arthurfiorette]

some linting

[arthurfiorette]

Thanks!