tcoopman / image-webpack-loader

Image loader module for webpack
MIT License
2.03k stars 131 forks source link

How to handle the webp option breaking images on Safari? #183

Open bitttttten opened 5 years ago

bitttttten commented 5 years ago

Hi,

I am using the webp loader but it breaks all my images in Safari. I expect that because Safari's support of webp is not quite there yet. However, what approach do you recommend?

Gatsby's image plugin fallsback to the original image if it's not compatible. It generates something like:

<picture>
        <source srcset="/d0d94/nyos.webp 340w, /0f49e/nyos.webp 680w, /fdcb7/nyos.webp 1000w" sizes="(max-width: 1000px) 100vw, 1000px" type="image/webp">
        <source srcset="/679f9/nyos.jpg 340w, /30027/nyos.jpg 680w, /97529/nyos.jpg 1000w" sizes="(max-width: 1000px) 100vw, 1000px" type="image/jpeg">
        <img class="gatsby-resp-image-image" style="width: 100%; height: 100%; margin: 0px; vertical-align: middle; position: absolute; top: 0px; left: 0px; box-shadow: white 0px 0px 0px 400px inset; opacity: 1; transition: opacity 0.5s ease 0s;" src="/97529/nyos.jpg" alt="nyos" title="">
</picture>

Which I am more than happy to do on my side, although I can't figure out a workflow here. Would it mean including 2 loaders of image-webpack-loader, one configured for webp and one without?

anikethsaha commented 4 years ago

This loader cant handle fallbacks as it doesn't know about the browser during the build time.

What I can suggest is to use this loader to compress your images. And then use the webpack copy plugin to copy the original image in which you want to do a fallback. Then you need to write the logic in your frontend to fall back to the original image.

bitttttten commented 4 years ago

Well instead of a string, the loader could return an object like:

{
  src: `/97529/nyos.jpg`,
  webp: {
    src: `/d0d94/nyos.webp 340w, /0f49e/nyos.webp 680w, /fdcb7/nyos.webp 1000w`,
    src: `/fdcb7/nyos.webp`,
    sizes: `(max-width: 1000px) 100vw, 1000px`,
    type: `image/webp`
  },
  jpg: {
    src: `/679f9/nyos.jpeg 340w, /30027/nyos.jpeg 680w, /97529/nyos.jpeg 1000w`,
    src: `/97529/nyos.jpeg`,
    sizes: `(max-width: 1000px) 100vw, 1000px`,
    type: `image/jpeg`
  }
}

So then this one loader handles all the cases. What do you think?

anikethsaha commented 4 years ago

A webpack loader just transforms the content of the file and then these files are handled by the webpack in its later stages. So even if you are returning it like a object (which we can't cause loaders need to return string) instead of string, the object will be lost and it will completely change the image content cause the content of the image file will be these object. You may need further processing of the image file to make it to image file

And this loader is often chained with other loaders like file, url and those loader just read the content of the require and injects the whole as a string. So you won't get a image there.

I am not telling that it can't be possible, but it break stuff and it is not up to the use case of this loader. You may need a plugin inside the loader to eject and extract out the image file.

You can try it in your fork. I would be happy to help there.

anikethsaha commented 4 years ago

@tcoopman I think returning extra data like mentioned here https://github.com/tcoopman/image-webpack-loader/issues/183#issuecomment-566448559 can be done using

metaData = {
  src: `/97529/nyos.jpg`,
  webp: {
    src: `/d0d94/nyos.webp 340w, /0f49e/nyos.webp 680w, /fdcb7/nyos.webp 1000w`,
    src: `/fdcb7/nyos.webp`,
    sizes: `(max-width: 1000px) 100vw, 1000px`,
    type: `image/webp`
  },
  jpg: {
    src: `/679f9/nyos.jpeg 340w, /30027/nyos.jpeg 680w, /97529/nyos.jpeg 1000w`,
    src: `/97529/nyos.jpeg`,
    sizes: `(max-width: 1000px) 100vw, 1000px`,
    type: `image/jpeg`
  }
};

imagemin
  .buffer(content, {
    plugins
  })
  .then(data => {
    callback(null, data, metaData);
  })
  .catch(err => {
    callback(err);
  });

But then need a chained loader to next to this in order to process these metaData

tcoopman commented 4 years ago

To me this still sounds like something that should and can be done in a separate loader. I would need to see what complexity this adds to this loader to be sure about it, but I'd like to keep this loader focused on one thing and one thing only.

anikethsaha commented 4 years ago

yes cause even though we add these data, it still needs to be processed with the next loader chained with.

anikethsaha commented 4 years ago

Unless we have a loader to process this data, it doesnt make sense to add.

bitttttten commented 4 years ago

Regarding adding complexity.. that's a fair point too, since this loader just imports images.

Plus my example is very web focused. For targets like native, they do not use the srcset or sizes properties in my comment. They do use techniques just like this to load the right image for the right screen size though, so there are use cases that span across multiple technologies.

But what about making it generic, and saying, this loader can import one image.. and give you that image back in different formats at different resolutions? Is that something that would fit in this library? Then the implementation of what to do with all the images and sizes is left up to the consumer.

tcoopman commented 4 years ago

I would need to see it in a PR first to see how that looks etc... and at this point I'm not inclined to add it, so it might be an effort that never gets merged. But of course you're free to create a PR to play with this idea.

anikethsaha commented 4 years ago

the loader needs to be developed first in order to add this change in this loader.