Add HTML support (with options!)#41
Conversation
…improvements Related to our LinkedIn post preview issue... The `og:image` can't be displayed because of missing `<head>` tags. Opened sensiolabs/minify-bundle#41.
…improvements Related to our LinkedIn post preview issue... The `og:image` can't be displayed because of missing `<head>` tags. Opened sensiolabs/minify-bundle#41.
…improvements (#149) Related to our LinkedIn post preview issue... The `og:image` can't be displayed because of missing `<head>` tags. Opened sensiolabs/minify-bundle#41.
|
omg this is open for so long oO |
| } | ||
|
|
||
| public function minify(string $input, string $type): string | ||
| public function minify(string $input, string $type/* , ?OptionsInterface $options = null */): string |
There was a problem hiding this comment.
Can we add a comment / deprecartion here, this feels so legit i'd agree to release a 2.0 there if we generalise this
smnandre
left a comment
There was a problem hiding this comment.
Minor comments here and there. Please add your author name in classes you introduce ;)
and... sorry for the (omg) time to see this.
I never got any notification here... but I should have seen it 🫢
|
(and obviously thank you for your PR ❤️ ) |
|
@Kocal (would not want you to receive notification during winter 😅 ) |
|
No worries, we've all been there :) I just pushed some modifications based on your comments |
|
Minor cs/sa fix and LGTML :) And then we'll be able to prepare 2.0 (i wanted to add other formats too, and maybe some proactive minification) Thx @Kocal |
|
Fixed. About PHPStan, I added the following errors in its baseline. It looks like PHPStan does not like the way to add a new method parameter on an interface without breaking the BC layer. Note: the CI run PHPStan only on |
We use this bundle on baksla.sh to programmatically minify HTML files (SSG), but today we ran into an issue where the site preview on LinkedIn wasn't working: our
og:imagetag was being ignored.It turns out that LinkedIn's parser (https://www.linkedin.com/post-inspector/) doesn't work correctly when the
<head>and</head>tags are missing... 😫This PR adds support for HTML and options (just one option for now, KISS), but only in the low-level
MinifyInterface::minify()method.I deliberately did not touch the bundle (i.e., adding configuration to
SensiolabsMinifyBundleand modifying theMinifierCompiler, etc..), because I don't think it makes sense with the AssetMapper's integration (JS/CSS only).