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: processwire/processwire@401fcb0
And the letter suffix hasn't changed since July 2021: processwire/processwire@003c7f9#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: processwire/processwire-requests#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: processwire/processwire-requests#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 processwire/processwire-requests#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