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

[Medium] Simplify down JS #219

Open
dawsbot opened this issue Aug 20, 2022 · 6 comments
Open

[Medium] Simplify down JS #219

dawsbot opened this issue Aug 20, 2022 · 6 comments

Comments

@dawsbot
Copy link
Contributor

dawsbot commented Aug 20, 2022

Monitor JS size over time here: https://ultrasound-bundle-sizes.vercel.app/

image

@dawsbot dawsbot changed the title Simplify down JS [Medium] Simplify down JS Aug 20, 2022
@alextes
Copy link
Collaborator

alextes commented Aug 20, 2022

Haha, well let me know when you figure out what to do here.

Do keep in mind we have a bunch of timers, count downs, updating gas prices, those are all naturally "long main-thread tasks" in fact, they never finish. Still, any reduction in JS complexity is welcome 🤗 !

In fact, reducing complexity is one of the things devs should value most in my opinion 😄 .

@dawsbot
Copy link
Contributor Author

dawsbot commented Aug 20, 2022

@alextes Do you see any easy fixes of fat libraries in this?

image

https://ultrasound-bundle-sizes.vercel.app/august-20-2022.html

In addition to replacing anything heavy here, it will likely be a LazyLoad wrapper which takes the performance through the roof so that this polling and charting JS does not run until the user scrolls down to it.

@alextes
Copy link
Collaborator

alextes commented Aug 22, 2022

I've looked at that map several times in the past, and removed some libs already, so it's getting harder to remove more.
The best I see is:

  • We have several tooltip implementations, some rather heavy, we also have our first CSS-only one, if we could convert the other JS heavy ones we could drop react-popper, but this is quite a bit of work.
  • I'm not too familiar with the storypreview page that uses framer-motion, but framer-motion looks quite large to me and I wonder if we could tree-shake it.

Then we have the huge highcharts / highcharts-annotations libs, we need them for our charts, or at least I don't see a way to switch to a lighter library without major work replicating all the behavior we built, and carefully making sure any other lib supports future desired behavior. So that option is probably out. So then we should make sure they impact loading minimally. Lazy loading on observation is one way to do it. It may be a great way to make the heaviest components no longer drag down the rest. I'd slightly prefer us to load critical stuff first but that could come later. Where we make sure that the lightest stuff that is the most visible loads first, whilst not blocking the browser from loading the rest when it has CPU cycles / network bytes available. In that line, putting everything that loads highcharts behind a nextjs dynamic import might be a quick and easy win. I think framer-motion if not tree-shakable also could majorly speed up the page by being behind dynamic import. react-popper too come to think of it, tooltips don't show on load, but then like I said, it may be possible to drop that lib entirely instead.

Lastly, I think some of the biggest wins actually lie in changing our design and API endpoints. That way we can both "not load what the user isn't looking at" more without "starting loading only when they already are", which is the other extreme and also not great.

@dawsbot
Copy link
Contributor Author

dawsbot commented Aug 24, 2022

I think some of the biggest wins actually lie in changing our design and API endpoints. That way we can both "not load what the user isn't looking at" more without "starting loading only when they already are", which is the other extreme and also not great.

I think a good approach for this would be server-side pre-fetching of this data with a cache. Next.js makes this easy with https://nextjs.org/docs/basic-features/data-fetching/get-static-props

@alextes
Copy link
Collaborator

alextes commented Aug 24, 2022

For some components yes, but for those which most frequently update I'm not sure.

from: https://nextjs.org/docs/basic-features/data-fetching/get-server-side-props#fetching-data-on-the-client-side

If your page contains frequently updating data, and you don’t need to pre-render the data, you can fetch the data on the client side. An example of this is user-specific data:

  • First, immediately show the page without data. Parts of the page can be pre-rendered using Static Generation. You can show loading states for missing data
  • Then, fetch the data on the client side and display it when ready

This approach works well for user dashboard pages, for example. Because a dashboard is a private, user-specific page, SEO is not relevant and the page doesn’t need to be pre-rendered. The data is frequently updated, which requires request-time data fetching.

So I'd say, identify which components rarely update their data, it's not many but there are a couple of heavy ones for which this could be great! For the ones that need to update their data every couple it's probably much more effective to cache that which doesn't change, and client-side fetch and update that which does. That way clients only fetch the new data, not the new rendered components.

Which reminds me, hydration is turned off for the Dashboard page 😅 , there were hydration issues, I haven't been able to narrow down what's causing them, even with source maps turned on Sentry isn't much help. So everything is rendered on the client at the moment, huge performance cost, fix the hydration issue, enable more benefits from server side rendering. To turn hydration back on see /src/pages/index.tsx

@dawsbot
Copy link
Contributor Author

dawsbot commented Aug 24, 2022

OH, that's probably the biggest factor for improving perf. Let's get the broken components setup to client-side render but enable it app-wide. Testing now

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

No branches or pull requests

2 participants