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

feat: support for Vercel Images in enhanced-img #11979

Closed
wants to merge 1 commit into from

Conversation

leoj3n
Copy link
Contributor

@leoj3n leoj3n commented Mar 14, 2024

Usage:

sizes: [480, 1024, 1920, 2560],

vercel_sizes: [480, 1024, 1920, 2560]

<enhanced:img
  src="./svelte-kit-machine.webp?cdn=vercel&tint=ffaa22"
  sizes="(min-width: 768px) min(100vw, 108rem), 64rem"
  class="hero-image"
  alt="SvelteKit illustration"
/>

Notice the cdn=vercel&tint=ffaa22.

Output (after npm run dev):

<img
 srcset="/@imagetools/dd29267694d95adde023ac14c36bec9956893809 480w, /@imagetools/dd29267694d95adde023ac14c36bec9956893809 1024w, /@imagetools/dd29267694d95adde023ac14c36bec9956893809 1920w, /@imagetools/dd29267694d95adde023ac14c36bec9956893809 2560w"
 src="/@imagetools/dd29267694d95adde023ac14c36bec9956893809"
 sizes="(min-width: 768px) min(100vw, 108rem), 64rem"
 class="hero-image s-V_4r9JSKzHNM"
 alt="SvelteKit illustration"
 width="1440"
 height="1440"
 title="NOTE(dev mode): The srcset image URLs will be more like /_vercel/image?url=...?w=&amp;q= in production."
>

image

The scope of adding this functionality is kind of large given the different CDN providers.

Not sure how you all would want to go about it, but this is something I wanted for use with Vercel instead of having to use a component.


Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

  • Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

Copy link

changeset-bot bot commented Mar 14, 2024

⚠️ No Changeset found

Latest commit: 7cf6703

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dummdidumm
Copy link
Member

I think enhanced-img is the wrong place to add this functionality. The package is mainly about static image optimization, I'm not sure if/how we should put dynamic image optimization into it - but definitely not through special-casing providers. There would need to some kind of adapter pattern.

@benmccann benmccann changed the title Example of how to add support for Vercel to enhanced-img feat: support for Vercel Images in enhanced-img Mar 15, 2024
@benmccann
Copy link
Member

I'm afraid I have to agree with Simon. Something like @unpic/svelte, which already has Vercel support seems like a better fit for this type of functionality

@leoj3n
Copy link
Contributor Author

leoj3n commented Mar 15, 2024

@benmccann Except they ship components which is bloated compared to a vite plugin.

@benmccann
Copy link
Member

Maybe better as a third-party preprocessor then. The idea of enhanced-img is to be entirely build-time and I'm afraid it starts getting murky if we start integrating CDNs into it

As far as the weight of @unpic/svelte goes, I think it could be reduced with something like ascorbic/unpic#96

@leoj3n
Copy link
Contributor Author

leoj3n commented Mar 15, 2024

Maybe better as a third-party preprocessor

That's why I originally had this titled "Example..." not "feat".

How would you go about sharing the sizes array between adapter-vercel and enhanced-vercel-img? I couldn't figure out how to access the svelte adapter-vercel config from within this plugin.

Best I could come up with was importing a file like allowed-vercel-image-sizes.js in svelte.config.js and vite.config.js that looks like that:

export default [480, 1024, 1920, 2560]

Then passing that along.

I don't think using components will outperform the standard <img> tag.

@benmccann
Copy link
Member

That's why I originally had this titled "Example..." not "feat".

Perhaps it's a bit unclear what the intention with this PR was. It was editing packages/enhanced-img, so it seemed like it was adding a feature to the existing preprocessor. If it's just an example of how to build such a preprocessor it probably shouldn't be sent as a PR to update the existing one

How would you go about sharing the sizes array between adapter-vercel and enhanced-vercel-img?

I don't think adapter options get passed to Vite plugins

Best I could come up with was importing a file like allowed-vercel-image-sizes.js in svelte.config.js and vite.config.js

