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

preload does not work and downloaded twice on firefox #1042

Closed
xhebox opened this issue Apr 15, 2021 · 27 comments
Closed

preload does not work and downloaded twice on firefox #1042

xhebox opened this issue Apr 15, 2021 · 27 comments
Labels
bug Something isn't working p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. vite
Milestone

Comments

@xhebox
Copy link

xhebox commented Apr 15, 2021

Describe the bug

  1. Here is the firefox bug of module preload: https://bugzilla.mozilla.org/show_bug.cgi?id=1425310, i.e. firefox does not support modulepreload. There is a similar issue for sapper, Replace modulepreload with preload for Firefox and Safari sapper#1387.

  2. The new vite already has a builtin preload plugin https://github.com/vitejs/vite/blob/main/packages/vite/src/node/plugins/importAnalysisBuild.ts#L156-L162 on the client side. It will insert preload links despite sveltejs/kit also does that:

    ...Array.from(js).map((dep) => `<link rel="modulepreload" href="${dep}">`),
    ...Array.from(css).map((dep) => `<link rel="stylesheet" href="${dep}">`)
    ].join('\n\t\t');
    .

Links from svelte are like /./_app/pages/$layout.svelte-3b4aa0d5.js, but vite checks /_app/pages/$layout.svelte-3b4aa0d5.js. So vite does not think the inserted link is duplicated. One fix is:

