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

Optimize file size #3

Closed
jstcki opened this issue Jan 22, 2016 · 22 comments
Closed

Optimize file size #3

jstcki opened this issue Jan 22, 2016 · 22 comments

Comments

@jstcki
Copy link
Contributor

jstcki commented Jan 22, 2016

Currently this loader can't be combined with other loaders which optimize image file size like image-webpack-loader. Add that option (how though?) or directly use imagemin.

@Klathmon
Copy link
Contributor

I actually had this issue myself, so I ended up making a webpack plugin that minifies all images called (unimaginatively) imagemin-webpack-plugin.

It acts like image-webpack-loader, but being a plugin it is able to optimize all images in the webpack plugin (images from this plugin included).

@jstcki
Copy link
Contributor Author

jstcki commented Jun 20, 2016

@Klathmon that looks very cool! Haven't tried it yet but a plugin sounds like a better choice than a loader for this kind of optimization.

@Klathmon
Copy link
Contributor

Well if you ever do try it out, please let me know if there is anything you run into that you think can be improved with it.

@timse
Copy link

timse commented Oct 7, 2016

i was looking at the resize-image-loader myself and kind of unsatisfied with the state of it, however didn't find this project in time, so i came up with my own loader srcset-loader

There you can find an approach how you can keep your loader chainable with file-loader or image-webpack-loader (also letting them do all the work).
Might be interesting for your loader here.

@gabrielbull
Copy link

Haven't tried it yet but a plugin sounds like a better choice than a loader for this kind of optimization.

I disagree, there is a lot that can be done with a loader and there are a lot of loaders out there. For instance, it'd be great to be able to base64 encode small images using url-loader, or to control the files using file-loader. If we can use the standard loader chaining it would make a lot more sense to everyone and make this tool much more powerful. I havent had the time to look at it, but maybe @timse got the answer.

@Klathmon
Copy link
Contributor

@gabrielbull I think he meant that imagemin is better as a plugin vs a loader.

Optimizers are almost always better as plugins because the plugin itself can determine when it's run, vs the user determining it with a loader. Plus optimizers work great as plugins with loaders like this that add assets to the project as they run.

@gabrielbull
Copy link

For sure, optimizers are better as plugins, but that shouldn't be in the scope of this project. I think this project should focus on making sure you can do responsive images while not losing the loader chaining ability of webpack.

@Klathmon
Copy link
Contributor

Klathmon commented Oct 29, 2016

And I agree, that's why I made the imagemin plugin instead of trying to shoehorn optimization into this loader 😀

But regardless, this is a tough loader to work with web pack, because web pack doesn't have any concept of an "asset" for loaders. It's more of a what you return is what you get situation.

That works great when it's one file in, and one file out, but when you start doing things like one file in and many files out, the chaining ability gets lost.

This isn't really something that this or any other loader can easily solve, and it would need to be solved by web pack itself.

That being said, this one is doing a pretty damn good job with what it's working with!

@gabrielbull
Copy link

gabrielbull commented Oct 29, 2016

Yes, that's right. @timse, how did you make it work with multiple assets?

There is one case where this library does pass one file down, when using the size parameter, like in this example:

.myImage { background: url('responsive?size=1140!myImage.jpg'); }

@gabrielbull
Copy link

gabrielbull commented Oct 29, 2016

Maybe it is just me, but if webpack and loader chaining can only handle one file at a time, then maybe so should the solution for responsive images.

I don't mind doing this in my code if it means that chaining can be kept.

const image = {
  "100w": require('resize?100!myImage.jpg'),
  "200w": require('resize?200!myImage.jpg'),
  "placeholder": require('url!resize?50!myImage.jpg'),
};

<div style={{ backgroundImage: `url(${placeholder})`, filter: 'blur(20px)'}}>
  <img
    srcset={`${image.100w} 100w, ${image.100w} 200w`}
  />
</div>

I thinks this solution cover more ground too:

Picture element

<picture>
  <source srcset={image.100w} media="(max-width: 600px)"/>
  <source srcset={image.200w} media="(max-width: 1200px)"/>
  <img src={image.200w}/>
</picture>

CSS (I know it is currently possible but this makes it one API for both, so more intuitive)

background-image: url("resize?100!myImage.jpg");
@media screen and (max-width: 1200px) {
  background-image: url("resize?200!myImage.jpg");
}

@jstcki
Copy link
Contributor Author