Yeah, that's probably the best solution for now

@leoj3n
Copy link
Contributor Author

leoj3n commented Mar 15, 2024

@benmccann Note that Vercel's API doesn't have any support for cropping, only resize, and that unpic has no support for imagetools, so this approach has the benefit of allowing transformations that are not supported by the CDN.

I was thinking perhaps enhanced-img could accept some kind of callback interface since the bulk of the code here is quite complex, where we can plug into enhanced-img to transform the result outside of a <picture> element result.

@leoj3n
Copy link
Contributor Author

leoj3n commented Mar 15, 2024

Hmmm... So trying to get this to work in reality (production) is not easy.

This function in preprocessor.js:

async function process(resolved_id, opts) {
	if (!opts.imagetools_plugin.load) {
		throw new Error('Invalid instance of vite-imagetools. Could not find load method.');
	}
	const hook = opts.imagetools_plugin.load;
	const handler = typeof hook === 'object' ? hook.handler : hook;
	const module_info = await handler.call(opts.plugin_context, resolved_id);
	// ...
}

Returns export default{sources:{},img:{src:"__VITE_ASSET__Ce96qHrT__",w:2000,h:1334}}; and that __VITE_ASSET__Ce96qHrT__ gets used everywhere until generateBundle.

We can try and hijack it at this point with something like this in index.js:

/**
 * @param {{ vercel_sizes: number[] }} options
 * @returns {Promise<import('vite').Plugin[]>}
 */
export async function enhancedImages(options) {
	const imagetools_instance = await imagetools_plugin();
	const image_plugin_instance = image_plugin(imagetools_instance, options);
	return !process.versions.webcontainer
		? [image_plugin_instance, imagetools_instance, vercel_encodeURIComponent_plugin()]
		: [];
}

/**
 * @returns {import('vite').Plugin}
 */
function vercel_encodeURIComponent_plugin() {
	return {
		name: 'vite-plugin-vercel-encodeuricomponent',
		enforce: 'post',
		async generateBundle(_, output_bundle) {
			Object.values(output_bundle).filter((element) => {
				return (element.fileName.endsWith('.svelte.js'));
			}).map((obj) => {
				// @ts-ignore
				const ms = new MagicString(output_bundle[obj.fileName].code);
				ms.replaceAll(new RegExp(/(\/_vercel\/image\?url=)(.+?)(&w=)/, 'g'), (_, $1, $2, $3) => {
					return $1 + encodeURIComponent($2) + $3;
				});
				if (ms.hasChanged()) {
					// @ts-ignore
					output_bundle[obj.fileName].code = ms.toString();
				}
			})
		},
	}
}

Which gets us the following unhydrated HTML sent to the browser:

<img alt="" fetchpriority=high height=1334 src=/_app/immutable/assets/14.3BN3nevY.jpg width=2000 sizes="
		(min-width: 50rem) 27rem,
		(min-width: 43.875rem) calc(100vw - 7.5rem),
		calc(100vw - 4rem)" srcset="/_vercel/image?url=%2F_app%2Fimmutable%2Fassets%2F14.3BN3nevY.jpg&w=80&q=99 80w, /_vercel/image?url=%2F_app%2Fimmutable%2Fassets%2F14.3BN3nevY.jpg&w=240&q=99 240w, /_vercel/image?url=%2F_app%2Fimmutable%2Fassets%2F14.3BN3nevY.jpg&w=300&q=99 300w, /_vercel/image?url=%2F_app%2Fimmutable%2Fassets%2F14.3BN3nevY.jpg&w=328&q=99 328w, /_vercel/image?url=%2F_app%2Fimmutable%2Fassets%2F14.3BN3nevY.jpg&w=540&q=99 540w, /_vercel/image?url=%2F_app%2Fimmutable%2Fassets%2F14.3BN3nevY.jpg&w=626&q=99 626w, /_vercel/image?url=%2F_app%2Fimmutable%2Fassets%2F14.3BN3nevY.jpg&w=640&q=99 640w, /_vercel/image?url=%2F_app%2Fimmutable%2Fassets%2F14.3BN3nevY.jpg&w=780&q=99 780w, /_vercel/image?url=%2F_app%2Fimmutable%2Fassets%2F14.3BN3nevY.jpg&w=860&q=99 860w, /_vercel/image?url=%2F_app%2Fimmutable%2Fassets%2F14.3BN3nevY.jpg&w=1000&q=99 1000w, /_vercel/image?url=%2F_app%2Fimmutable%2Fassets%2F14.3BN3nevY.jpg&w=1200&q=99 1200w, /_vercel/image?url=%2F_app%2Fimmutable%2Fassets%2F14.3BN3nevY.jpg&w=1400&q=99 1400w, /_vercel/image?url=%2F_app%2Fimmutable%2Fassets%2F14.3BN3nevY.jpg&w=1600&q=99 1600w, /_vercel/image?url=%2F_app%2Fimmutable%2Fassets%2F14.3BN3nevY.jpg&w=1800&q=99 1800w, /_vercel/image?url=%2F_app%2Fimmutable%2Fassets%2F14.3BN3nevY.jpg&w=2000&q=99 2000w">

