Cache busting does not account for changes to main.js

[Toutouwai]

Short description of the issue

After upgrading the PW version of a site I get this error:

Uncaught TypeError: ProcessWire.trim is not a function at urlKeydown (ProcessPageEditLink.min.js?v=112-1689898720:1:3921)

ProcessWire.trim() was added in PW 3.0.216: https://github.com/processwire/processwire/blob/c1a939be0cd4ad3f49b7cd1b38dd5d5cb588021c/wire/templates-admin/scripts/main.js#L534

But the cache-busting version string for main.js only changes based on the version number of the AdminTheme: https://github.com/processwire/processwire/blob/c1a939be0cd4ad3f49b7cd1b38dd5d5cb588021c/wire/modules/AdminTheme/AdminThemeUikit/_head.php#L17 https://github.com/processwire/processwire/blob/c1a939be0cd4ad3f49b7cd1b38dd5d5cb588021c/wire/modules/AdminTheme/AdminThemeUikit/_head.php#L29

The version number of AdminThemeUikit hasn't changed since May 2021: https://github.com/processwire/processwire/commit/401fcb078125dcb99cc1ad6383f0fc11e957bac5 And the letter suffix hasn't changed since July 2021: https://github.com/processwire/processwire/commit/003c7f91b0cf7664509998a1d4b7fe4cae336144#diff-64a4fd5ec14df300f1835eab32deff8cd30897483596b07c4cb6f5e47ddb0e6c

This approach to cache-busting seems fraught with risk because any time there is a change to admin JS/CSS assets Ryan will need to remember to update the version for admin theme(s).

Would it not be better to use filemtime() for the cache-busting string? There would be some small overhead but worth it to avoid issues like this IMO.

Setup/Environment

• ProcessWire version: 3.0.223

[matjazpotocnik]

@Toutouwai, I know it doesn't solve the issue, just to let you know that Ryan bumped the version number of admin themes in this commit.

[ryancramerdesign]

I prefer not to add any more filemtime() checks than is absolutely necessary in the short term. Perhaps we will later, when I can better quantify the effect on overhead, if any.

[cb2004]

Could you not just add the ProcessWire version in the calls to these ?3.0.204, that way you don't need to have to remember to update anything, and you are not adding any overhead.

[Toutouwai]

Okay, will close this but I still think needing to remember to update _head.php any time one of a number of different files is edited is rather error prone.

For the record, another case of the same issue is this CSS glitch that I have seen many times when updating sites but never got around to reporting:

[ryancramerdesign]

Could you not just add the ProcessWire version in the calls to these ?3.0.204, that way you don't need to have to remember to update anything, and you are not adding any overhead.

@cb2004 This makes sense to me. Too obvious actually, why didn't I think of that. :) I'll add it this week, and this would enable us to get rid of some of the existing filemtime() calls, perhaps just delegating them to debug mode.

[Toutouwai]

@ryancramerdesign, does this latest commit mean you intend to bump the PW version number every time there is a change to any core JS or CSS? Because anything less than that doesn't solve the problem IMO. It actually makes it worse because before I believe there was at least the intention to bump the admin module version number even if in practice it was easy to overlook.

There are typically many commits between version number changes - for example 24 commits between v3.0.223 and v3.0.224 over a period of two weeks. If any of those commits involve a change to JS or CSS then there is the potential for cache-related bugs when users of the dev branch update a site.

Take the bug that prompted this issue: this caused the total breakage of the PW link dialog - I had clients contacting me because they couldn't insert any links. I use ProCache and the recommended htaccess settings set the cache expiry time for JavaScript to "access plus 1 year". That's a long time to wait for a cache-related problem to resolve if the core doesn't correctly deal with cache-busting.

In my opinion the admin is too aggressive with caching. This is another example: https://github.com/processwire/processwire-requests/issues/268 When it comes to the admin back-end I would much rather have bug avoidance prioritised over very tiny performance advantages. In the linked request my testing showed virtually no performance advantage to caching the admin menus. It's a similar story for filemtime()...

