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

Issue with vitePluginAnalyzer #7116

Closed
1 task done
loucyx opened this issue May 18, 2023 · 11 comments · Fixed by #7144
Closed
1 task done

Issue with vitePluginAnalyzer #7116

loucyx opened this issue May 18, 2023 · 11 comments · Fixed by #7144

Comments

@loucyx
Copy link
Contributor

loucyx commented May 18, 2023

What version of astro are you using?

2.4.5

Are you using an SSR adapter? If so, which one?

Vercel

What package manager are you using?

pnpm

What operating system are you using?

MacOS

What browser are you using?

Chrome

Describe the Bug

I found a bizarre bug with Astro's build: Sometimes, some hoisted JS files are imported but don't exist. What I mean by this is the produced hoisted js has something like:

import 'hoisted.somehash.js';

But that hoisted.somehash.js file doesn't exist at all. After digging for hours, I discovered that vitePluginAnalyzer from Astro is telling Vite to generate those hoisted references but is not considering Vite's build.assetsInlineLimit. So the problem was that, for some reason, my script inlined in an astro file was in the threshold of that build.assetsInlineLimit, and Vite was inlining it, so that's the reason it didn't exist in the final build.

The "solution" for now is to use vite.build.assetsInlineLimit in the Astro config and set it to 0 so everything "exists" for the imports defined by Astro, but I think it should be solved more elegantly.

This issue is weird because, for some reason, I can't reproduce it in Stackblitz, but maybe you all can figure it out. The link to reproduce is a Vercel deployment of my site with the assetsInlineLimit unset (which produces the broken state). You can see in the DevTools terminal the error with the missing hoisted file.

Here's the JS static log from Vercel:

# Unset assetsInlineLimit (broken)

_astro/hoisted.3d77b2bb.js    915 B
_astro/hoisted.e2e9241b.js    667 B
_astro/page.6cf2c0d7.js       1.75 kB

# assetsInlineLimit: 0 (working)

_astro/hoisted.3d77b2bb.js    915 B
_astro/hoisted.8ad81f47.js    1.6 kB # This is the file that's inlined
_astro/hoisted.e2e9241b.js    667 B
_astro/page.6cf2c0d7.js       1.75 kB

I'm willing to pair with anyone on the team, maybe through Discord, to show you the build output and help you folks out ^_^

Link to Minimal Reproducible Example

https://luke-hs72bavte-lukeshiru.vercel.app/

Participation

  • I am willing to submit a pull request for this issue.
@matthewp
Copy link
Contributor

@lilnasy possible this is related to the changes made for inlining stylesheets?

@lilnasy
Copy link
Contributor

lilnasy commented May 18, 2023

I doubt it, but I'll look into it.

@lilnasy
Copy link
Contributor

lilnasy commented May 18, 2023

@lukeshiru You've linked the production website, but I'll need the source code of a minimal project to investigate.

@loucyx
Copy link
Contributor Author

loucyx commented May 18, 2023

@lilnasy Sorry, but It's a private project. I added you as a collaborator so you can check it out. There's a broken branch on it with the config that produces the faulty build.

@lilnasy
Copy link
Contributor

lilnasy commented May 18, 2023

@lukeshiru looking into it now

@lilnasy
Copy link
Contributor

lilnasy commented May 18, 2023

@lukeshiru Can't reproduce; tested on node 16, 18, and 19.

Screenshot (174)

Note the commit hash.

@loucyx
Copy link
Contributor Author

loucyx commented May 18, 2023

@lilnasy I'm on node 20.2.0 locally, and Vercel has 18 set, so the issue shouldn't depend on Node's version. The log goes ok but check .vercel/output/static/_astro. You should see there that a hoisted script is missing, but it's being imported by the ones there. There should be 3 hoisted.hash.js files and one page.hash.js, but there are only 2 hoisted.hash.js and both do a reference to a missing third.

@lilnasy
Copy link
Contributor

lilnasy commented May 18, 2023

  1. The issue is that there are dangling references to inlined hoisted script in the client assets.
    Screenshot (175)

  2. @matthewp The issue has existed since before inlineStylesheets.
    Screenshot (176)

  3. @lukeshiru This change (not a solution) is sufficient to prevent the dangling reference, by also outputting the inlined script in the bundle.
    main...lilnasy:astro:patch-1

@loucyx
Copy link
Contributor Author

loucyx commented May 18, 2023

Thanks for digging into it so quickly and explaining it so clearly, @lilnasy! I'll keep the vite.build.assetsInlineLimit config until that patch goes live. You rock! 💖

@lilnasy
Copy link
Contributor

lilnasy commented May 18, 2023

This patch won't go live.

The root cause is Vite inserting import statements for modules that are completely unrelated: the referenced and the referee scripts are authored in two separate astro components, neither of which import anything. Maybe it's trying to ensure sequence: one should be executed before the other.

@loucyx
Copy link
Contributor Author

loucyx commented May 19, 2023

Oh yeah, sorry if I expressed myself incorrectly. I meant I'll keep that until it's fixed anywhere it needs to be fixed. Thanks again for the explanation!

I'II close! ✨

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

Successfully merging a pull request may close this issue.

3 participants