rolling-scopes / site

đŸĻĨđŸ›ŧ Website supporting The Rolling Scopes and The Rolling Scopes School educational process
https://rollingscopes.com
8 stars 7 forks source link

Investigate: Correct srcset and sizes when no optimized images #421

Open thirdmadman opened 1 month ago

thirdmadman commented 1 month ago

🐛 Describe the Bug

Currently, on screen sizes less than 768px in the console, we can see an error 404, a problem loading some images. Also, there is a problem with images optimization. Some images won't load their smaller, optimized versions for smaller screens. This affects the loading time and also for LightHouse score overall because it's it's trigges on errors in the console.

🔄 Steps to Reproduce

Provide the steps to reproduce the behavior:

  1. Navigate to any page
  2. Click on Delepers Tools and find any image with a source size less than 425 or 768
  3. Check srcset propery and sizes
  4. Observe the error when in srcset and sizes added, there are not existing images, or the whole srcset does not exist.

🏁 Actual Behavior

Currently, optimizeImages/optimizeImages.js compresses images to webp and resizes images to 425 or 768 px, these images have been used in srcset. If the image width is less than 768px it's resized version won't be created. The same is true for images with a width less than 425px. Than in component Image /src/shared/ui/image/image.tsx, src prop - file name, converted in filename with extension webp and used as prop src for resulting <img>. But also, this component is responsible for generating srcset and sizes for the same resulting <img>. Srcset and sizes are generated based on env variables and nothing else. Corresponding functions generateSrcSet and generateSizes knows nothing about real images after build or even after optimizeImages.js,

They are just surely used:

const srcSet = `${srcNoExtension}-${MOBILE_W}.webp ${MOBILE_W}w, ${srcNoExtension}-${TABLET_W}.webp ${TABLET_W}w, ${src} ${DESKTOP_W}w`;

and

const generateSizes = () => {
  return `(max-width: ${MOBILE_W}px) ${MOBILE_W}px, (max-width: ${TABLET_W}px) ${TABLET_W}px, ${DESKTOP_W}px`;
};

Thats why some images getting srcset and sizes, even if they are less than 425px and in reality no needs this properties.

🏁 Expected Behavior

Images from srcset has to exist and only be used if needed to avoid errors, unneeded loads, and unneeded assets in the build. If no resized images exist, srsset must not be used.

📸 Screenshots

image

🕹 Device Information

ℹī¸ Additional Context

This bug is part of trilogy of images bugs, see also:

411

420

421

I need to mention that this issue has been known before, and it's also been tried to solve this bug in #333. Important information is also mentioned in the discussion at https://github.com/rolling-scopes/site/pull/333/files#r1650181876

Now this bug report has been transformed into an investigation

This bug report has been transformed into an investigation because we need additional information to make a decision on how to solve this problem. 23.07.2024

Deadline

This investigation has to end in 10 working days.

thirdmadman commented 1 month ago

This problem has different solutions, but all of them are not ideal:

1. Dirty hack.

We can forcly generate "resized" images, but not resize them, just change the name to the name wtith needed suffix.

This method adds useless images to build and loads useless images by client, but no errors will be shown.

2. A semi-dirty hack.

We can, in run-time, check if any errors occurred, and adapt srcsect.

This will redraw the UI several times and still produce console errors because the files do not exist.

// /src/shared/ui/image/image.tsx
  const handleError: ReactEventHandler<HTMLImageElement> = (e) => {
    // fallback to basic src if there are no responsive sizes for an image
    const currentSrc = e.currentTarget.currentSrc;
    const srcToExcludeStartIndex = currentSrc.indexOf(srcWebp.replace('.webp', ''));
    const srcToExclude = currentSrc.substring(srcToExcludeStartIndex);

    if (srcSet) {
      const index = srcSet.findIndex((el) => el.includes(srcToExclude));

      if (index < 0) {
        return;
      }
      const newData = [...srcSet];

      newData.splice(index, 1);

      if (newData.length === 0) {
        setSrcSet(undefined);
        return;
      }

      setSrcSet(newData);
    }

    // setSizes(undefined);
  };

    return (
    <img
      // ⚠ī¸ Firefox and Safari wants the loading attribute to be BEFORE the src, in order lazy loading to work
      // see https://github.com/facebook/react/issues/25883#issuecomment-1410060269
      loading={loading}
      srcSet={Array.isArray(srcSet) ? srcSet.join(', ') : srcSet}
      sizes={sizes}
      decoding={decoding}
      // FIXME: remove this line when fetchPriority prop will be fixed
      // see https://github.com/facebook/react/issues/27233#issuecomment-2035176576
      // @ts-expect-error
      // eslint-disable-next-line react/no-unknown-property
      fetchpriority={fetchPriority}
      src={srcAttr}
      alt={alt}
      draggable="false"
      onError={handleError}
      {...props}
    />
  );
// /src/shared/ui/image/utils/generateSrcSet.ts
const generateSrcSet = (src: string) => {
  const srcNoExtension = src.slice(0, src.lastIndexOf('.'));
  const srcSet = [`${srcNoExtension}-${MOBILE_W}.webp ${MOBILE_W}w`, `${srcNoExtension}-${TABLET_W}.webp ${TABLET_W}w`, `${src} ${DESKTOP_W}w`];

  return srcSet;
};

3. Vite plugins?

Yeah, plugins. Vite have many plugins and also some plugins from Rollup can be used.

Examples: https://www.npmjs.com/package/@kingkongdevs/vite-plugin-image-sizes https://www.npmjs.com/package/vite-imagetools/v/3.0.1 https://github.com/arma-events/vite-plugin-srcset https://www.npmjs.com/package/vite-plugin-image-optimizer https://github.com/vHeemstra/vite-plugin-imagemin https://github.com/jw-12138/vite-plugin-vsharp

It's hard to say how to optimize images and generate resized images with srcset, It should be possible, but it can take some time to understand which plugins to use and how.

It's also required to change the whole codebase to add corresponding props to images and query parameters to image imports.

This solution takes time and may not work fully, but in my opinion, it's the correct one's.

4. My little bicycle vite plugin?!

Yeah, it's totally possible that none of the existing plugins can help us, but that's not the end. We can create our custom plugin from existing files or from scratch.

For my opinion and experience, it's not possible to inject srcset and sizes into build, resize and optimize images without little "hack" like it's made in other plugins:

import { src, width, height, channels } from 'example.jpg?w=200'

So, we cannot avoid, in this solution, the modification of image imports and corresponding props passing through the whole codebase. We can use the existing optimizeImages/optimizeImages.js and its utils and customize processes as we want.

This solution will take the most time of any in the list, but it will undoubtedly yield the best result.

thirdmadman commented 1 month ago

Now this bug report has been transformed into an investigation

This bug report has been transformed into an investigation because we need additional information to make a decision on how to solve this problem. 23.07.2024

Deadline

This investigation has to end in 10 working days.

Results of investigation

As a result, there have to be detailed cons and pros to the main solutions and ways to implement them:

Already existing plugins

Find plugins for the vite or rollup and make them work together to fully replace the current optimizeImages/optimizeImages.js.

This includes compression of images in svg and the use of srcset and the possibility to use this srcset in code somehow (import, injection after build, or else), so the resulting build contains the correct srcset and sizes.

Find out what we have to change in configs, packages, and code.

Custom plugin for vite

Find out how to create a custom vite plugin using the API of Rollup to fully replace the current optimizeImages/optimizeImages.js.

Use existing plugins soure code to understand what to do. Use other already-created solutions if possible.

If there is any other solution, please suggest it in the comments.