--- a/node_modules/@sveltejs/kit/dist/chunks/index5.js
+++ b/node_modules/@sveltejs/kit/dist/chunks/index5.js
@@ -229,8 +229,8 @@ async function build_server(
 
                find_deps(file, js_deps, css_deps);
 
-               const js = Array.from(js_deps).map((url) => prefix + url);
-               const css = Array.from(css_deps).map((url) => prefix + url);
+               const js = Array.from(js_deps).map((url) => path.join(prefix, url));
+               const css = Array.from(css_deps).map((url) => path.join(prefix, url));
 
                const styles = config.kit.amp
                        ? Array.from(css_deps).map((url) => {
  1. Vite inserted links like <link rel="preload" as="script" crossorigin="" href="/_app/pages/$layout.svelte-a23c4e9e.js">, but it does not take effect somehow. So when sveltekit import scripts, resource will be downloaded again.

To Reproduce

Create an app with layout, build & start, and you should be able to see the twice downloading on the latest firefox.

Expected behavior

Despite of preloading or not, I only want one time downloading.

Information about your SvelteKit Installation:

Diagnostics -
  System:
    OS: Linux 5.8 Noname Linux
    CPU: (16) x64 AMD Ryzen 7 4800H with Radeon Graphics
    Memory: 20.62 GB / 30.36 GB
    Container: Yes
    Shell: 5.0.7 - /bin/bash
  Binaries:
    Node: 14.13.1 - /bin/node
    Yarn: 1.22.10 - ~/.npm-packages/bin/yarn
    npm: 6.14.8 - /bin/npm
  Browsers:
    Firefox: 86.0.1
  npmPackages:
    @sveltejs/kit: next => 1.0.0-next.74 
    svelte: ^3.37.0 => 3.37.0 
    vite: ^2.1.5 => 2.1.5 
  • Your browser: firefox 86.0.1

  • Your adapter: vercel, but I am experimenting with npm run start.

Severity

I think it can be a major bug, nobody like downloading things twice.

@xhebox

This comment has been minimized.

@Rich-Harris
Copy link
Member

are you 100% sure you're not just seeing firefox request stuff from the service worker, and the service worker request stuff from the network?

@Rich-Harris

This comment has been minimized.

@xhebox
Copy link
Author

xhebox commented Apr 16, 2021

are you 100% sure you're not just seeing firefox request stuff from the service worker, and the service worker request stuff from the network.

I am sure. I am not using service worker, though. I've seen $layout-xxx.js downloaded twice. One comes from preload-helper-xxx.js, which looks exactly same to the source code of vite. And the second request comes from the entry/startup script block.

don't ever ping maintainers with "PTAL" or similar

Are there some unspoken rules that I don't know :O? Sorry if that bothers you.

@rmunn

This comment has been minimized.

@xhebox

This comment has been minimized.

@rmunn

This comment has been minimized.

@benmccann
Copy link
Member

benmccann commented Apr 18, 2021

Some discussion of the double download here: https://bugzilla.mozilla.org/show_bug.cgi?id=1425310#c3

I think the line here should detect if it's Chrome vs Firefox/Safari and return modulepreload vs preload as appropriate:

...Array.from(js).map((dep) => `<link rel="modulepreload" href="${dep}">`),

@benmccann benmccann added the bug Something isn't working label Apr 18, 2021
@benmccann benmccann added this to the 1.0 milestone Apr 18, 2021
@johnnysprinkles
Copy link
Contributor

Support for modulepreload is pretty scattered according to https://caniuse.com/link-rel-modulepreload, wouldn't it be safer to opt-in Chrome and Chromium-based Edge specifically? I just ran into this double download issue with a bone stock npm init @svelte@next with starter app, but I may be the only person left on Earth who uses FF as their daily driver.

Screen Shot 2021-05-21 at 11 21 57 PM

56k of JS while on Chrome I get 40k.

@johnnysprinkles
Copy link
Contributor

johnnysprinkles commented May 22, 2021

Well now I'm just confused, I commented out the modulepreload tags but still see that double load. I'm not sure I trust the Firefox dev tools now. I might try running this through a proxy like Charles proxy.

Screen Shot 2021-05-21 at 11 56 38 PM

@johnnysprinkles
Copy link
Contributor

And you're right @benmccann I see in the compiled client JS code it's inspecting link.relList.supports('modulepreload'), which I guess comes from https://github.com/vitejs/vite/blob/4112c5d103673b83c50d446096086617dfaac5a3/packages/vite/src/node/plugins/importAnalysisBuild.ts#L39, but for some reason I'm seeing modulepreload in Firefox even though it definitely reports no support for it. I don't know.

@johnnysprinkles
Copy link
Contributor

Oh wait, this is in the prerendered index.html file, client-side checks like this wouldn't and couldn't be in effect. Anyway this just sounds like a Vite issue, I'll look into their bug tracker.

@xhebox
Copy link
Author

xhebox commented May 22, 2021

I have filed an issue for vite, check vitejs/vite#3133.

@johnnysprinkles

This comment has been minimized.

@xhebox

This comment has been minimized.

@johnnysprinkles
Copy link
Contributor

So I tried this on a somewhat more substantial project, something I'm working on at work, and it looks like only the vendor chunk and those two tiny layout and error files are duplicated, everything is single loaded, so it's like a 10kb penalty for Firefox that seems fairly constant as the app grows. I'm not too worried about this issue at the moment anymore.

Screen Shot 2021-05-22 at 6 10 19 PM

@eikaramba
Copy link

eikaramba commented Jun 23, 2021

i am seeing the same problem with latest FF 89.0.1
grafik

if anyone wants to check himself. https://notific.at (might be down, the site is online for testing for now)

unfortunately it actually loads every component two times for me. i haven't been able to check it with charles myself, but will do so.

update:
so yes it seems the components are all loaded twice according to charles.
grafik

@silvestrevivo

This comment has been minimized.

@benmccann benmccann added the p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. label Aug 4, 2021
@benmccann
Copy link
Member

Would anyone be able to test this with Vite 2.5 beta? They've added a modulepreload polyfill: vitejs/vite#4058

@benmccann
Copy link
Member

I just tested with vite@2.5.0-beta.2 and this appears fixed now

@frankmayer
Copy link

frankmayer commented Aug 28, 2022

Not sure if I should open a new issue on this, but I am seeing this issue with Firefox (103.0.2) on latest sveltekit/vite.
It seems it has crept up again.

To reproduce, you can check against https://kit.svelte.dev/ where the issue is visible in devtools/network. Some files are loaded twice and even three times (favicon). This is showing 202kB (Chrome) vs 240kB (Firefox) transferred (with cache disabled) in 36 (Chrome) vs 50 (Firefox) requests.

@GrygrFlzr
Copy link
Member

Can reproduce on a very simple kit project. Reopening.

@GrygrFlzr GrygrFlzr reopened this Sep 10, 2022
@Rich-Harris
Copy link
Member

This seems like something that will have to be fixed in Vite. As a test, I removed all the <link rel="modulepreload"> elements using transformPageChunk, and saw the same result, which suggests the issue is Vite's module preloading logic. Not immediately sure what the fix is though.

@benmccann benmccann added the vite label Sep 21, 2022
@frankmayer
Copy link

Maybe this will help fix the issue?
vitejs/vite#9938

@Tal500
Copy link
Contributor

Tal500 commented Oct 19, 2022

This seems like something that will have to be fixed in Vite. As a test, I removed all the <link rel="modulepreload"> elements using transformPageChunk, and saw the same result, which suggests the issue is Vite's module preloading logic. Not immediately sure what the fix is though.

As someone that was involved in this Vite issue, I it was fixed in vitejs/vite#9970, which is applied from vite>=3.2.0-beta.0.

Please test again and report if the issue continues.

@benmccann
Copy link
Member

Thanks for the update! I'll go ahead and close this now since it's been fixed in Vite. 3.2.0 final should be released soon

@benmccann
Copy link
Member

Vite 3.2 has been released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. vite
Projects
None yet
Development

No branches or pull requests

10 participants