Skip to content

Conversation

danilowoz
Copy link
Contributor

@danilowoz danilowoz commented Oct 7, 2022

Set of measures to improve the Sandpack bundler performance:

  • Cache babel assets;
  • Use a minimal babel config instead of babel/standalone;
  • Improve spinner component transition;
  • Explorer how to decrease the number of dependency files; I don't it's still relevant because it only affects the very first load, and then the following loading will be cached;

@github-actions
Copy link

github-actions bot commented Oct 7, 2022

Size Changes

📦 Next.js Bundle Analysis

This analysis was generated by the next.js bundle analysis action 🤖

⚠️ Global Bundle Size Increased

Page Size (compressed)
global 89.02 KB (🟡 +23 B)
Details

The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!

@danilowoz
Copy link
Contributor Author

@gaearon

  • I managed to remove the solid-babel plugin from the babel worker (a bit hacky, but it doesn't really matter now);
  • Fixed an issue that turned the preview height higher than its body (it was a UI regression, actually);
  • Fixed the issue mentioned in previous Dan's comment, but with caveats.

The loading state simply appeared because its initial state was set to be visible. However, this initial loading was important to ensure that the iframe had time to get the height value based on its body (we need to consider that images might drastically increase the iframe height).

So, as we no longer have a loading component over the iframe, I'd like to propose we increase the initial iframe height, which will avoid this flash between different height values but might create an "extra padding" at the bottom of some sandboxes.

Original initial height

Screen.Recording.2022-10-17.at.21.15.26.mov

Proposal

Screen.Recording.2022-10-17.at.21.16.02.mov

@gaearon
Copy link
Member

gaearon commented Oct 17, 2022

Hmm. The way I was imagining it is that the entire white pill should be hidden until the first time the bundler completes. The user should never see an empty white pill. The white pill itself signifies completion. The white pill should be fading in with the content already — not the content fading in against the pill. If we can’t show the white pill with the content fast enough then we do need to show the spinner (on the grey background). We should never be showing an empty white pill without content inside it.

@danilowoz
Copy link
Contributor Author

danilowoz commented Oct 18, 2022

Yeah, it looks like a good idea. However, the problem is always the images because they take a bit to load, causing another resize on the white pill. I wish we could predict or set an initial body height (via sandbox style.css?) for those sandboxes that contain images and appear on the first fold, but it's not ideal either. What do you think?

Screen.Recording.2022-10-18.at.23.48.14.mov

@gaearon
Copy link
Member

gaearon commented Oct 18, 2022

Hm. Why does it shift layout when images have hardcoded heights in JSX?

@danilowoz
Copy link
Contributor Author

I was investigating why this layout shift happens even with images with fixed height, and then I realized the bundler was sending the resize event too early. It didn't use to be a problem because it used to have the spinner component over everything, but now without it, it's kind of noticeable.

The event used to be sent in between the evaluation step. From now on, it's sent after everything (bundling and evaluation), which might add some extra ms to the white pill be done.

@gaearon
Copy link
Member

gaearon commented Oct 19, 2022

Can we make the delay something like 200ms? So if we complete within 200ms, don't show the cube. Otherwise, show the cube.

@gaearon
Copy link
Member

gaearon commented Oct 19, 2022

(And btw, this looks great. Massive kudos for this work.)

@gaearon
Copy link
Member

gaearon commented Oct 19, 2022

Would be nice to reduce the flashing when runtime erroring, if possible.

Screen.Recording.2022-10-19.at.20.54.55.mov

@danilowoz
Copy link
Contributor Author

danilowoz commented Oct 21, 2022

@gaearon I think it's ready to go 😄

@gaearon
Copy link
Member

gaearon commented Oct 25, 2022

let's give this a try! thank you ❤️

@danilowoz danilowoz deleted the sandpack-bundler-improvements branch October 25, 2022 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants