-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
feat: image preprocessor #10788
feat: image preprocessor #10788
Conversation
🦋 Changeset detectedLatest commit: a56c459 The changes in this PR will be included in the next version bump. 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 |
Yesterday Angular rolled a big update, with that functionality included: https://angular.dev/guide/image-optimization |
- move ambient declarations into its own file and make sure they are loaded when anything from the package is loaded by importing it in the root types file - move element augmentation into root types file. Needs to be there because it can't be in an ambient module, the augmentations wouldn't work in such a module - add fix to SvelteKit to ensure vite.config.js is also loaded, so that when someone uses svelte:image with a vite.config.js/ts its types are loaded
Great work on this! @benmccann @Rich-Harris I'm slightly late on this, but a recent thought on naming: If this were named |
That's true there. I've stumbled across it a few times, and it got me to the point where I want to use |
Implements the preprocessor described in #241. Please read both the description of that issue and the README in this PR for more details.
Together with with the dynamic solution in #10323 this is most of what we need to handle images. This pair of PRs would replace #9787.
Package setup
I named it
@sveltejs/enhanced-img
, which is updated from@sveltejs/image
in the earlier PR since this one is focused on the static case and we'll have a dynamic one as wellThis makes
vite-imagetools
a dependency.vite-imagetools
includessharp
, which is a bit heavy and platform-specific, so we wouldn't want to include it for the dynamic case. However, for the static case, this means users don't need to add two packages to theirpackage.json
as in the earlier PR, which required installingvite-imagetools
as well.However, if we have a dynamic preprocessor as well it will have to live in a separate package. This means that users could end up having to install
@sveltejs/enhanced-img
and@sveltejs/dynamic-img
and instantiate both of them. I.e. one possible idea would be to combine this PR with thegetImage
PR, to have an option to transformto:
This could be a little more natural for the user to write.
Performance
Inserting
picture
markup for each individual image could bloat the HTML-creation JS. E.g. this REPL outputs 42 lines of code in Svelte 5 today while this REPL outputs 67 lines of code. Compare this to a component-based approach where you have 19+89 lines of output for a single image and 22+89 lines of code for two images.However, we resolve the import to the image and inline the values such that a single image would output something like this and two images would output something like this.
It should be noted that the preprocessor also has the advantage of not needing to ship
getWidths
to the client, which saves about another 25 lines of code.There's still some optimization that can be done in the Svelte 5 compiler to optimize the dynamic part a bit to better handle non-reactive values in template strings.
Passing transformed image imports to non-transformed
<img>
Right now this only has
vite-imagetools
transform images with theenhanced-img
query parameter to avoid taking over existing imports. We have the preprocessor add this automatically. You must add it manually when you want to dynamically choose an image, but that's probably infrequent enough that it's okay.The other way of dealing with this would be to transform all imported images by default and users can opt-out with
?url
. I think it's pretty rare that you'd want to import an image and not transform it and can't really think of when you'd want to do that, so that option does have an appeal, but it could be unexpected. If we went this route it would be more important to work on our error messages as noted below.It's an error if you pass a transformed
img
to a standardimg
tag. The error message right now isn't great and can be slightly hard to diagnose. It ends up generating HTML that looks like<img src="[object Object]" />
which then makes a request giving the error message:I can think of a few ways we might be able to detect this:
img
tags with a mustachesrc
into preprocessing in DEV mode and then add a check that it's being passed a string/[object%20Object]
as URL in our 404 handling and suggest you might have forgotten a<!-- static-img-enable -->
and shouldview-source:
to see where needs to be updatedsrc="[object Object]"
in the generated page source and print a warning similar to the one aboveIf there's someplace a user was using an imported image other than an image tag most of these checks wouldn't save us, but that seems less likely to happen. The only thing I can think is if they were trying to output some sort of meta data via
{@html}
.Configuring
For now, the preprocessor doesn't take any options. This keeps the API smaller and make it easier to land this PR. We'll probably have to add some options in the future, but it's not necessary to get an initial usable version out.
There are some options like
deviceSizes
that we may want to share with the dynamic implementation and have live in the Svelte config. I think the dynamic approach needs to advance a bit more before we can say what it should look like thoughWe could have the user add another instance of
vite-imagetools
for anything custom they want to do. However, this could be very frustrating because you'd be forced to create anImage
component to use with it, which would be not only cumbersome, but would reintroduce the problems we're trying to avoid here by having a preprocessor rather than a component (i.e. event handling and usage of:global
for styling). It might also force you to setupsvelte-preprocess-import-assets
yourself. ExposingdefaultDirectives
in some manner would probably be quite appreciatedFor reference, here are all the options in vite-imagetools
include
/exclude
- it might be better for us to have our own filter or we might want to apply these filters to the URLs we find inimg
tags to decide whether to process that tagdefaultDirectives
- could be useful. we would want to ensure correctas
directive is used (as=picture
unless there's a single format). there aren't too many directives you'd want to set on every image (background
,flatten
,grayscale
,lossless
might be some of the most likely). but some are useful - e.g.background
plusflatten
would further reduce file size. you may also want to create your own presetsextendTransforms
- a common use case is the low quality placeholder discussed in the next bullet. could also be useful in other advanced cases, but seems uncommonextendOutputFormats
- it seems the main use case for this is the low-quality placeholder. i'm not sure this would be possible for users to do with this package without us explicitly adding support for itresolveConfigs
- this is pretty niche and not too likely to be usedremoveMetadata
- leaving metadata on optimized could be useful, but I expect uncommon. I think this should be made a directive instead of an option invite-imagetools
Including in the default SvelteKit template
For now, this is not included in the default template. However, after we've released the dynamic one and let this bake for a little while, I think it would be nice to add it.
Todo
Later
imagetools
improvements:imagetools-core
intovite-imagetools
without breakingastro-imagetools