Skip to content

Conversation

@maier49
Copy link
Contributor

@maier49 maier49 commented Jun 1, 2020

Type: feature

The following has been addressed in the PR:

  • There is a related issue
  • All code has been formatted with prettier
  • Unit or Functional tests are included in the PR
  • schema.json has been updated appopriately

Description:
Proposed solution for adding the image-webpack-loader and related config to the build.

  • Only optimizes images if the imageOptimization flag is present and for dist builds. This can be an expensive process so opting in seems reasonable.
  • The image-webpack-loader defers to multiple different tools for optimizing different types of images, and they take a wide variety of arguments. Rather than try to abstract over all those different APIs this just accepts an object for each tool and passes those options directly.
    Resolves Consider supporting image optimisation out of the box #328

@matt-gadd
Copy link
Contributor

@maier49 if we are going to allow a direct pass through can you please document the options and improve the schema, we shouldn't have to forward users to a webpack loader page for this. I'm going to presume this at least does optimization with some defaults being config free? Seems a bit weird to turn it on you have to specify an empty object rather than a boolean from what I can see in the schema?

@maier49
Copy link
Contributor Author

maier49 commented Jun 2, 2020

@matt-gadd I've updated the schema to allow you to simply pass true to enable image optimization with default settings, and yes we are doing optimization with defaults in that case.

I also added a section to the documentation for this. I added the enabled flag that can turn off (or on for webp) each specific optimization library to the schema, but if you take a look at the options APIs for each of those libraries adding all of that to our schema seems excessive to me. The loader itself takes the same approach as I have now in the docs and links out to each of the libraries individually. If you still want me to though I can add in the options for each.

Copy link
Contributor

@matt-gadd matt-gadd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small note: I think we don't currently use the README.md for anything on dojo.io so we might want to consider updating whatever the build bit is on there too.

@maier49 maier49 merged commit 8ce0bc8 into dojo:master Jun 8, 2020
@maier49 maier49 deleted the image-optimization branch June 8, 2020 16:14
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.

Consider supporting image optimisation out of the box

2 participants