Copying and pasting these srcset URI encoded URLs prove to be working and resized by Vercel.

However, since Vite was using that __VITE_ASSET__Ce96qHrT__ asset identifier it looks like it is also picked up by SvelteKit and so when the HTML gets hydrated like so:

        h() {
            S(e, o = "/_vercel/image?url=" + new URL("../assets/14.3BN3nevY.jpg",import.meta.url).href + "&w=80&q=99 80w, /_vercel/image?url=" + new URL("../assets/14.3BN3nevY.jpg",import.meta.url).href + "&w=240&q=99 240w, /_vercel/image?url=" + new URL("../assets/14.3BN3nevY.jpg",import.meta.url).href + "&w=300&q=99 300w, /_vercel/image?url=" + new URL("../assets/14.3BN3nevY.jpg",import.meta.url).href + "&w=328&q=99 328w, /_vercel/image?url=" + new URL("../assets/14.3BN3nevY.jpg",import.meta.url).href + "&w=540&q=99 540w, /_vercel/image?url=" + new URL("../assets/14.3BN3nevY.jpg",import.meta.url).href + "&w=626&q=99 626w, /_vercel/image?url=" + new URL("../assets/14.3BN3nevY.jpg",import.meta.url).href + "&w=640&q=99 640w, /_vercel/image?url=" + new URL("../assets/14.3BN3nevY.jpg",import.meta.url).href + "&w=780&q=99 780w, /_vercel/image?url=" + new URL("../assets/14.3BN3nevY.jpg",import.meta.url).href + "&w=860&q=99 860w, /_vercel/image?url=" + new URL("../assets/14.3BN3nevY.jpg",import.meta.url).href + "&w=1000&q=99 1000w, /_vercel/image?url=" + new URL("../assets/14.3BN3nevY.jpg",import.meta.url).href + "&w=1200&q=99 1200w, /_vercel/image?url=" + new URL("../assets/14.3BN3nevY.jpg",import.meta.url).href + "&w=1400&q=99 1400w, /_vercel/image?url=" + new URL("../assets/14.3BN3nevY.jpg",import.meta.url).href + "&w=1600&q=99 1600w, /_vercel/image?url=" + new URL("../assets/14.3BN3nevY.jpg",import.meta.url).href + "&w=1800&q=99 1800w, /_vercel/image?url=" + new URL("../assets/14.3BN3nevY.jpg",import.meta.url).href + "&w=2000&q=99 2000w") || r(e, "srcset", o),
            r(e, "sizes", u[0]),
            r(e, "fetchpriority", "high"),
            r(e, "alt", G()),
            I(e.src, s = "" + new URL("../assets/14.3BN3nevY.jpg",import.meta.url).href) || r(e, "src", s),
            r(e, "width", "2000"),
            r(e, "height", "1334")
        },

The previously relative and encodeURIComponent URLs are replaced with fully qualified URLs like that (which don't look to be URI encoded):

/_vercel/image?url=https://the-origin-domain.vercel.app/_app/immutable/assets/14.3BN3nevY.jpg&w=80&q=99

Now, while I would prefer to have the URLs be relative and explicitly URI encoded, it looks like the swapped out URLs can be made to work by specifying domains to the config for adapter-vercel:

/** @type {import('@sveltejs/kit').Config} */
const config = {
	preprocess: vitePreprocess(),
	kit: {
		adapter: adapter({
			images: {
				minimumCacheTTL: 300,
				formats: ['image/avif', 'image/webp'],
				sizes: allowedSizes,
				domains: ['the-origin-domain.vercel.app']
			}
		}),

Now that works:

<img alt="Property Photo" fetchpriority="high" height="1334" src="/_app/immutable/assets/14.3BN3nevY.jpg" width="2000"
    sizes="
		(min-width: 50rem) 27rem,
		(min-width: 43.875rem) calc(100vw - 7.5rem),
		calc(100vw - 4rem)"
    srcset="/_vercel/image?url=https://the-origin-domain.vercel.app/_app/immutable/assets/14.3BN3nevY.jpg&amp;w=80&amp;q=99 80w, /_vercel/image?url=https://the-origin-domain.vercel.app/_app/immutable/assets/14.3BN3nevY.jpg&amp;w=240&amp;q=99 240w, /_vercel/image?url=https://the-origin-domain.vercel.app/_app/immutable/assets/14.3BN3nevY.jpg&amp;w=300&amp;q=99 300w, /_vercel/image?url=https://the-origin-domain.vercel.app/_app/immutable/assets/14.3BN3nevY.jpg&amp;w=328&amp;q=99 328w, /_vercel/image?url=https://the-origin-domain.vercel.app/_app/immutable/assets/14.3BN3nevY.jpg&amp;w=540&amp;q=99 540w, /_vercel/image?url=https://the-origin-domain.vercel.app/_app/immutable/assets/14.3BN3nevY.jpg&amp;w=626&amp;q=99 626w, /_vercel/image?url=https://the-origin-domain.vercel.app/_app/immutable/assets/14.3BN3nevY.jpg&amp;w=640&amp;q=99 640w, /_vercel/image?url=https://the-origin-domain.vercel.app/_app/immutable/assets/14.3BN3nevY.jpg&amp;w=780&amp;q=99 780w, /_vercel/image?url=https://the-origin-domain.vercel.app/_app/immutable/assets/14.3BN3nevY.jpg&amp;w=860&amp;q=99 860w, /_vercel/image?url=https://the-origin-domain.vercel.app/_app/immutable/assets/14.3BN3nevY.jpg&amp;w=1000&amp;q=99 1000w, /_vercel/image?url=https://the-origin-domain.vercel.app/_app/immutable/assets/14.3BN3nevY.jpg&amp;w=1200&amp;q=99 1200w, /_vercel/image?url=https://the-origin-domain.vercel.app/_app/immutable/assets/14.3BN3nevY.jpg&amp;w=1400&amp;q=99 1400w, /_vercel/image?url=https://the-origin-domain.vercel.app/_app/immutable/assets/14.3BN3nevY.jpg&amp;w=1600&amp;q=99 1600w, /_vercel/image?url=https://the-origin-domain.vercel.app/_app/immutable/assets/14.3BN3nevY.jpg&amp;w=1800&amp;q=99 1800w, /_vercel/image?url=https://the-origin-domain.vercel.app/_app/immutable/assets/14.3BN3nevY.jpg&amp;w=2000&amp;q=99 2000w">

But I think the browser is helping us out here as those should technically be URI encoded.

It's almost like we need partial hydration here. Although really the problem starts with when imagetools returns a asset identifier. I'm only just learning about vite and plugins so it is not clear to me if that asset identifier can be resolved early so we can use it as an URI encoded query param, and stop SvelteKit from hydrating it?

Any other ideas?

@Rich-Harris
Copy link
Member

Rich-Harris commented Mar 15, 2024

I don't think there's any real value in using an image CDN for static images. Just process them at build time and serve them with the rest of your assets. Otherwise you're going to have to pay for them!

For dynamic images, I've used this approach to good effect in the past:

import { dev } from '$app/environment';

export function optimize(src: string, widths = [640, 960, 1280], quality = 90) {
  if (dev) return src;

  return widths
    .slice()
    .sort((a, b) => a - b)
    .map((width, i) => {
      const url = `/_vercel/image?url=${encodeURIComponent(src)}&w=${width}&q=${quality}`;
      const descriptor = i < widths.length - 1 ? ` ${width}w` : '';
      return url + descriptor;
    })
    .join(', ');
}

Usage:

<img alt={photo.description} srcset={optimize(photo.url)} />

Ways to improve it:

  • expose that from @sveltejs/adapter-vercel/image or similar (though if you set custom sizes then we need a way to configure that without having to pass them in every time you call optimize)
  • augment adapters such that we can simulate the /_vercel/image route in dev, rather than just using the original URL

As for keeping sizes in sync between adapter config and wherever you configure the optimize function... I mean, it's just an array of three-ish numbers that will change very infrequently (and can have sensible defaults that most people wouldn't need to touch), I don't think we need to overengineer this. But if you really did need it then a allowed-vercel-image-sizes.js module seems like a perfectly cromulent solution.

@benmccann
Copy link
Member

I appreciate the effort on this, but I'm going to go ahead and close it given the comments here

@benmccann benmccann closed this Mar 15, 2024
@leoj3n
Copy link
Contributor Author

leoj3n commented Mar 23, 2024

I don't think there's any real value in using an image CDN for static images.

@Rich-Harris It sounds like you might have a more behind-the-scenes understanding of how Vercel caches images on the edge... Are you saying images pumped through https://vercel.com/docs/image-optimization are cached and served exactly the same as if they were just part of the initial deploy? F.ex is there really no difference between /my-image.jpg and /_vercel/image?url=%2Fmy_image.jpg&w=800&q=99, other than perhaps serving the image in an "optimized" format / resizing the image / adjusting the quality? The way those linked docs read, it makes it sounds like images pumped through the image optimization CDN are cached differently at the edge, and it is/was my intuition that images explicitly going through the CDN are going to be served better than if they were treated like regular deploy-time assets. Although it may be that someone like you who has more inside knowledge of how Vercel works can verify there is no benefit.

Regarding my original inquiry about how can we resolve the identifier returned by imagetools, to answer my own question it looks like we may need to wait until Vite exports the following function for external plugins to use:

https://github.com/vitejs/vite/blob/6a07243a0ecc06a81e87d5b3de7b88023a045e6b/packages/vite/src/node/plugins/asset.ts#L269-L279

https://github.com/vitejs/vite/pull/7167
|_ https://github.com/vitejs/vite/issues/7162
    |_ https://github.com/vitejs/vite/issues/13459

I may try making a fork of Vite to do this now, as I do have the use case for wanting to do this outside of the potential(?) caching and serving benefits (hint: to support doing things like <link as="image" rel="preload" fetchpriority="high" imagesizes=... imagesrcset=... />; note that browsers don't have a feature like this for <picture> with <sources>, only currently a possibility for <img srcset> relying on the 304 dance such as used by Vercel to serve the optimized format).

Am I wrong in thinking creating a totally static <img /> tag at build time is going to outperform using a component and hydrating? It seems to me that if we can create a static <img /> tag this will be the ideal and most performant option.

@benmccann
Copy link
Member

enhanced-img produces images that are output to SvelteKit's immutable folder and cached forever. If you want to use edge serving, multiple adapters have an edge option that you can enable

<img /> tags must also be hydrated

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 this pull request may close these issues.

None yet

4 participants