docs: revise documentation for best image practices #11792

leoj3n · 2024-02-03T11:38:40Z

This PR adds information about compression, priority, cumulative layout shift, and using rem (#11791).

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.

Add information about making priority, using rem, etc.

Add information about compression, priority, cumulative layout shift, and using rem.

changeset-bot · 2024-02-03T11:38:44Z

⚠️ No Changeset found

Latest commit: 52ba694

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

Add information about compression, priority, cumulative layout shift, and using rem.

documentation/docs/40-best-practices/07-images.md

leoj3n · 2024-02-17T11:54:42Z

@benmccann When working with some SVG, I noticed if I put it in a component like I see often suggested, then the SVG nodes get parsed by Svelte and end up duplicated in the prerendered HTML as well as the JavaScript bundle.

If you do {@html `<svg>...</svg>`} at least the nodes aren't all parsed and tracked with class applied to them, but the string is still duplicated in the JavaScript.

For this reason I opted to link to a "stacked" SVG like so:

<svg xmlns="http://www.w3.org/2000/svg">
    <style>
        svg { display: none }
        svg:target { display: inline }
    </style>

    <svg id="key" viewBox="0 0 876 240">
        <defs>
            <linearGradient ...

<img src="/svg/above-the-fold.svg#key" alt="" loading="eager" fetchpriority="high" />

This works. However this file is only 5kb after compression so I decided it might be best to inline it again, but to avoid having it duplicated in the JavaScript bundle, I put it inside the <body> tag of app.html:

And linked to the symbols like so:

<svg aria-hidden="true">
  <use href="#key" />
</svg>

Do you think an explanation like this would be worth adding to the best image practices documentation? Is there some other way to have svelte not duplicate the SVG in JS?

benmccann · 2024-03-14T19:12:26Z

Do you think an explanation like this would be worth adding to the best image practices documentation? Is there some other way to have svelte not duplicate the SVG in JS?

I tend to think that we shouldn't document this for a couple of reasons. The first is that it probably only helps for images in your layout - if it's on a sub-page then you're including the image on the page without necessarily using it. The bigger issue in my mind though is that this doesn't seem like the ideal long-term fix. Instead, Svelte should figure out how to support something like partial hydration. See #1390 for a discussion of that. It's something we're thinking about, but won't get too soon as it's tricky to figure out what to do. I'll post some updated thoughts there shortly.

leoj3n · 2024-03-14T22:11:53Z

@benmccann What about a <svelte:body> tag as a stopgap for now?

benmccann · 2024-03-14T22:46:33Z

I think there may have been a <svelte:body> in the past, which did something a bit different, so that could end up being confusing. Anyway, I don't think anything's likely to happen until after the initial release of Svelte 5 as everyone's focused on that at the moment.

I'm not sure what you think about my suggestions above, but if you don't have any objections to them I could merge them into my PR. I did have one question about whether there are any references for more detailed info here: #11792 (comment)

leoj3n · 2024-03-14T23:56:52Z

I did have one question about whether there are any references for more detailed info here:

Here was my reply to (that you may have missed):

Main point was to have more information that just "set priority" which was unclear to me as someone just starting to read about image optimizing and this document being on of the launching points. From what I gather decoding is something you can try and see if it helps with for example lighthouse score or from wht you can observe on throttled cpu/network, but it is not likely to make a big difference. I found this to be a good resource that touches on this as well as the other things:

https://web.dev/learn/design/responsive-images

As it seems you want to keep this document terse, I think a link to a resource like that would be good here regardless of what you decide.

That website explains:

https://web.dev/learn/design/responsive-images#preloading_hints

The decoding attribute will not change how fast the image decodes, but merely whether the browser waits for this image decoding to happen before rendering other content.

In most cases this will have little impact, however in certain scenarios it can allow the image or content to be displayed slightly faster. For example, for a large document with lots of elements that take time to render, and with large images that take a while to decode, setting sync on important images will tell the browser to wait for the image and render both at once. Alternatively, setting async can allow the content to be displayed faster without waiting for the image decode.

However, the better option is usually to try to avoid excessive DOM sizes and ensure responsive images are used to reduce decoding time meaning the decoding attribute will have little effect.

Furthermore, from MDN:

https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/decoding

No preference for the decoding mode; the browser decides what is best for the user. This is the default value, but different browsers have different defaults:

Chromium defaults to "sync".
Firefox defaults to "async".
Safari defaults to "sync" except in a small number of circumstances.

In reality, the differences between the two values are often difficult to perceive and, where there are differences, there is often a better way.

For images that are inserted into the DOM inside the viewport, "async" can result in flashes of unstyled content, while "sync" can result in small amounts of jank.

For images inserted into the DOM outside of the viewport, modern browsers will usually decode them before they are scrolled into view and there will be no noticeable difference using either value.

As we are discussing what is best for an above-the-fold high-priority image, this line seems pertinent:

For images that are inserted into the DOM inside the viewport, "async" can result in flashes of unstyled content, while "sync" can result in small amounts of jank.

However, there is other info there about decoding often having a defualt of sync already, and general idea that it shouldn't need to be set on a typical website... However, libraries like unpic default to setting the value to async, although I don't know if they have a solid technical reason for that or not.

benmccann · 2024-03-15T03:43:14Z

Thanks. MDN says:

In reality, the differences between the two values are often difficult to perceive and, where there are differences, there is often a better way.

And web.dev says:

In most cases this will have little impact, however in certain scenarios it can allow the image or content to be displayed slightly faster. ... However, the better option is usually to try to avoid excessive DOM sizes and ensure responsive images are used to reduce decoding time meaning the decoding attribute will have little effect.

Given that it seems to have little impact I think I'll remove that part

leoj3n added 2 commits February 3, 2024 04:57

Update 07-images.md

8c5260a

Add information about making priority, using rem, etc.

Update 07-images.md

b976282

Add information about compression, priority, cumulative layout shift, and using rem.

leoj3n changed the title ~~Revise documentation for best image practices~~ docs: revise documentation for best image practices Feb 3, 2024

leoj3n added 3 commits February 3, 2024 05:42

Update 07-images.md

5ed1c54

Add information about compression, priority, cumulative layout shift, and using rem.

Update 07-images.md

753ec98

Add information about compression, priority, cumulative layout shift, and using rem.

Update 07-images.md

667e9f5

Add information about compression, priority, cumulative layout shift, and using rem.

benmccann added documentation Improvements or additions to documentation pkg:enhanced-img labels Feb 8, 2024

benmccann reviewed Feb 8, 2024

View reviewed changes

documentation/docs/40-best-practices/07-images.md Outdated Show resolved Hide resolved

benmccann reviewed Feb 8, 2024

View reviewed changes

documentation/docs/40-best-practices/07-images.md Outdated Show resolved Hide resolved

benmccann reviewed Feb 8, 2024

View reviewed changes

documentation/docs/40-best-practices/07-images.md Outdated Show resolved Hide resolved

benmccann reviewed Feb 8, 2024

View reviewed changes

documentation/docs/40-best-practices/07-images.md Outdated Show resolved Hide resolved

benmccann added 4 commits March 14, 2024 20:52

Update documentation/docs/40-best-practices/07-images.md

359bd27

Update documentation/docs/40-best-practices/07-images.md

c8ab6f4

Update documentation/docs/40-best-practices/07-images.md

943989a

Update documentation/docs/40-best-practices/07-images.md

52ba694

vercel bot deployed to Preview – kit-default-template March 15, 2024 03:57 View deployment

benmccann approved these changes Mar 15, 2024

View reviewed changes

vercel bot deployed to Preview – kit-svelte-dev March 15, 2024 03:58 View deployment

benmccann merged commit 5132c32 into sveltejs:main Mar 15, 2024
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: revise documentation for best image practices #11792

docs: revise documentation for best image practices #11792

leoj3n commented Feb 3, 2024 •

edited

Loading

changeset-bot bot commented Feb 3, 2024 •

edited

Loading

leoj3n commented Feb 17, 2024 •

edited

Loading

benmccann commented Mar 14, 2024

leoj3n commented Mar 14, 2024

benmccann commented Mar 14, 2024

leoj3n commented Mar 14, 2024 •

edited

Loading

benmccann commented Mar 15, 2024

docs: revise documentation for best image practices #11792

docs: revise documentation for best image practices #11792

Conversation

leoj3n commented Feb 3, 2024 • edited Loading

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

Tests

Changesets

Edits

changeset-bot bot commented Feb 3, 2024 • edited Loading

⚠️ No Changeset found

leoj3n commented Feb 17, 2024 • edited Loading

benmccann commented Mar 14, 2024

leoj3n commented Mar 14, 2024

benmccann commented Mar 14, 2024

leoj3n commented Mar 14, 2024 • edited Loading

benmccann commented Mar 15, 2024

leoj3n commented Feb 3, 2024 •

edited

Loading

changeset-bot bot commented Feb 3, 2024 •

edited

Loading

leoj3n commented Feb 17, 2024 •

edited

Loading

leoj3n commented Mar 14, 2024 •

edited

Loading