search

contao / manager-bundle

[READ-ONLY] Contao Manager Bundle
GNU Lesser General Public License v3.0
17 stars 10 forks source link

Provide sensible defaults for performance #58

Closed Toflar closed 6 years ago

Toflar commented 6 years ago

Ref https://github.com/contao/core-bundle/pull/1404

aschempp commented 6 years ago

LGTM! Strange that javascript should be cached for a year but fonts only for a month, but I guess we should just rely on html5boilerplate here.

Toflar commented 6 years ago

Yeah, my thoughts and arguments were exactly the same :)

leofeyer commented 6 years ago

On second though, I am not a big fan of this change.

  1. A properly configured Apache already contains these settings.
  2. The optimization only applies to Apache. What about Nginx users?
  3. Large .htaccess files slow down the performance.
Toflar commented 6 years ago
  1. I have never encoutered any setup that contains all of these changes on shared hosting. On dozens of hosting providers and thousands of setups. Never.
  2. We provide a sensible default if Apache allows overriding. If you disallow overriding you have the same situation as nginx: do it yourself and control it yourself.
  3. It's the same here. If you care about performance, you should not allow overriding (or not use Apache at all) in the first place. So in that case this file is not relevant anyway. It's only relevant if you allow overriding and don't configure anything at all in which case we currently only make sure the rewrites are correct but nothing else.
leofeyer commented 6 years ago

in which case we currently only make sure the rewrites are correct

Which is all that the .htaccess file should do by default IMHO, because this is the only thing that cannot be preconfigured in Apache.

Your changes are not wrong, however, we should leave it up to the users to decide whether they want to optimize their server through an .htaccess file. And if they do, they might want to include all of HTML5 boilerplate instead of just a few things.

I guess that is why Symfony does not ship any .htaccess optimizations, either.

Toflar commented 6 years ago

I disagree. This is an absolute must-have to me. It doesn't harm at all and it provides a sane default for a lot of smaller sites and it serves as a reference to copy stuff from if you care.

aschempp commented 6 years ago

I don't know what the mime type definition is necessary for (if we wanna keep the file small), but I'd love to have the expire headers set.

Toflar commented 6 years ago

Just to ensure the headers are set correctly e.g. on sitemap.xml files. This is especially important if you set X-Content-Type-Options: nosniff (what you should) and your server did not set the mime type config accordingly. I think we should keep it, it can reduce issues with compatibility (and again, serve as a reference to copy from).

leofeyer commented 6 years ago

As discussed in Mumble on March 15th, we should not add this to the default .htaccess file, because if the server admin forbids to overwrite certain things, it will lead to an internal server error. This is one of the main reasons why we are shipping an .htaccess.default file in Contao 3.5 instead of an .htaccess file.

We might ship an .htaccess.default file instead, although people could use HTML5 boilerplate in this case as well. Also you cannot know that all JavaScript files are cacheable for one year.

frontendschlampe commented 6 years ago

use https://github.com/hofff/contao-htaccess instead ;-)

... but we have to update for Contao 4!

leofeyer commented 6 years ago

It was quite a heated discussion and eventually we agreed on providing an example configuration in the docs and adding a link to it in the .htaccess file.

ghost commented 6 years ago

A new issue thereto has been created at contao/docs#493.