search

botman / web-widget

MIT License
37 stars 68 forks source link

Update State using setState() #15

Closed mohsinht closed 4 years ago

mpociot commented 5 years ago

Thanks for your PRs - can you tell me why this is better than modifying the state directly?

mohsinht commented 5 years ago

I was getting an error when I ran 'npm rum dev' on my computer. I've reported the issue with the error msg I was getting.

I went through all of the code and I changed many pieces to check what was causing the problem. I noticed it was the state update. State values are never recommended to be updated directly in React. I changed the lines where state was updated and the error never came again.

Test it before merging in the master repo. I've tested on two machines and its working fine now.

On Mon, Aug 20, 2018, 3:32 PM Marcel Pociot notifications@github.com wrote:

Thanks for your PRs - can you tell me why this is better than modifying the state directly?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/botman/web-widget/pull/15#issuecomment-414271572, or mute the thread https://github.com/notifications/unsubscribe-auth/AWa28MHkpgDsLKu6OuHf2PF7PRSdAzHcks5uSpBEgaJpZM4V6dFT .

mohsinht commented 5 years ago

This is the issue: https://github.com/botman/web-widget/issues/14

On Mon, Aug 20, 2018, 4:08 PM Malik Mohsin Hayat FastNU < l164333@lhr.nu.edu.pk> wrote:

I was getting an error when I ran 'npm rum dev' on my computer. I've reported the issue with the error msg I was getting. I'm a React developer for over an year and I've noticed that updating state using setState not only keeps the dom reactive, it is very secure.

I went through all of the code and I changed many pieces to check what was causing the problem. I noticed it was the state update. State values are never recommended to be updated directly in React. I changed the lines where state was updated and the error never came again.

Test it before merging in the master repo. I've tested on two machines and its working fine now.

On Mon, Aug 20, 2018, 3:32 PM Marcel Pociot notifications@github.com wrote:

Thanks for your PRs - can you tell me why this is better than modifying the state directly?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/botman/web-widget/pull/15#issuecomment-414271572, or mute the thread https://github.com/notifications/unsubscribe-auth/AWa28MHkpgDsLKu6OuHf2PF7PRSdAzHcks5uSpBEgaJpZM4V6dFT .

mohsinht commented 5 years ago

You haven't updated the repository yet.

danlopez00 commented 5 years ago

Thanks for your PRs - can you tell me why this is better than modifying the state directly?

The build fails. as mohsinht explained this will fix all build errors and I confirm

mohsinht commented 5 years ago

Please merge the branch.