Skip to content
This repository has been archived by the owner on Jul 23, 2021. It is now read-only.

Equals not used when values updated #150

Open
Methuselah96 opened this issue Oct 17, 2020 · 6 comments
Open

Equals not used when values updated #150

Methuselah96 opened this issue Oct 17, 2020 · 6 comments
Labels
enhancement New feature or request from-original-repo

Comments

@Methuselah96
Copy link

From @SebastianStehle on Sat, 07 Sep 2019 20:12:55 GMT

Hi, I was playing around with immutable.js and I like the equals checks.

What I do not understand is that these checks are not used for updates, e.g. I was playing around with Record and Map:

https://codesandbox.io/s/hidden-sound-h5o2t

Copied from original issue: immutable-js#1732

@Methuselah96
Copy link
Author

Methuselah96 commented Oct 17, 2020

I very quickly skimmed through the original thread and I believe this is at most a feature request and is mostly just intended behavior. Keeping open for now.

@SebastianStehle
Copy link

Coming from C++, Java and C# I still think this is not very intuitive and unexpected.

@Methuselah96
Copy link
Author

Methuselah96 commented Oct 17, 2020

I read through the issue again a little more carefully. I believe your argument/suggestion is that the is method should be used for value equality (so that it will work for Immutable objects and/or custom objects that implement equals()) or there should be a way to define a custom equality/comparer for elements in a collection.

The first suggestion would be a breaking change which makes it less likely to happen. We would have to be convinced that the improvement is worth more than the cost of the breaking change.

The second suggestion could be released in a minor version, so it has more potential.

C++, Java, and C# are all (at least in some sense) object-oriented programming languages and are all similar to each other. JavaScript is not intended to be object-oriented. There are going to be things that are not intuitive and unexpected when switching from an object-oriented programming language to JavaScript. Not every object having an equals() method (and it not being used even if it exists) is one of them.

@Methuselah96 Methuselah96 added the enhancement New feature or request label Oct 17, 2020
@SebastianStehle
Copy link

I understand your first argument.

But I don't agree with the second argument. For me the point is not that C# behaves differently than javascript. The point is consistency. C# is very consistent when you use equality. You can either use the default equals implementation or implement your own and it is respected by all collections.

Immutable-js also has its own definition of equality or value equality to be more precise and and it is not used consistently and this hurts performance, because it is more likely that 2 collections with the different references are actually equal and therefore it is causing unnecessary updates in the UI layer.

@Methuselah96
Copy link
Author

Methuselah96 commented Oct 17, 2020

I was just disagreeing with your point that "this is not very intuitive and unexpected." If immutable were implemented in C++, Java, and C#, I would agree that it would unexpected since each of those languages has a way of defining equality which is expected to be respected by code that uses equality. I was just trying to point out the fact that JavaScript does not have that same ubiquitous definition of equality that can be override by any data type (i.e., an Equals() method). This is a language difference.

immutable uses two definitions of equality and I think that it fairly consistently uses the is when accessing something by key (e.g., includes(), get(), keyOf(), update(), etc.), but uses strict equality when updating the value. I could be wrong about which is used where, but this is what I gathered from taking a quick look at the code.

I'm not saying your proposal is not a good idea (and it might help performance). I'm just saying that it's working as intended (i.e., this isn't a bug in immutable) and I don't think it needs to be a priority to change it.

@SebastianStehle
Copy link

But how are the equal methods implemented within the collection?

Immutable.js collections are treated as pure data values. Two immutable collections are considered value equal (via .equals() or is()) if they represent the same collection of values.

I cannot find the implementation for that. deepEquals uses the is function and therefore the equals method which you can override and has the same semantics together with hashCode as in C#. So for me it looks very consistent except this one case for updates.

But I totally understand your point and priorities.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request from-original-repo
Projects
None yet
Development

No branches or pull requests

2 participants