Use cloneDeep to make deep copies of filters

[grimasod]

Related issues

Short description and why it's useful

Some search filters contain arrays, so they need to be deep copied, to avoid polluting the config object. Previously filters were cloned with Object.assign(), but this results in arrays being copied by reference. Now using lodash's cloneDeep.

Screenshots of visual changes before/after (if there are any)

Which environment this relates to

Check your case. In case of any doubts please read about Release Cycle

• Test version (https://test.storefrontcloud.io) - this is a new feature or improvement for Vue Storefront. I've created branch from develop branch and want to merge it back to develop
• RC version (https://next.storefrontcloud.io) - this is a stabilisation fix for Release Candidate of Vue Storefront. I've created branch from release branch and want to merge it back to release
• Stable version (https://demo.storefrontcloud.io) - this is an important fix for current stable version. I've created branch from hotfix or master branch and want to merge it back to hotfix

Upgrade Notes and Changelog

• No upgrade steps required (100% backward compatibility and no breaking changes)
• I've updated the Upgrade notes and Changelog on how to port existing VS sites with this new feature

IMPORTANT NOTICE - Remember to update CHANGELOG.md with description of your change

Contribution and currently important rules acceptance

• I read and followed contribution rules

[andrzejewsky]

Actually I don't really like cloneDeep because it's totally inefficient and if you need to use it in so many places that mean's you did something wrong... and your code should be refactored. Of course in the VSF context it means refactor sometimes really big piece of code with using code that doesn't cause mutation such as maps, filters, reduces, object destructing, always returning new object etc.

On the other hand I really know how these references are problematic and I have used cloneDeep few times so far, because it was just easier in comparison to figure out with wider refactoring. For now we have to keep in mind that we have so many references and let's try to get rid of them.

PS. I left comments, I think in each case you can use just one cloneDeep.

[grimasod]

Yeah, I understand. The issue comes from passing around the Config object, assigning parts of it to other variables and then passing those vars to other functions. It becomes very difficult to determine where the mutations are happening.

Of course the Config object should be absolutely read-only. When it's shallow copied, using Object.assign() or destructuring, then there's always a chance of causing problems. So, wouldn't it be safer to always clone the relevant section? Especially in cases like this, when we can't know what the contents are.

Anyway, if it's a concern, I can look into each one in more detail.

I'll comment on your suggestions and wait for your answers before making any changes

[andrzejewsky]

Of course the Config object should be absolutely read-only. When it's shallow copied, using Object.assign() or destructuring, then there's always a chance of causing problems. So, wouldn't it be safer to always clone the relevant section?

Not really, we can use config directly but we should always keep it mind that we can't do anything that can mutate it. For example when you picking just one section from the config, please don't operate on it later, create a new object instead (eg. with using spread operator) etc.

 const newObj = { ...config.section, yourModidication: ... }

instead of:

 const newObj = config.section
 newObj.yourModidication = ....

Of course mentioned config.section should be a shallow object.

I think you totally know what I mean.

@grimasod please update the changelog as well.