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

!== or shallow comparison in shouldComponentUpdate ? #10

Open
happytuna75 opened this issue Jan 9, 2019 · 2 comments
Open

!== or shallow comparison in shouldComponentUpdate ? #10

happytuna75 opened this issue Jan 9, 2019 · 2 comments
Labels
feature help wanted Extra attention is needed

Comments

@happytuna75
Copy link

JsonEditor.prototype.shouldComponentUpdate = function shouldComponentUpdate(_ref2) { var htmlElementProps = _ref2.htmlElementProps; return htmlElementProps !== this.props.htmlElementProps; };

In this function you compare with !== the new (yet to be applied) and the old (current) htmlElementProps.

In the docs you specify
* @property {object} [htmlElementProps] - html element custom props

but if htmlElementProps is an object than the comparison above will always return true, since you are comparing two distinct objects and for example, '{a: 1} !== {a: 1}' gives true.
Shouldn't you have a shallow comparison here? like suggested in https://reactjs.org/docs/shallow-compare.html and in particular
screen shot 2019-01-09 at 15 13 48

@vankop vankop added help wanted Extra attention is needed feature labels Jan 9, 2019
@vankop
Copy link
Owner

vankop commented Jan 9, 2019

@happytuna75 thanks for advice! I thinks its great feature. I will implement it later, or if you want to help - PR wellcome=)
My suggestion to use https://github.com/dashed/shallowequal

@happytuna75
Copy link
Author

I'm very happy to volunteer :)
You'll see my pull request soon...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants