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

add format: 'webp' to support Astro automatically. #13

Open
1 task done
hexiro opened this issue Jul 5, 2024 · 8 comments
Open
1 task done

add format: 'webp' to support Astro automatically. #13

hexiro opened this issue Jul 5, 2024 · 8 comments

Comments

@hexiro
Copy link

hexiro commented Jul 5, 2024

Description

Hello, first I want to say thank you for making this amazing package! this is exactly what I was looking for!
I have absolutely no complaints but wanted to suggest something to see if you would be interested in adding this to your package.

Screenshot 2024-07-05 at 7 35 11 PM

I am using Astro with this plugin and Astro's image component requires the props src, width, height, and format. using your plugin with ?lqip, lqip is of course added but format is left behind. this is far from the end of the world as the solution i have been using (pictured above) works lovely.

with that being said, I don't think it would neccesarily be a bad idea to pass to output the format even though it's just going to be webp everytime, so that it automatically satisfies Astro's ImageMetadata type, and becomes even more out-the-box for those users.

Please let me know what thoughts you have on this! and once again this package is easy to use and convenient! thank you so much!

@hexiro hexiro changed the title add format: '.webp' to support Astro automatically. add format: 'webp' to support Astro automatically. Jul 5, 2024
@hexiro
Copy link
Author

hexiro commented Jul 5, 2024

this is so specific it probably doesn't even matter, i'm just thinking about people less familiar with TypeScript or web development

@drwpow
Copy link
Owner

drwpow commented Jul 6, 2024

That’s a great suggestion! Would love a PR if you’re able to add one. No worries if not though!

@hexiro
Copy link
Author

hexiro commented Jul 6, 2024

will do!!!

@hexiro
Copy link
Author

hexiro commented Jul 6, 2024

That’s a great suggestion! Would love a PR if you’re able to add one. No worries if not though!

hmmm so i have encountered a bit of a problem.
image

i was under the impression that the output would always be a webp because of this,

image

but i see now that the mimetype is used to be the same as the original image -- i'm not exactly sure how this works. I certainly think it could be beneficial to add format to the output but it wouldn't fill my original use case as sharp's formats and Astro's formats don't line up 1:1 so a type issue could still occur there.

let me know if i'm incorrect in any of this or where you would like to go from here :)

@drwpow
Copy link
Owner

drwpow commented Jul 7, 2024

but i see now that the mimetype is used to be the same as the original image -- i'm not exactly sure how this works.

Oh I see; it’s because original is the original, untouched image. So yes I think you’re correct format should probably match the source image. But as a hint, width and height are pulled from the original so there’s a way to grab format as well

@hexiro
Copy link
Author

hexiro commented Jul 8, 2024

but i see now that the mimetype is used to be the same as the original image -- i'm not exactly sure how this works.

width and height are pulled from the original so there’s a way to grab format as well

hi, sorry for late reply.
I found the format fyi it's metadata.format. what i'm trying to say though is that the type overide that includes format that the user would need to add to their globals.d.ts could use sharp's image type or potentially just string as an override wouldn't satisfy Astro's ImageMetadata format type, which is more specific. i was originally under the incorrect assumption that all lqip images were webp so that the type would just be the literal "webp". I don't think adding format would be a bad idea but

346237883-16ffcf33-1ce3-4f25-8e28-b57b9841d48e

it wouldn't matter in this specific use case as either this or AboutSrc as ImageMetadata would still needed to be used as it wouldn't satisfy ImageMetadata out of the box.

I don't mind adding it, but the PR name should be changed from:
add format: 'webp' to support Astro automatically.

to something like
add image format to lqip output

@drwpow
Copy link
Owner

drwpow commented Jul 17, 2024

Ah I think we’re probably touching on some confusion on how this should be used in Astro, which should probably be part of the docs. All LQIP images are webp, always. But depending on how you use it, you may want it as src (if you’re swapping via JS), or if using no JS you may want it on a background element (via CSS, etc.). This plugin isn’t prescriptive on how it gets used.

That said, I think it’s better to just have an example that makes it easy to use in Astro so people can just copy–paste an example and It Just Works™.

I’ll be working on an Astro project next month that will use this plugin again (had a gap between the last Astro project; I used it before the <Image> component existed), and I’ll try all your suggestions and will have more of an opinion at a later date. But until then, if you want to just make a decision on how you would like to use it, I’d be open to a PR and version release 🙂.

@hexiro
Copy link
Author

hexiro commented Jul 21, 2024

Ah I think we’re probably touching on some confusion on how this should be used in Astro, which should probably be part of the docs. All LQIP images are webp, always. But depending on how you use it, you may want it as src (if you’re swapping via JS), or if using no JS you may want it on a background element (via CSS, etc.). This plugin isn’t prescriptive on how it gets used.

That said, I think it’s better to just have an example that makes it easy to use in Astro so people can just copy–paste an example and It Just Works™.

I’ll be working on an Astro project next month that will use this plugin again (had a gap between the last Astro project; I used it before the <Image> component existed), and I’ll try all your suggestions and will have more of an opinion at a later date. But until then, if you want to just make a decision on how you would like to use it, I’d be open to a PR and version release 🙂.

Ahh I see what you're saying. This isn't a huge pain point but documenting it somewhere would be 👍🏼 would you like me to add an use with Astro example to the readme? A small section under Optimizing with vite-imagestools would make sense to me.

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

No branches or pull requests

2 participants