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

Provide sensible defaults for performance #58

Closed
wants to merge 1 commit into from
Closed

Conversation

Toflar
Copy link
Member

@Toflar Toflar commented Feb 28, 2018

No description provided.

@Toflar Toflar added the feature label Feb 28, 2018
@Toflar Toflar self-assigned this Feb 28, 2018
@Toflar Toflar added this to the 4.6.0 milestone Feb 28, 2018
@Toflar
Copy link
Member Author

Toflar commented Feb 28, 2018

Ref contao/core-bundle#1404

@aschempp
Copy link
Member

aschempp commented Mar 1, 2018

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
Copy link
Member Author

Toflar commented Mar 1, 2018 via email

@leofeyer
Copy link
Member

leofeyer commented Mar 1, 2018

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.

@leofeyer leofeyer removed this from the 4.6.0 milestone Mar 1, 2018
@Toflar
Copy link
Member Author

Toflar commented Mar 1, 2018

  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
Copy link
Member

leofeyer commented Mar 1, 2018

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
Copy link
Member Author

Toflar commented Mar 1, 2018

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
Copy link
Member

aschempp commented Mar 5, 2018

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
Copy link
Member Author

Toflar commented Mar 5, 2018

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
Copy link
Member

leofeyer commented Mar 15, 2018

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
Copy link

frontendschlampe commented Mar 15, 2018

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

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

@leofeyer
Copy link
Member

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
Copy link

ghost commented Jun 19, 2018

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

@leofeyer leofeyer closed this Jun 19, 2018
@leofeyer leofeyer deleted the htaccess-perf branch June 19, 2018 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants