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

Changed method comparing file object in componentDidUpdate #428

Closed
wants to merge 6 commits into from

Conversation

mathiasosmar1
Copy link

Hi @wojtekmaj, I was having a problem loading files, when passed an object in the file property the component was looped, but when it was string the problem did not occur.

I started to investigate the problem and found a failure to compare objects in the componentDidUpdate method, which uses the! == operator. As a beginner in react and js, I looked for a component that would do this work. I found react-fast-compare I can't say if it's the best on the market, but it worked very well

#353 @Kerumen

The react-fast-compare package will be used to compare two objects.
When the file object passed to Document is string the componentDidUpdate method comparison works perfectly, but when passing a File object consisting of url and httpHeader the componentDidUpdate method comparison validation fails. For this reason the way of comparison between file objects has been modified.
@mathiasosmar1
Copy link
Author

mathiasosmar1 commented Jul 26, 2019

Hi @wojtekmaj,
I can't understand the failure reported by codeclimate.
Would you help me?

@wojtekmaj
Copy link
Owner

Uh, oh. I don't think it's your fault actually. Will have a look and fix on master. By the way the issue you'e trying to tackle is #308

@mathiasosmar1
Copy link
Author

I think the fix is ​​related to both problems, because in the problem #353 when setState on the onLoadSucess event is performed it will reload the component with the same value passed to the file props and will fail to compare the componentDidUpdate method and will loop ( To reproduce the problem, pass the value {url: }). It will also correct the problem #308 as it currently does not perform depth comparison for the props file, the comparison !== compares only the props reference.

@wojtekmaj
Copy link
Owner

@mathiasosmar1 As much as I like how simple this solution is, this adds entire lodash libary to bundles of thousands of people. And lodash weighs quite a lot. I'm afraid I'll need to do another fix.

@wojtekmaj wojtekmaj closed this Jul 27, 2019
@wojtekmaj
Copy link
Owner

My approach: ef46915

Although still not production-ready...

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