jstcki commented Oct 29, 2016

The main benefit of this loader is to generate multiple images and a valid srcset with one require call. For more advanced cases like <picture> you can use the images property of this loader's output directly.

If someone can make it work with loader chaining, and usage doesn't get more complicated, I'd be happy to accept a PR for that. But apart from the optimization step, I'm not aware of any useful loaders to combine it with? When using a srcset, you'd want it to behave like file-loader anyway because inlining multiple resized images with url-loader kinda defeats the purpose of a srcset, doesn't it?

@gabrielbull
Copy link

Well, for base64 images for instance, when you want a placeholder, like in my example above, url-loader is going to base64 encode smaller image or provide a file for larger images. My point was that it shouldn't be the scope of this project to decide what loaders people will want to use and they should be able to use them as they are already with everything else in web pack. You can imagine loaders for effects, resize (this one), base64 encode (url), etc, etc.

Think of it this way:

const myBlackAndWhiteImage = require('url!filter?grayscale!responsive?500!myImage.jpg');

<img
  src={myBlackAndWhiteImage}
/>

I think the idea of having srcset the main focus of the library is what is limiting its potential. If it's just another chain in the normal web pack loaders it can be so much more. But, to do that, it would need to abandon the idea of srcset and leave that part to the user like in my example above.

@gabrielbull
Copy link

Just let me know if this is not something you would consider.

@jstcki
Copy link
Contributor Author

jstcki commented Oct 29, 2016

We're adding support for base64 encoded placeholders in #16.

Creating srcsets is the whole point I created this loader, so I'm reluctant to drop support for that 😆

That said, as srcset-loader proves, it's not impossible to combine a 1-to-many-files loader with other loaders. It's just not straightforward to implement, and I don't know if webpack 2 changes anything on that front. If you wanna give it a shot in a PR, go for it!

@timse
Copy link

timse commented Oct 29, 2016

Heya. Im on my mobile so i try to give the best answer possible :P

Basically i solve the chainability with one to many request as described here: https://github.com/timse/srcset-loader/blob/master/README.md#why-is-the-srcset-loader-before-the-other-loaders

If desired i can go into detail how to do this with webpack when im on a computer again the next days :)

And as mentioned here i want to keep chainability because i trust that other people are much more skilled at solving all those tasks like optimization much better than me. My biggest reason to keep chainability as i basically only want the loader to split up requests and resize images but not try to reimplement all the other things.

(I did add placeholder images into the loader if wanted, which i already kind of mislike as i dont think it's actually part of the srcset loader and i might take it out of there again:) )

@jstcki
Copy link
Contributor Author

jstcki commented Oct 30, 2016

Thanks @timse. I may not have time to do this in the next few weeks, so if anyone wants to do this, I'd be happy to accept contributions. I wonder if you'd be open to join forces @timse, since responsive-loader and srcset-loader seem to do basically the same.

Things to consider, as this would be a non-trivial addition in my opinion:

  • make sure it works with webpack 1 and 2
  • write tests (for webpack 1 and 2)
  • make it easy to use – I don't want users to have to chain with file-loader
  • write good documentation for it

@timse
Copy link

timse commented Oct 31, 2016

heya sure I wouldn't mind to join forces, but as you my time currently is limited and I would have to assess if and how those loaders can work together without creating too many breaking changes.
E.g. how to handle I don't want users to have to chain with file-loader

@langri-sha
Copy link

sokra lays out a nice API here, to use the loaders=n query param to hint the loader to rewrite the request and inject a child loader at position n.

@strarsis
Copy link

strarsis commented Apr 6, 2019

So when I want to resize a JPEG using responsive-loader and then optimize the JPEG (quality 80%) using mozjpeg, how can I combine these two? Current default is just running mozjpeg over all the images.

@Klathmon
Copy link
Contributor

Klathmon commented Apr 7, 2019

@strarsis You can use my imagemin-webpack-plugin's test option to make a regex or a function which will make it ONLY run on specific files.

@phartikainen
Copy link

Screenshot 2019-04-18 at 9 07 14

I ended up ditching responsive-loader because of this @strarsis – here's the setup using srcset-loader and image-webpack-loader.

@dazuaz
Copy link
Owner

dazuaz commented Jun 9, 2022

File size is solved by defining the desired compression quality

@dazuaz dazuaz closed this as completed Jun 9, 2022
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

8 participants