Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistency logic behavior between mergeWith & mergeDeepWith #9

Open
Methuselah96 opened this issue Oct 16, 2020 · 4 comments
Open

Comments

@Methuselah96
Copy link
Owner


Migrated from don't!
Originally created by @vpcvdc on Tue, 07 Jul 2020 06:43:07 GMT


What happened

mergeWith working fine as per https://immutable-js.github.io/immutable-js/docs/#/mergeWith docs.
mergeDeepWith NOT working fine as per https://immutable-js.github.io/immutable-js/docs/#/mergeDeepWith.
It says like mergeDeepWith is recursive implementation of mergeWith. @leebyron Isn't it?

How to reproduce

Screen Shot 2020-07-07 at 12 05 50 PM

@Methuselah96
Copy link
Owner Author


Migrated from don't reference!
Original comment by @Methuselah96 on Fri, 14 Aug 2020 19:00:41 GMT


@vpcvdc Can you expound on what you think the inconsistent behavior is? It looks like it is working correctly to me.

The first example only goes one layer deep, so it ends up calling console.log([1], [1, 2, 3]);.

The second and third examples go all the way to the deepest layer and since arrays get merged together, it ends up merging the arrays together and returning an object without calling console.log.

To clarify, in both mergeWith and mergeDeepWith the merger function only ends up getting called on non data structure values (i.e., not on arrays), or in the case of mergeWith after it gets past the first layer.

@Methuselah96
Copy link
Owner Author


Migrated from don't reference!
Original comment by @vpcvdc on Mon, 17 Aug 2020 08:17:51 GMT


Hi @Methuselah96

As per document says

calling the merger function whenever an existing value is encountered.

In the above screenshot, merger => console.log
mergeWith function calling merger function when an exist value encountered on particular property key
But the same thing NOT happening on mergeDeepWith 🤯

In the above screenshot, mergeDeepWith behaving like mergeDeep 😳

I hope you get the clarity, what I really mean 🙂

@Methuselah96
Copy link
Owner Author


Migrated from don't reference!
Original comment by @Methuselah96 on Mon, 17 Aug 2020 13:33:07 GMT


@vpcvdc I don't think that arrays are considered values in this case. The merger function isn't called on objects or arrays EXCEPT in the case where we're more than one layer deep in mergeWith.

For example the merger function is called on objects here (similar to how they were called on arrays in your first example):
image

@Methuselah96
Copy link
Owner Author


Migrated from don't reference!
Original comment by @conartist6 on Fri, 04 Sep 2020 01:53:34 GMT


I'm trying to pick apart what's going on here.

To start with the docs are wrong. There is no such method as Collection.prototype.mergeWith. There is only Map.prototype.mergeWith. This explains why the merger is never called on arrays or Lists.

To fix this we need to either remove Immutable.mergeWith or implement Collection.prototype.mergeWith.

And of course the situation is the same for mergeDeepWith.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant