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

Write asset string directly into source file instead of an imported file #18119

Closed
4 tasks done
benmccann opened this issue Sep 16, 2024 · 8 comments
Closed
4 tasks done
Labels
enhancement: pending triage performance Performance related enhancement

Comments

@benmccann
Copy link
Collaborator

Description

I want the Svelte compiler to be able to know that import img from './myimg.png' results in img being a string so that it can optimize it. The Svelte compiler doesn't look across files, which makes this quite difficult today.

Suggested solution

Write asset string directly into source file instead of in an imported file.

Today if you do import img from './myimg.png' it will leave the import in place and will have ./myimg.png be a JS file with default export.

What we could do instead is rewrite import img from './myimg.png' to something like const img = "__VITE_ASSET_a3de12__". At that point, it's clear that img is always defined and is a string without looking across files, which makes it much easier to do subsequent compiler optimizations on it.

https://github.com/vitejs/vite/blob/993d45bf45400157e0a7bec91e038fc87d598dac/packages/vite/src/node/plugins/asset.ts

Alternative

I tried to solve this directly in the Svelte compiler (sveltejs/svelte#13242), but it turns out that default exports can sometimes be live bindings which means it's not a safe optimization to make there, so it has to be done on the Vite side.

Additional context

The fact that it doesn't know the type of img is especially pathological on my site where importing a paid SVGs resulted in hundreds of lines of extra JS being generated totaling 4kb of extra JS.

Validations

@bluwy
Copy link
Member

bluwy commented Sep 17, 2024

I could see the benefit in dev to reduce the network calls, but in build it would still be better to not inline it so assets code are deduplicated? Maybe the string path isn't long enough to matter, but it could be long for relative base, which results in export default new URL(...).

If we can't do this in build, then the idea will probably not work for Svelte 🤔 Also, this will require the Svelte plugin to be a "normal" plugin to transform itself after the asset plugin.

@benmccann
Copy link
Collaborator Author

I guess the other option would be for vite-plugin-svelte to do it. We could split vite-plugin-svelte so that it resolves in pre and transforms in the "normal" stage. (CC @dominikg)

@dominikg
Copy link
Contributor

we would at least have to run preprocessors in pre as well, and then vite still has to understand svelte script blocks? not sure vps is the right place for this. it reads like a generic plugin not related to svelte itself.

also the duplication issue is something to consider. imagine an icon resused in 100 components, would that end up in multiple different chunks in the end?

@benmccann
Copy link
Collaborator Author

Maybe the solution then is to turn:

import img from './myimg.png';

To:

import __TMP_IMG_1__ from './myimg.png';
const img = __TMP_IMG_1__;

That would give the Svelte compiler a way to recognize that it's a constant. I could do that in @sveltejs/enhanced-img, but it would be nice if users doing manual image import also benefited. The cost of importing a single image can add multiple KB of needless hydration code (can I say once again how much I hate hydration).

@dominikg
Copy link
Contributor

It would be a constant with an unknown value though, so not sure if that changes much in terms of creating a simple string in enhanced-img. It would still require help from esbuild/rollup to inline the const into a template literal after the svelte compiler is done.

@benmccann
Copy link
Collaborator Author

Hmm. That's true. I could add a JSDoc to show it's a string, but I doubt Svelte knows how to handle that. I might be able to do const img = __TMP_IMG_1__.toString(); to prove it's a string and then update the Svelte compiler to handle that.

@benmccann
Copy link
Collaborator Author

Hmm. That would break hydration if it were import { BROWSER } from 'esm-env';

@benmccann
Copy link
Collaborator Author

Okay. New idea: sveltejs/svelte#13298

@github-actions github-actions bot locked and limited conversation to collaborators Oct 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement: pending triage performance Performance related enhancement
Projects
None yet
Development

No branches or pull requests

3 participants