Critical error when using CDN module

I can't replicate any problem but I see a typo in the use statement that might be the cause, so fixing that.

@KarenS I think the constructor should typehint on the AssetOptimizerInterface, type hints for specific classes should be avoided if possible. If the CDN module doesn't extend this core class (and it shouldn't have to), this will still fail.

I'm having the same error on the latest Drupal 8.9.3, although with a different optimizer:

TypeError: Argument 5 passed to Drupal\\amp\\Asset\\AmpCssCollectionRenderer::__construct() must be an instance of Drupal\\Core\\Asset\\CssOptimizer, instance of Drupal\\vendor_stream_wrapper\\Asset\\VendorStreamWrapperAssetOptimizer given, called in /var/www/vhosts/shoppinginromania.ro/core/lib/Drupal/Component/DependencyInjection/Container.php on line 259

I need to have vendor_stream_wrapper enabled as it's required for ColorPicker.

The site is running on PHP 7.2.31

Looking at vendor_stream_wrapper it implements AssetOptimizerInterface.

Any ideas? Would it have anything to do with my environment?

Clarifying the problem, I think Dave is right, need to change CssOptimizer to CssOptimizerInterface in the constructor.

And there's no patch so it's not "Needs work".

Utterly naive patch attached, swapping out references to Drupal\Core\Asset\CssOptimizer in favour of Drupal\Core\Asset\AssetOptimizerInterface.

Patching against commit 09bb5f4 on 8.x-3.x

Only "tested" insofar as my site no longer displays the error, and the AMP version of my pages validate okay.

Definitely needs further attention, but this appears to have got me over the line for today.

Update:
Ah. Spoke far too soon, further errors still being reported (but at least there's no white screen of death...):

Warning: preg_replace_callback(): Requires argument 2, 'Drupal\cdn\Asset\CssOptimizer::rewriteFileURI', to be a valid callback in Drupal\amp\Asset\AmpCssCollectionRenderer->render() (line 182 of modules/contrib/amp/src/Asset/AmpCssCollectionRenderer.php).

Another naive, minimally-tested workaround, this time for the warning in #9

Wraps the preg_replace_callback() call in a conditional that just checks for a rewriteFileURI() method on $this->optimizer before running.

Essentially untested ― I don't have any url()s within my AMP theme ― but I assume the cdn module takes care of URL replacement as part of its implementation of Drupal\Core\Asset\AssetOptimizerInterface::optimize(), and any CssOptimizer implementation that does need to run the replacement regex will provide a suitable rewriteFileURI() method, and therefore run the conditional replacement code.

I think this is the only way to go. It looks like the CDN module does not use that method. The note on that method in Drupal\Core\Asset\CssOptimizer looks like it was deliberately left out of the interface. I don't have any way to test what happens in the CDN module to see if urls in css work without this or not.

I guess the other solution would be adding something like "if not method exists" then create the functionality in this module instead. Or maybe do that anyway, copy that method to this class but rename it, then use the renamed method so it's always available and there's no conflict either way.

OK, the only method we're using is the one that isn't in the interface. So rather than this fix we should remove the CSSOptimizer altogether. Then copy the method we want into our class (code duplication but there's no way around it). Then use the new method. No other module decorating the optimizer will be impacted.

Add a little more documentation about the issue for future reference.

Realized we only need to use this if css files have not been aggregated, so I reworked the logic. If files are aggregated the code won't change anything so it didn't hurt anything but is not necessary. I also added a test that the css gets altered correctly.

Committed, thanks!