I tested by looping over 75 JS and CSS assets (I can't imagine any single admin page loading more than this) and the total time cost is negligible.

$t = Debug::timer();
$items = [
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldAsmSelect\InputfieldAsmSelect.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldCheckboxes\InputfieldCheckboxes.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldCKEditor\InputfieldCKEditor.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldDatetime\InputfieldDatetime.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldFile\InputfieldFile.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldIcon\InputfieldIcon.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldImage\InputfieldImage.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldPage\InputfieldPage.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldPageAutocomplete\InputfieldPageAutocomplete.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldPageListSelect\InputfieldPageListSelect.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldPageListSelect\InputfieldPageListSelectMultiple.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldPageName\InputfieldPageName.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldPageTable\InputfieldPageTable.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldPageTitle\InputfieldPageTitle.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldPassword\InputfieldPassword.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Fieldtype\FieldtypeRepeater\InputfieldRepeater.min.js',
    'D:\_Websites\_www\1raw\wire\templates-admin\scripts\inputfields.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldSelector\InputfieldSelector.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldText\InputfieldTextLength.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldTextTags\InputfieldTextTags.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Inputfield\InputfieldToggle\InputfieldToggle.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessCommentsManager\ProcessCommentsManager.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessField\ProcessField.min.js',
    'D:\_Websites\_www\1raw\wire\modules\LanguageSupport\ProcessLanguage.min.js',
    'D:\_Websites\_www\1raw\wire\modules\LanguageSupport\ProcessLanguageTranslator.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessLogger\ProcessLogger.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessLogin\ProcessLogin.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessModule\ProcessModule.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPageAdd\ProcessPageAdd.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPageEdit\ProcessPageEdit.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPageEditImageSelect\ProcessPageEditImageSelect.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPageEditLink\ProcessPageEditLink.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPageList\ProcessPageList.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPageLister\ProcessPageLister.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPageSearch\ProcessPageSearch.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPagesExportImport\ProcessPagesExportImport.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPageType\ProcessPageType.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessProfile\ProcessProfile.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessRole\ProcessRole.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessTemplate\ProcessTemplate.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessTemplate\ProcessTemplateFieldCreator.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessUser\ProcessUser.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Jquery\JqueryCore\jquery.cookie.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Jquery\JqueryMagnific\JqueryMagnific.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Jquery\JqueryWireTabs\JqueryWireTabs.min.js',
    'D:\_Websites\_www\1raw\wire\modules\LanguageSupport\LanguageTabs.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Markup\MarkupAdminDataTable\MarkupAdminDataTable.min.js',
    'D:\_Websites\_www\1raw\wire\modules\System\SystemNotifications\Notifications.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Page\PageFrontEdit\PageFrontEdit.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Page\PageFrontEdit\PageFrontEditLoad.min.js',
    'D:\_Websites\_www\1raw\wire\modules\Jquery\JqueryMagnific\JqueryMagnific.css',
    'D:\_Websites\_www\1raw\wire\modules\Markup\MarkupAdminDataTable\MarkupAdminDataTable.css',
    'D:\_Websites\_www\1raw\wire\modules\Markup\MarkupPagerNav\MarkupPagerNav.css',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPageEdit\PageBookmarks.css',
    'D:\_Websites\_www\1raw\wire\modules\Page\PageFrontEdit\PageFrontEdit.css',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessCommentsManager\ProcessCommentsManager.css',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessField\ProcessField.css',
    'D:\_Websites\_www\1raw\wire\modules\LanguageSupport\ProcessLanguage.css',
    'D:\_Websites\_www\1raw\wire\modules\LanguageSupport\ProcessLanguageTranslator.css',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessLogger\ProcessLogger.css',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessLogin\ProcessLogin.css',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessModule\ProcessModule.css',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPageAdd\ProcessPageAdd.css',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPageEdit\ProcessPageEdit.css',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPageEditImageSelect\ProcessPageEditImageSelect.css',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPageEditLink\ProcessPageEditLink.css',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPageList\ProcessPageList.css',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPageLister\ProcessPageLister.css',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPageSearch\ProcessPageSearch.css',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPagesExportImport\ProcessPagesExportImport.css',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPageType\ProcessPageType.css',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessPermission\ProcessPermission.css',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessRole\ProcessRole.css',
    'D:\_Websites\_www\1raw\wire\modules\Session\SessionHandlerDB\ProcessSessionDB.css',
    'D:\_Websites\_www\1raw\wire\modules\Process\ProcessTemplate\ProcessTemplate.css',
];
foreach($items as $item) {
    $mtime = filemtime($item);
}
d(Debug::timer($t));

The way I imagine this being implemented is something like a core getAsset($path) method that would return the asset URL with the cache-busting filemtime string appended. And it could do the minified/non-minified substitution based on debug mode - would be quite nice to have this centralised so it's across-the-board rather than case-by-case, ref: https://github.com/processwire/processwire-requests/issues/244

[szabeszg]

For my frontends, I've been using the following ever since I started using PW, so assets are always fresh:

<?php namespace Novo;

class Tools {

    /**
     * Adds file modified date as query string and returns URL to given asset.
     * <script src="<?= Tools::assetUrl("assets/js/site.js") ?>"></script>
     *
     * @param $relPath string File path relative to /site/templates/, such as "assets/css/site.css"
     * @return string URL with cache busting query parameter
     */
    public static function assetUrl(string $relPath): string {
        $fullPath = wire()->config->paths->templates . $relPath;
        $modified = "";

        if (file_exists($fullPath)) {
            $modified = "?v=" . filemtime($fullPath);
        } else {
            wire('log')->error("WARNING: " . __METHOD__ . " reports: file of {$fullPath} is missing! (current page ID: " . page() . ")", Notice::debug);
        }

        return wire()->config->urls->templates . $relPath . $modified;
    }
}

[ryancramerdesign]

It really depends on the file system. Locally things are very fast with filemtime(). On our AWS setup, not so much, I suppose because the file system is distributed. For now we'll stick with the core version based method, as that's really the one that I think will matter most outside of debug mode. But maybe we'll add an optional file time based one, once we're back to adding features.

[BernhardBaumrock]

@ryancramerdesign I think @Toutouwai is making some very good points here.

I think this is also related to my old request that we need some kind of generic ->url() and ->path() methods. See https://github.com/processwire/processwire-requests/issues/326 It feels weird now to have them on $config but that does not matter too much where we add them. The point is that we need an easy way to query for the url, or the path, or convert between those two and - now comes the relation to this topic - when requesting the url we also need an easy way to add a cache busting timestamp.

I think we can do all that with two simple methods and solve all the problems at once: $something->path(...) and $something->url(...).

I've been using filemtime() for this in RockFrontend and RockMigrations all over without any noticeable lag. But I did not think of AWS or such. I think PW should provide the proper API and handle the complexity in the background as always.

Some examples

$path = "/some/project/site/templates/file.css";

$url = $pw->url($path, true); // url with cache busting timestamp: site/templates/file.css?m=1234567
$pw->path($path); // /some/project/site/templates/file.css
$pw->path($url); // /some/project/site/templates/file.css
$pw->url($path); // site/templates/file.css

[Toutouwai]

If getting the admin asset URLs was delegated to some method it would be easy to allow the user to choose between the PW version number (maybe fine if you never update sites to the latest dev within a single version number) and filemtime (if you want zero risk of exposing users to cache-related bugs), via a $config setting.

For the record, here is the same test on a shared cPanel hosting:

[BernhardBaumrock]

I've tried something similar:

$files = $files->find($config->paths->wire, ['extensions' => ['css', 'js']]);
db($files);
$timer = Debug::timer();
foreach($files as $file) {
    $m = filemtime($file);
}
db(Debug::timer($timer));

Macbook Air M1

VPS

[teppokoivula]

Another AWS user here — just posting to say that I'm quite happy with core version based approach.

I get that there may be cases where core version remains as-is for a week or two, but to me that seems like a relatively small issue, and I would assume that it will mostly only affect those using the dev branch. Which should not be the normal case, in my opinion.

For e.g. EFS file read/open/fetch delays are a real problem. The problem is that the more files you access, the worse it gets; these systems are optimized for accessing fewer but larger files, while accessing many small files is where they struggle :)

