Improving the file name generation logic for JS and CSS files

[boesbo]

Description (*)

The functions that merge CSS and JS files generate a file name based on criteria that are not optimal for caching. In fact, if you always merge the same files, the resulting file name is always the same, so this can cause problems for the visitor who already has that file in his cache, since he has a version with the same name but which is obsolete.

Expected behaviour (*)

Add the timestamp of the files in the MD5 HASH generation criteria of the files.

Benefits

CSS and JS files alway

In detail, it is sufficient to modify the functions: getMergedJsUrl and getMergedCssUrl of app/code/core/Mage/Core/Model/Design/Package.php

public function getMergedJsUrl($files)
    {
        $olderTimestamp = '';
        foreach ($files as $file) {
            if ($olderTimestamp < filemtime($file)) {
                $olderTimestamp = filemtime($file);
            }
        }

        $targetFilename = md5(implode(',', $files) . "|{$olderTimestamp}") . '.js';
        ...

public function getMergedCssUrl($files)
    {
        ...
        // merge into target file
        $olderTimestamp = '';
        foreach ($files as $file) {
            if ($olderTimestamp < filemtime($file)) {
                $olderTimestamp = filemtime($file);
            }
        }

        $targetFilename = md5(implode(',', $files) . "|{$hostname}|{$port}|{$olderTimestamp}") . '.css';
        ...

I hope to propose a commit soon. (If someone is faster than me, welcome).

[fballiano]

wasn't it using the same name because it was already generating a lot of files? will this lead to even more files being generated?

I don't using the merging since some time (since HTTP2 actually) so I don't know

[boesbo]

wasn't it using the same name because it was already generating a lot of files? will this lead to even more files being generated?

I don't using the merging since some time (since HTTP2 actually) so I don't know

The file it generates is always only 1 (1 css file and 1 js file), what will change with the PR I made is the HASH for general naming which will take into account the timestamp of the files. This is a patch I have in my Magento 1 for years.

[fballiano]

sure but the the files stay there forever no? so before they were being overwritten and now they change so a new one is generated? am i getting everything wrong?

[kasper-agg]

I'm sorry for not being able to provide code or a PR. We've solved the issue by checking the content of the file (I believe sha1).

This way changing the content yields a different filename (hash), which can be cached locally by browsers.

[boesbo]

sure but the the files stay there forever no? so before they were being overwritten and now they change so a new one is generated? am i getting everything wrong?

The question is this, I will give you an example. If my project has four JS files:

- vendors.js
- main.js
- slide.js
- mixed.js

If I decide to merge the files with the current function: $targetFilename = md5(implode(',', $files) . '.js';

The result will always be this: 6f4aac17233f21f6b0278cc6f258ddb4.js

So, for example, if I edit the contents of main.js tomorrow, the merged file is created correctly, but will always have the same name: 6f4aac17233f21f6b0278cc6f258ddb4.js, so if I am returning static assets like JS and CSS with a cache, having the same name, the visitor will never download the new JS file, because he has a file with the same name in the browser cache.

So it is necessary when merging files to take into account whether the files being merged have any changes, so that a different filename is generated and therefore you will be sure that the visitor always downloads the latest version.

[boesbo]

I'm sorry for not being able to provide code or a PR. We've solved the issue by checking the content of the file (I believe sha1).

This way changing the content yields a different filename (hash), which can be cached locally by browsers.

Yes, I did the PR, and it's linked to this issue. It's here: https://github.com/OpenMage/magento-lts/pull/4004

[kasper-agg]

I'm sorry for not being able to provide code or a PR. We've solved the issue by checking the content of the file (I believe sha1). This way changing the content yields a different filename (hash), which can be cached locally by browsers.

Yes, I did the PR, and it's linked to this issue. It's here: #4004

Apologies, I meant I am currently unable to provide an example (being it code or a full PR). What I'm trying to say is that we solved it by actually checking the contents if the js and css instead of relying on timestamps. They can be a bit unrealiable in some cases.