QwikDev / qwik

Instant-loading web apps, without effort
https://qwik.dev
MIT License
20.87k stars 1.31k forks source link

fix(prefetch-sw): only prefetch direct imports #6853

Closed wmertens closed 2 months ago

wmertens commented 2 months ago

It was adding dynamic imports, which is not correct. Those should be handled by Link prefetching and Insights.

Furthermore, the graph is reduced by not including dependencies that other dependencies already import.

changeset-bot[bot] commented 2 months ago

🦋 Changeset detected

Latest commit: da895f66ef8a3a2ea50893c9d4250e2139da7c3f

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

pkg-pr-new[bot] commented 2 months ago

Open in Stackblitz

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

commit: da895f6

github-actions[bot] commented 2 months ago
built with Refined Cloudflare Pages Action

âš¡ Cloudflare Pages Deployment

Name Status Preview Last Commit
qwik-docs ✅ Ready (View Log) Visit Preview da895f66ef8a3a2ea50893c9d4250e2139da7c3f
wmertens commented 2 months ago

@mhevery We can differentiate imports, handlers, segments, and dynamic imports. We can differentiate by negative numbers indicating the type of what follows, -1 handlers, -2 segments, -3 dynamic.

Alternatively, Qwik loader can just tell us which handlers are actually on the page and those then get fetched opportunistically, no differentiation needed, only direct imports. What do you think?

🤔 Or indeed any dynamically imported segments that are not handlers are likely to be tasks etc.

Ah and we can sort by module size, should we fetch big or small modules first?

Not for this PR, but the prefetch SW is also missing:

wmertens commented 2 months ago

@mhevery I now detect tasks (by the fact that they also export _hW) and fetch them with lower priority. Other dynamic imports are left up to the qprefetch events.

WDYT?