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

isEqual fails hard. #4949

Merged
merged 10 commits into from
May 7, 2018
Merged

isEqual fails hard. #4949

merged 10 commits into from
May 7, 2018

Conversation

keyurpatel
Copy link
Member

@keyurpatel keyurpatel commented May 4, 2018

Issue: - #4950

small description.
stateful mixin method isEqual throw an exception when it tries to compare an array value with null.

@asturur
Copy link
Member

asturur commented May 6, 2018

So i added tests to your branch executing the bug you are fixing for arrays and objects, and indeed the array tests pass, while the object tests fails,
Can you fix also the object case since you are on this topic?
Is a couple of lines later, we are executing object.Keys on a variable that can be or not an object.
I added a test for string and for null, i have no idea if they fail both or just one.

@keyurpatel
Copy link
Member Author

@asturur Thanks for adding a test. Sure will take a look to fix the object case.

@keyurpatel
Copy link
Member Author

Everything passed locally !!!
screen shot 2018-05-07 at 1 10 03 pm

@asturur
Copy link
Member

asturur commented May 7, 2018

So why it was going locally no idea. The test was actually catching a bug, calling null is of type Object and Object.keys(null) fails.

@asturur asturur merged commit 885545a into fabricjs:master May 7, 2018
@asturur asturur mentioned this pull request May 8, 2018
thiagocunha pushed a commit to thiagocunha/fabric.js that referenced this pull request Nov 18, 2019
* fix current value null issue
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

Successfully merging this pull request may close these issues.

2 participants