Skip to content

[AssetMapper] Section about minifying points to deprecated CloudFlare feature #19878

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

Closed
cavias opened this issue May 10, 2024 · 7 comments
Closed
Labels
AssetMapper hasPR A Pull Request has already been submitted for this issue.

Comments

@cavias
Copy link

cavias commented May 10, 2024

Page: https://symfony.com/doc/current/frontend/asset_mapper.html#optimizing-performance

Text:

Compress your assets: Your web server should compress (e.g. using gzip) your assets (JavaScript, CSS, images) before sending them to the browser. This is automatically enabled in Caddy and can be activated in Nginx and Apache. In Cloudflare, assets are compressed by default and you can also enable auto minify to further compress your assets (e.g. removing whitespace and comments from JavaScript and CSS files).

Issue:
The auto minify link points to https://developers.cloudflare.com/speed/optimization/content/auto-minify/ which clearly states the following:

Deprecation notice

Auto Minify is deprecated and will be removed on 2024-08-05. After this date, Auto Minify will no longer be available via the Cloudflare dashboard, API, or Terraform.

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

Which makes AssetMapper look like a 🤡 after boasting so much about how it doesn't need minifying.

@nicolas-grekas
Copy link
Member

Please keep a respectful tone; "look like a clown" isn't very nice if you don't mind.
That being said, I'd be fine removing minification from the assetmapper doc since compression should already provide most of the benefit.
Would you like to have a look and send a PR?

@cavias
Copy link
Author

cavias commented May 13, 2024

I am not trying to be disrespectful, but merely remarking upon the silliness of trying to make a (rather controversial, imo) point about not needing to minify (which is an industry standard), then linking to a webpage which says the complete opposite, on a website from a major web company which still recommends minifying.

Personally I'm not convinced that compression is all that is needed. We're working on a huge monolith of a project with megabytes of JS, and I find that minifying still saves us a couple hundred of kbs. That's with compression, as without compression the difference would be many hundreds of KBs, perhaps up to an MB.

And while we're aware that our JS needs to change and are working on it, this does block us from switching to AssetMapper until we've got that somehwat sorted in many months or even years, or maybe even never. And that's kinda frustrating as all docs point towards using AssetMapper and the documentation this issue is about, where it is stated that minifying is obsolete, then linking to docs which state the opposite.

@nicolas-grekas
Copy link
Member

We have a concept of asset compilers, so maybe we could make it easy to plug one? Did anyone write a bundle for this already? Up to have a look?

@cavias
Copy link
Author

cavias commented May 13, 2024

Well there is https://github.com/WebGardenGroup/minify-bundle which implements https://github.com/tdewolff/minify

But the former doesn't seem to have a stable release yet and no promise nor history of LTS, so it's not something we can probably use at this point (though I am keeping an eye on it).

The latter, however, seems like a great tool to implement in the current AssetMapper as an extra native option or officially supported bundle like the TypeScript bundle.

For now, however, we'll probably switch to webpack-encore or Vite (has our company's preference) instead.

EDIT: Note that this issue isn't to bash AssetMapper and I quite like the concept. It's sincerely to point out an outdated reference in the docs of which I have no idea what to replace it with - otherwise I'd have made a PR.

@javiereguiluz
Copy link
Member

@cavias you mentioned that your project has several megabytes of JavaScript. From my own experience (which by definition, is limited) that's rare in web applications. Maybe you are working on a videogame or some other kind of app that is very heavy in JavaScript usage?

I any case, e.g. when working with libraries like Bootstrap, the difference between "just compress" and "minify + compress" is almost inexistent (I tested it with brotli and zstd compression).

Also, if you are using e.g. Tailwind CSS, the TailwindBundle (https://symfony.com/bundles/TailwindBundle/current/index.html) provides the --minify option in the tailwind:build command.

@cavias
Copy link
Author

cavias commented May 13, 2024

@javiereguiluz Our total JS is 3,5mb. This is not all loaded into every pageload, but it does stack quickly. That's what over a decade of development has accumulated. I wish I could change this overnight, but I can't.

In my testing with disabling minifying I noticed a 50kbs difference just in the initial main JS file, and there are many more as some effort has already been put into splitting it up into module specific files. And Lighthouse dropped 15 points instantly.

So I am happy to hear that you do not face this issue, but that doesn't solve my issue yet.

@javiereguiluz javiereguiluz added the hasPR A Pull Request has already been submitted for this issue. label Jun 13, 2024
@javiereguiluz
Copy link
Member

Closing as fixed in #19905, where we removed the mention to this deprecated feature from Cloudflare.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AssetMapper hasPR A Pull Request has already been submitted for this issue.
Projects
None yet
Development

No branches or pull requests

4 participants