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

[AssetMapper] Support minifying JS/CSS/SVG/etc #54907

Open
cavias opened this issue May 13, 2024 · 6 comments
Open

[AssetMapper] Support minifying JS/CSS/SVG/etc #54907

cavias opened this issue May 13, 2024 · 6 comments

Comments

@cavias
Copy link

cavias commented May 13, 2024

Description

Hello there!

I quite like the idea of AssetMapper, but...

I am working on an old PHP monolith with multiple mbs of JS and CSS. We are in the process of porting this app to Symfony and optimising it. As part of this I did some testing with AssetMapper, and though it works great, I've ran into an issue with not being able to minify the JS/CSS.

The problem I am facing specifically is that for us there is a very significant file and performance difference between the minified and non-minified versions of the JS we're serving. That is with gzip compression.

Numbers
On average, depending on the file, the difference is about 25% in terms of file size. And Google Lighthouse does start to yell at me to minify the JS.

Our main JS file has a 50kb difference WITH compression between minifying and not minifying - without compression that would be about 280kb. And that main JS file only represents 1/6 of the total JS in the app spread through various pages.

Also Lighthouse dropped us 15 points instantly.

So I was wondering, can we get an implementation for minification of JS and CSS?

For CSS I can't do any real in-app testing atm, because we use a custom compiler for that to inject values into placeholders - we want to replace that with something else like AssetMapper or webpack or Vite in the future as well.


DISCLAIMER
Yes, I know our JS needs optimising, but I can't really do this in a few days. It's a work in progress, but that'll take time. Right now, for my usecase, and I imagine for other people's as well, being able to minify would solve our issue while allowing us to sort out our JS with the help of AssetMapper.


TL;DR: Can we get minification of JS/CSS and other things supported in AssetMapper?

Example

Something like https://github.com/WebGardenGroup/minify-bundle which runs on https://github.com/tdewolff/minify

But with more official and guaranteerd support.

Update 27-05-24

We (as a company) currently are implementing Vite using https://symfony-vite.pentatrion.com/

Although I am not a fan of using more 3rd party bundles because of various risks, it does work really well.

Part of the reason we choose Vite over Webpack is that it is becoming the company-wide standard (so we have in-house experts). This will also simplify sharing (Vue/SCSS) assets between our projects, which is a desire by many here. On top of that, many people here are of the opinion that Vite = harder/better/faster/stronger > Webpack.

I do hope the discussion about implementing some kind of minifying+uglifying in AssetMapper will continue, as I am convinced it will be a very important requirement for many others to switch over.

@javiereguiluz
Copy link
Member

@cavias as you pointed in this Symfony Docs issue: symfony/symfony-docs#19878 Cloudflare has decided to deprecate the minify feature. They explain this:

The reasons why we plan to remove the feature are manifold but boil down to the fact that it’s a performance feature used for the purpose of reducing page weight and it is not as efficient at doing this as other projects Cloudflare’s working on, like improving compression.

In the related email that Cloudflare sent to customers they even give this number:

the benefit (of minify) for page size reduction is less than 0.1% across all minified files on Cloudflare's network

And also:

As we continue to invest in compression, we have observed a consistent decrease in users of Auto Minify


In your case, it's clear that minifying assets produces better results. However, generally speaking, minifying asset contents is no longer required for most apps, as long as they use an excellent compression feature such as Brotli or zstd.

@RobertMe
Copy link
Contributor

With regards to Cloudflares arguments. Couldn't it also be the case that with a lot of modern websites there is an assets build pipeline which does (already) minify the assets? So in those cases it doesn't make sense to enable auto minify on Cloudflares end (as there should be nothing to minify anymore, as it's already minified). And in those cases where the auto minify would still be enabled while the "original" assets are already minified it would obviously also result in not having any effects (or less effect).

And this is also (clearly) stated in their deprecation notice:

We recommend that you minify at the origin during the build phase. Minification is included in most modern web development frameworks.

(emphasis mine)

So IMO this is a catch 22. Symfony docs refer to (for example) using Cloudflares auto minify feature. While Cloudflare refers to using it at the origin as part of a build process.

@Nek-
Copy link
Contributor

Nek- commented May 21, 2024

2 things are to consider here:

  1. Minification helps with performance and must be done.
  2. Many people are concerned about avoiding giving internal details to anybody who opens their JS files and having the ability to read the comments. When you write twig templates, you use {# #} comments. Why pretend JS would be any different when it's more critical? (theme: privacy & security)

I wanted to claim the second point because I'm so surprised nobody raised it besides it seems so critical to me (even though I agree security by obfuscation is not great).

Besides that, I'm also surprised that it has been fine at some point to say "to do so, use cloudflare" which is a private company not related to Symfony in any way 🧐 , and it's not even free! But nvm, they do not offer the feature anymore so there's no point understanding why this recommendation anymore...

@stof
Copy link
Member

stof commented May 22, 2024

AssetMapper has the necessary extension points to allow hooking a minification step. The main issue for that is that there is no reliable implementation of a JS minifier in PHP (which is a huge project in itself, and needs to be maintained regularly to cope with updates of the JS syntax every year).
There are a bunch of JS/CSS minifiers available on Packagist but as far as I can tell, all of them use regex-based approaches, which will actively break some valid JS code.

@javiereguiluz
Copy link
Member

@stof maybe we could do the same as with TailwindCSSBundle ... download a binary that does the job and that's it.

There's a Golang project that includes many minifiers ... and provides a standalone CLI binary for multiple platforms. See https://github.com/tdewolff/minify

@smnandre
Copy link
Contributor

For me the challenging part is there :

But with more official and guaranteerd support.

There are bundle out there that does help for who wants to minify its assets.

But to ensure some garantee here, in the front-end world, that's a massive leap of faith.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants