MDL-68677 core: Correct usage of templaterev for caching templates
authorAndrew Nicols <andrew@nicols.co.uk>
Tue, 12 May 2020 01:18:32 +0000 (09:18 +0800)
committerAndrew Nicols <andrew@nicols.co.uk>
Thu, 14 May 2020 06:10:55 +0000 (14:10 +0800)
The M.cfg.templaterev variable should only be used to present persistent
caching, not caching of content within the same page load.

Preventing caching of same-page content makes it difficult to develop
for real user experiences as content is slow to load and does not give a
realistic and consistent loading experience.

This change affects the loading of partials specifically which notably
includes the loading spinner. Without this patch the loading icon is
often not seen at all because it does not load in a timely fashion and
the content being loaded is loaded first.

lib/amd/build/templates.min.js
lib/amd/build/templates.min.js.map
lib/amd/src/templates.js

index e9bf2bf..c907e2b 100644 (file)
Binary files a/lib/amd/build/templates.min.js and b/lib/amd/build/templates.min.js differ
index e18317b..202d4bd 100644 (file)
Binary files a/lib/amd/build/templates.min.js.map and b/lib/amd/build/templates.min.js.map differ
index 8ac79ab..db830df 100644 (file)
@@ -79,11 +79,6 @@ define([
      * @return {Object} jQuery promise resolved with the template source
      */
     var getTemplatePromiseFromCache = function(searchKey) {
-        // Do not cache anything if templaterev is not valid.
-        if (M.cfg.templaterev <= 0) {
-            return null;
-        }
-
         // First try the cache of promises.
         if (searchKey in templatePromises) {
             return templatePromises[searchKey];
@@ -96,6 +91,11 @@ define([
             return templatePromises[searchKey];
         }
 
+        if (M.cfg.templaterev <= 0) {
+            // Template caching is disabled. Do not store in persistent storage.
+            return null;
+        }
+
         // Now try local storage.
         var cached = storage.get('core_template/' + M.cfg.templaterev + ':' + searchKey);
         if (cached) {
@@ -183,7 +183,11 @@ define([
                                 // Cache all of the dependent templates because we'll need them to render
                                 // the requested template.
                                 templateCache[tempSearchKey] = data.value;
-                                storage.set('core_template/' + M.cfg.templaterev + ':' + tempSearchKey, data.value);
+
+                                if (M.cfg.templaterev > 0) {
+                                    // The template cache is enabled - set the value there.
+                                    storage.set('core_template/' + M.cfg.templaterev + ':' + tempSearchKey, data.value);
+                                }
 
                                 if (data.component == component && data.name == name) {
                                     // This is the original template that was requested so remember it to return.