-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
refactor: use rollup hashing when emitting assets #10878
Conversation
SvelteKit and Rollup have one file naming scheme, but Vite has a different file naming scheme for inserting the hash into the file names (I don't recall the difference exactly, but think it was something like |
@benmccann we are using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some nitpicks but other looks good to me!
Co-authored-by: 翠 / green <green@sapphi.red>
Here the entry: https://github.com/vite-pwa/sveltekit/blob/main/src/config.ts#L53 This will go to Vite 4? We can break existing plugins, at least Vite PWA, we're forcing to detect Vite version? EDIT: PR sent to PWA Plugin to support both layouts, also sent PR to kit integration to remove it |
Done: This PR depends on rollup/rollup#4712, Rollup 3.3 includes it and we updated it in a separate PR.
Description
Rollup 3 has a new hashing algorithm that fixes the issues we had when emitting assets before:
We currently do our own hashing and deduplication, see:
This is the first PR in a series to try to move back some of the responsibilities to Rollup. This should help to be compatible with external plugins using
emitFile
or config settings likeassetFileNames
. There was actually a faulty test in workers that was not letting us see a bug for workers andassetFileNames
.Ideally, we should try to move to also use
import.meta.ROLLUP_FILE_URL_referenceId
instead of our__VITE_ASSET__hash__
. I still don't know if it is feasible though because we still need our own scheme for references in CSS. This PR already moves from our own hash to usingreferenceId
though, and future PRs may move us closer to at least using rollup's scheme in JS files.What is the purpose of this pull request?