[BernhardBaumrock]

@teppokoivula great input. But I think we should not forget the frontend. I think we have the same problem there and I think PW should provide a simple and bullet proof solution.

I just had an idea.

I get that it might not be as easy as just using filemtime() all over. But on the other hand I hate to add cache busting timestamps manually. I think we have two situations with different priorities here:

1) Development 2) Production

During development the priority would be ease of use. So we could use a filemtime() based solution. This approach would be very fast as we are working on a local filesystem and also the penalty should not really matter.

On production on the other hand we might not want the overhead. We want the fastest solution possible, but still I don't like to update version numbers manually. So why not simply rely one one single version number or timestamp or whatever?

In my projects where I use automated deployments I always get a package.json for example that holds the version number of the deployment. If I could tell PW to use this file as a base for the cache busting timestamp then it would be equally easy to use (I don't have to think about updating cache busting urls manually) and it should always work (=flush the cache) and it should also be almost as performant as a single cache busting string would be.

I have different configs for both environments anyhow, so I could add a setting like this:

// config-local.php on DEV
// use filemtime() of every file requested
$config->cacheBusting = true;

// config-local.php on Production
// use filemtime of package.json for all cache busted urls
$config->cacheBusting = "package.json";

// alternative example with non-root file
$config->cacheBusting = "site/templates/foo.txt";

The default setting here could be true. In that case any call of ->url(..., true) would return an url like foo.css?m=1234 where 1234 would be the filemtime of exactly that file.

If anybody needs more performance or has a non-standard setup like AWS he/she could just add the config setting to config-local.php and that's it.

BTW: This is another example why my PR about config-local.php would make sense. Namely to have a standard way of doing (and communicating) these things: https://github.com/processwire/processwire/pull/267

[Toutouwai]

I'm not going to die in ditch over it or anything, but one more point: after the recent commit if you do strike a cache-related bug the options for resolving it are:

1. Contact every admin user and and explain to them that they must clear their browser cache, and how to do that, and the reason is that their CMS software doesn't automatically handle changes to the included JS and CSS. That's pretty embarassing I think.
2. Editing the core ProcessWire.php to change the version number. That sounds like it could have many unintended consequences.

Personally I'd prefer a few milliseconds of additional load time in the admin where it hardly matters, but if it was configurable then each user could make that choice for themselves.

[szabeszg]

1. Contact every admin user and and explain to them that they must clear their browser cache, and how to do that, and the reason is that their CMS software doesn't automatically handle changes to the included JS and CSS. That's pretty embarrassing I think.

This keeps happening to all of us, and I do agree that this is embarrassing and clients consider ProcessWire less less reliable, just because such thing can happen.

Why can't it be made configurable?

Why do those who use a host which has no issue with using filemtime() cache busting have to struggle with this embarrassing issue?

Please let us decide (by making it configurable) what is OK and what is not in cases like this.

[ryancramerdesign]

Lots of good ideas to revisit after the next master version. Admittedly, I've not run into any of these browser cache issues, despite being the one that makes the changes to the files. When I see something unexpected with CSS/JS, I hit reload in the browser. I guess it's an old automatic habit. Redundant filemtime calls on every-single request served by the admin just seems profligate, and I'd like to avoid that. I felt that our existing use of filemtime calls was too much. Though obviously many file systems are up to it, so likely not an issue for most. Either way, multiply the overhead by every single admin request and it adds up. If there are going to be lots of filemtime calls I'd want to be a bit more respectful with it, maybe caching in the session or something so that it doesn't have to do that on every single request. But when debug mode is enabled, redundant filemtime calls are fine.

[Toutouwai]

Okay, will close and revisit this later after the next master, maybe with a PR.

[ryancramerdesign]

@Toutouwai Have a look at the $config->versionUrls() method that was just aded. This uses filemtime based URLs when config.debug is true or the dev branch is being used. Still left to do is remove the versioned URLs that are added elsewhere since this does them all at output time.

[Toutouwai]

@ryancramerdesign, fanstastic, thanks for adding this.

The only thing I'm not clear on is if the default version string is configurable by the user. I think it would be good if it was because based on the different comments in this discussion it seems like people will differ on if/when they want filemtime used for the version string.

Personally I will want filemtime on all the time regardless of debug or master/dev version because it has negligible performance impact in the hosting environments I use and I want the option that is most reliably going to avoid file caching issues.

But others may want the auto-detect option, and others still may want the PW version only option.

So could the choice of version string be determined by a $config setting?

[ryancramerdesign]

@Toutouwai I've updated it so that you can specify what you want the default $useVersion argument to be in your /site/config.php file. See here for values that can be specified.

[BernhardBaumrock]

Great addition, great solution! Thx :)