Skip to content

perf(qwikloader): put qwikloader in a separate bundle #7613

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

Merged
merged 3 commits into from
Jun 2, 2025

Conversation

wmertens
Copy link
Member

@wmertens wmertens commented May 19, 2025

  • move qwikloader to its own eagerly loaded chunk
  • automatically unregister events that have no listener attributes
  • optimize compressed qwikloader size

We were embedding the qwikloader, which is faster for initial load, but recent experiments with modulepreload show that this isn't necessary.

By putting the qwikloader in its own bundle and using modulepreload + async module script:

  • the bundle is discovered during the html download
  • the bundle downloads together with the html
  • the bundle can be served with brotli compression, which is more costly to do during SSR
  • subsequent SSR requests never have to download the 1.5-2kb again
  • multiple containers with the same qwik version can all embed the script import tag and it will only be loaded once

This doesn't seem to impact LCP at all.

In the pathological 3G simulation with the fixed 2s lag, the chunk is fetched first, while the html is still downloading, and finishes pretty soon after html finished downloading and while the other resources are also downloading, so the user doesn't really notice anything.

image

compared to before:
image

(the other two yellow preloads you see are the preloader bundle and the qwik core bundle)

PS it would be really cute to find a string to embed in the preloader that would make the current uD3ipkCl hash start with ql_. Anybody with a lot of compute time to burn? :)

@wmertens wmertens requested review from a team as code owners May 19, 2025 19:59
Copy link

changeset-bot bot commented May 19, 2025

🦋 Changeset detected

Latest commit: 61213c5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@builder.io/qwik Patch
eslint-plugin-qwik Patch
@builder.io/qwik-city Patch
create-qwik Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

pkg-pr-new bot commented May 19, 2025

Open in StackBlitz

npm i https://pkg.pr.new/@builder.io/qwik@7613
npm i https://pkg.pr.new/@builder.io/qwik-city@7613
npm i https://pkg.pr.new/eslint-plugin-qwik@7613
npm i https://pkg.pr.new/create-qwik@7613

commit: eedb9c2

Copy link
Contributor

github-actions bot commented May 19, 2025

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
qwik-docs ✅ Ready (View Log) Visit Preview eedb9c2

@wmertens wmertens force-pushed the qwikloader-chunk branch 3 times, most recently from f0c1116 to 4163cce Compare May 20, 2025 10:07
@maiieul
Copy link
Contributor

maiieul commented May 21, 2025

If I'm not mistaken the page can now be visible before the qwikloader is finished downloading, which means some user events might get lost between that time.

For example:
image
If a user clicks between ~3100ms and ~4400ms on slow 3G the user event won't be registered. It's a little annoying for demos.

On the other hand you could argue that slow 3G latency is unrealistic and that this change theoretically should improve performance for real users.

So I'm not sure what I prefer here ^^.

I'd like to test on my Xiaomi on real 2G before we merge this.

@wmertens
Copy link
Member Author

would be interesting to see if 103 Early Hints would help, but Safari doesn't implement preload so that's a huge chunk of the mobile userbase for which it won't work.

@maiieul maiieul self-requested a review June 1, 2025 19:06
@maiieul maiieul moved this from Backlog to Waiting For Review in Qwik Development Jun 1, 2025
@wmertens
Copy link
Member Author

wmertens commented Jun 2, 2025

I tried it on my Android phone as follows:

  • go to settings, make sure you have developer options enabled
  • search for rate limit, go there, set to 256kb
  • go to https://qwikdev-qwikloader-chunk.qwik-8nx.pages.dev/ in incognito mode
  • as soon as you see the search button, click it once
  • after 25s, it shows the search box

this shows that the qwikloader was loaded fast enough IMHO

@wmertens wmertens force-pushed the qwikloader-chunk branch 2 times, most recently from d7aa008 to eedb9c2 Compare June 2, 2025 09:15
maiieul
maiieul previously approved these changes Jun 2, 2025
Copy link
Contributor

@maiieul maiieul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay so in 2G for me there's only a tiny window where I can click and qwikloader isn't loaded yet. It's more visible on slow 3G throttling because of the fake latency. So users with real world latency could theoretically experience it, although they'd have to click pretty fast. But I believe this should slightly improve the FCP for most apps so I think it's a good change.

@wmertens wmertens enabled auto-merge June 2, 2025 13:55
@wmertens wmertens disabled auto-merge June 2, 2025 13:59
@wmertens wmertens enabled auto-merge June 2, 2025 13:59
@shairez
Copy link
Contributor

shairez commented Jun 2, 2025

@wmertens few things -

1, The code looks great, good job!

2, I think a changeset is needed here

  1. can we ship a tiny inline script that just records (remembers) the user events that were emitted until the qwik-loader loads and emits them again? (just to cover all cases)

That way we're safe even if it takes way longer for some reason

WDYT?

@wmertens
Copy link
Member Author

wmertens commented Jun 2, 2025

@shairez hmm, the inline script would actually mean repeating much of the qwikloader internals. Our tests show that the qwikloader is available pretty instantly.

Added the changeset.

@shairez
Copy link
Contributor

shairez commented Jun 2, 2025

ok cool
thanks!

@wmertens wmertens merged commit 52d81be into main Jun 2, 2025
17 checks passed
@wmertens wmertens deleted the qwikloader-chunk branch June 2, 2025 18:50
@github-project-automation github-project-automation bot moved this from Waiting For Review to Done in Qwik Development Jun 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants