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

OptimizerChainFactory refactoring broke "quality" config #218

Closed
glye opened this issue May 15, 2024 · 2 comments · Fixed by #219
Closed

OptimizerChainFactory refactoring broke "quality" config #218

glye opened this issue May 15, 2024 · 2 comments · Fixed by #219

Comments

@glye
Copy link

glye commented May 15, 2024

CC @0xb4lint

Hi, sorry if I misunderstand things. I'm looking at the "OptimizerChainFactory refactor with config processor" change from 1.7.3:
e74cdf1

The problem occurs when the config in OptimizerChainFactory::create($config) contains a 'quality' entry, which it can, according to the code in getOptimizers() below. Previously, such a "generic" quality would be applied to all optimizers that support it.

The call to the new self::configHasOptimizer($config) results in config being returned verbatim, including the 'quality' entry:
e74cdf1#diff-4f7d0501c10934549ecacfae146962d03a7dcacf1ffab4cadef7984a68409cdfR32

Then when this is instantiated, we get an attempt to call new quality(...) which fails with a ClassNotFoundErrror.
e74cdf1#diff-4f7d0501c10934549ecacfae146962d03a7dcacf1ffab4cadef7984a68409cdfR21

In short: Since 1.7.3 it is no longer possible to specify $config['quality'], doing so results in a crash.
Thanks for reading.

@0xb4lint
Copy link
Contributor

Hi @glye!

You are very right, sorry for the breaking change.
I was able to reproduce the issue with a failing test: https://github.com/0xb4lint/image-optimizer/actions/runs/9098545560/job/25008988995

Working on a fix. 😉

@glye
Copy link
Author

glye commented May 16, 2024

Thank you!

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 a pull request may close this issue.

2 participants