search

git-masi / bitcoin-data-visualizer

An app that shows various financial data related to bitcoin.
0 stars 0 forks source link

Decoupling react component updates #1

Open jmbothe opened 5 years ago

jmbothe commented 5 years ago

Hi Eric! I saw your talk at CharmCityJS last night (great talk! btw), and heard you mention you were looking for someone to review this app. I haven't looked at any of the code yet (I like to start with the raw UX first, then move to devtools inspection, before making assumptions from reading code. Also, I am not sure how much detail you want. Maybe you want to investigate and solve these issues yourself). But I cloned the repo and spun up the app and just started poking at it.

The one major thing I noticed is that, on the Price Index page, certain component updates cause updates in other unrelated components. For example, selecting a new option in the Fast Actions dropdown causes updates in the nav and the Exchange Currency box.

You can see this by using the react devtools browser extension, and toggling the Highlight Updates setting.

Since you aren't using redux or the context API (ok, now I am looking at code), I would say this is probably because your other components are receiving props that they don't actually need. This often stems from lifting state too high, and unnecessary prop drilling. It could also be related to one of the libraries you are using. I don't know anything about React CSS Transitions, but I do know that any third-party Components wrapped around your app can lead to all sorts of behavior when used incorrectly.

Let me know if you want to dig deeper.

Cheers!

jmbothe commented 5 years ago

I guess I should mention why this is important.

For your app as it currently is, this does not matter at all. But as your app grows, these problems will grow with it, and eventually lead to performance issues.

Something like changing the value of a dropdown is a one-off action, and even if it does cause the entire page to update, no one is going to notice. But let's say you later add a text input, which updates state on every key stroke. Now, when your user starts rapidly typing in the text input, they are going to be slamming all the state-coupled components on your page with updates as they type. And now lets say that one of those components makes some performance-costly calculation every time it updates... You're definitely going to start seeing some lag while typing, and depending on which components are coupled to these updates, maybe some jank on the screen as well.

git-masi commented 5 years ago

Hi Jeff! Apologies for not responding sooner I literally just saw this, not sure why I didn't get a message notifying me about the issue you submitted but whatever. That's a great insight you've offered and something I will have to look into further.

I recently learned a bit more about React performance and preventing needless re-rendering with pure components and shouldComponentUpdate which might be helpful in preventing the performance issues you've described. Will post more when I have something to show. Thanks again for your response!