Clone all objects passing in-to and out-of the in memory store #150
Conversation
|
Taking review ☝️ |
| }; | ||
|
|
||
| MemoryStore.prototype._clone = function(obj) { | ||
| return JSON.parse(JSON.stringify(obj)); |
There was a problem hiding this comment.
Are we sure we never have circular references?
There was a problem hiding this comment.
This data is intended to be serialised at this point for storage. If anything does have a circular reference at this point, it's going to break for real when the memory handler gets replaced for a real storage mechanism?
There was a problem hiding this comment.
Since we don't really need the MemoryStore object instance here, I am wondering if this shouldn't simply me a module level helper function, i.e., MemoryStore._clone. Alternatively it could also be implemented as a resource method...
| }); | ||
| }; | ||
|
|
||
| MemoryStore._clone = function(obj) { |
There was a problem hiding this comment.
Any reason for not using lodash's cloneDeep?
There was a problem hiding this comment.
It'd mean adding another dependency for something we really don't need.
|
Minor comment, otherwise it's looking good 👍 |
|
👍 |
I'm so happy you've implemented that 
We've had some issues with stubbing journey tests via the memory handler, and they boil down to how the memory handler doesn't clone objects passing through it. This PR ensures all objects get cloned to allow us to mutate resources without effecting the data in the store.