Skip to content
This repository has been archived by the owner on Feb 22, 2018. It is now read-only.

fix(change_detection): should properly support objects with equality #735

Closed
wants to merge 1 commit into from

Conversation

pavelgj
Copy link
Contributor

@pavelgj pavelgj commented Mar 14, 2014

Fixes #670

@@ -823,3 +854,22 @@ class MapRecordMatcher extends Matcher {
return equals;
}
}

int fooIds = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that this is just test code, but would this not make more sense as a static member of FooBar?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@pavelgj pavelgj added cla: yes and removed cla: no labels Mar 14, 2014
@chalin
Copy link
Contributor

chalin commented Mar 14, 2014

Hmm, very interesting fix. But I thought that equality was not taken into consideration in the new CDA --- e.g., see Misko's comment to this post. I am still trying to learn how the new CDA code works and it must be subtle, but I am not seeing how the slight change you made influences how the FooBar class equality is used by the CDA. If you happen to have 2 min at some point and would be willing to explain why your fix works, that would be appreciated. (By the way, I am not doubting the correctness of the fix, I just feel that there is something important in that fix that is important to understand! :)

@caitp
Copy link
Contributor

caitp commented Mar 14, 2014

We don't have operator overloading in ES6, so the rationale for this fix might not matter there (without adding some opinionated method name used for equality within CD, which doesn't really make sense) --- but what seems to be changing is operation ordering. Would that apply to the JS port? Seems like that's potentially still an issue for us.

@pavelgj pavelgj closed this in 9b480da Mar 14, 2014
@chalin
Copy link
Contributor

chalin commented Mar 14, 2014

@pavelgj , nm. Is see that the FooBar equality & hashCode affect map lookup in DuplicateMap (of dirty_checking_change_detector.dart).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

Dirty checker or digest seems buggy
3 participants