WIP: Support for Maps and Sets

[RikuVan]

I had a go at this for learnings sake, but I am pretty unsure if this was the way to go about it. I didn't see the benefit in using a real 'Proxy' since all mutations go through the methods. This does not support WeakMap and WeakSet (i have my doubts it is really worth it as you cannot clone them in the same way you can Maps and Sets).

[coveralls]

Coverage increased (+1.1%) to 94.231% when pulling 6de8e9e on RikuVan:support_map_and_set into 84229e2 on mweststrate:master.

[RikuVan]

ok the es5 part is not working correctly. Will try to look again at it when I have time.

[RikuVan]

The main problem I am having in es5 after add the Map and Set proxy is that array operations are affected--so push and pop result in undefined value. I will add an example failing test. Would thankful if someone could give me a tip about how the added code could affect arrays.

[RikuVan]

Well that is weird. I can't seem to write a failing test with the array problem I described 🤔 so maybe something with how I am using Immer locally.

[mweststrate]

Thanks a lot! Will be travelling next week and will review in detail afterwards, but looking promising so far!

[RikuVan]

Been meaning to come back to this for some time as I never really addressed the nested Map/Sets possibility. I have not written tests or really looked at this at all, but I am sure the current code doesn't work for nested Maps/Set updates, like Map.get("a").get("b").add(10). But i thought to look at this on the weekend, or at least write some failing tests.

[mweststrate]

Ok, thanks in advance!

[mweststrate]

N.B. if you are stuck with the implementation somewhere just let me know, but having full test coverage is super useful already!

[RikuVan]

Yeah, I spent a number of hours the last week playing with this and could not get nested Maps and Sets to be correctly cloned. I tried, for example, recursively proxying all the nested Maps and Sets initially. This mostly seems to work, although it caused a few es5 tests to fail. But it doesn't result in the new test passing. I don't really get why the copying fails to result in new objects. I logged out e.g. old Set and new Set and they are different after being marked as changed. But somehow this is overwritten by the old version--maybe by a parent. I also tried a completely different approach, just tracking all the Maps and Sets in a WeakMap and then doing deep comparisons to see if something should be copied. But this resulted in a crap load of code and a lot of indirection, plus probably horrible performance if there are lots of Maps. I don't think that is the way to go. So, yeah, kind of stuck.

It is perhaps also good just to clarify what the goal is. I presumed that there should be structural sharing, which I understand to mean that only nested Maps/Sets with changes should be copied. But perhaps it would be easier to achieve if the idea was that however deep the update, everything inside that root Map or Set would be copied by the parent--so really the parent just needs a signal from a chlld that it changed rather than the chain of updates down to the parent. If that makes any sense...

[RikuVan]

Hi, I am going to close this PR, which I have not worked on since I did it. It was interesting exercise to try this but I didn't really see a clear way forward after my initial attempts.

[samuelamars]

?????

[samuelamars]

Maybe

[samuelamars]

Good