Beginning with angular-1.3.6 $templateRequest does not populate $templateCache

[zoowar]

I upgraded from angular-1.3.0 to angular-1.3.8 and noticed that $templateCache.get(template) was not consistently returning a template string. Without getting into the code I determined the issue was introduced in angular-1.3.6.

I use $templateRequest to load templates into the $templateCache. Beginning with angular-1.3.6, the first call to $templateCache.get(template) returns the template string. All subsequent calls return an array in the following form [200, "template string", Object, "OK"].

[caitp]

You should let $http deal with this for you, rather than trying to cut in manually. Otherwise, look at how $http deals with the problem if you really need to imitate it

[zoowar]

caitp,
If you're suggesting I replace the call to $templateRequest with something like

$http.get(template, config)
    .success(function(data, status, headers, config) {
        $templateCache.put(template_name, data);
    }).error(function(data, status, headers, config) {
      // handle this
    });

It would work, but I would have to verify it doesn't have the same issue. However, your comment begs the question, "What's the point of $templateRequest"? I prefer to use $templateRequest, and if it's broken, have it fixed.

[caitp]

$templateRequest is a wrapper for $http which never tries to parse JSON, among other things. You were saying $templateCache before.

$http and $templateRequest will both deal with caching for you (with $http explicitly requiring that you ask for it via cache: true or cache: $templateCache)

[zoowar]

caitp,

I didn't change the terms I'm using. If you reread the initial comment, how I use both $templateRequest and $templateCache is clear.

I just looked at the diffs of angular-1.3.5 and angular-1.3.6 and see that $TemplateRequestProvider no longer calls $templateCache.put(tpl, html); when it returns $http.get(...);

Here is the commit that introduces the issue.
8c3a42c

[caitp]

You're misunderstanding what I'm saying...

If you use $http.get() or $templateRequest(), it will never give you the data as the array cache entry. The contents of the cache should be irrelevant to you, they're meant to be managed by those classes. You should not be messing with them (unless you know what you're doing, and are willing to deal with breaking changes)

[zoowar]

I don't know what you mean by "messing around with them". I don't do that. I populate the cache implicitly by calling $templateRequest. Later, when I'm ready to load that page, I call $templateCache.get to get the template for the content controller.

[caitp]

I call $templateCache.get to get the template for the content controller.

This is what I'm telling you not to do, unless you're prepared to deal with the ways $http messes with the cache entry (how it currently messes with the cache entry in http.js). I'm not speaking a weird language here, this is just english. You're mixing manual cache management with $http cache management, and expecting $http not to do the stuff it does with it. But we want $http to do those things, so that we can provide better info (to $http, and thus to callers of the API)

[zoowar]

I don't see the documentation for https://docs.angularjs.org/api/ng/service/$templateCache provide any information regarding "the ways $http messes with the cache entry". When I call $templateCache.get(...), I expect a string representing the template in the first position result.

[gkalpak]

@zoowar: $templateCache is a "normal" cache created by Angular (using $cacheFactory) to accomodate for its template caching needs. As such, Angular stores there whatever maked sense for her needs. You are free to access it directly, but there should be no assumptions regarding its content (and their format).

The "proper" way to handle requesting templates is using $templateRequest, which will internally use $templateCache/$http to retrieve the template, so if it is already cached it will return the cached instance without making an actual HTTP request.

@caitp: That said, @zoowar seems to have a point that currently the docs imply that one can use $templateCache.GET to retrieve a cached template (which isn't exactly the case since 1.3.6).
Do you think we should revise the docs or rather refactor template caching to either have $templateCache.get return the template string or consistently return an array (even for manually cached templates) ?

[zoowar]

Trivial to reproduce:

$templateRequest(template).then(
      function(data) {
        // print the data to see that it is in fact the template string.
        console.log('$templateRequest: OK', data);
        // next we examine the $templateCache.
        console.log('$templateCache.get:', $templateCache.get(template));
        // Oops! The result is an array and not the template string.
        // If we call $templateCache.put(template, data) at this point,
        // subsequent calls to $templateCache.get(template) will return
        // the expected template string.
      },
      function(data) {
        console.log('$templateRequest: not OK', data);
      });

[caitp]

@caitp: That said, @zoowar seems to have a point that currently the docs imply that one can use $templateCache.GET to retrieve a cached template (which isn't exactly the case since 1.3.6).
Do you think we should revise the docs or rather refactor template caching to either have $templateCache.get return the template string or consistently return an array (even for manually cached templates) ?

If you want to add a snippet like "DON'T USE $templateCache.get() UNLESS YOU KNOW WHAT YOU'RE DOING", I think that's fine. Another option is, $templateCache could have an extra method for getting "just" the template text, and nothing else.

[pkozlowski-opensource]

Let me comment, since it was me who pushed 8c3a42c: I agree with both @caitp and @gkalpak that it should be $templateRequest business and no one else how it goes about caching data and what internal format it uses / what it caches.

Now, the only practical problem I can see here is a situation where people are mixing "pre-loaded" templates with dynamically requested ones via $templateRequest - in such a case the $templateCache will contain strings for pre-loaded templates and arrays for the dynamically retrieved ones. This will be transparent to most people due to this check in $http but people accessing $templateCache directly will see "inconsistent" entries so will be obliged to do the same check as $http does.

So, it all kind of works and makes sense but maybe we should revert this commit (+ add explicit tests for this) as not to confuse people? I would prefer to do this rather than adding more docs, to be honest.

[zoowar]

Those of us dynamically loading templates use $templateRequest() to load the template, which is asynchronous, followed by $templateCache.get() because it is synchronous. Also, I presume that calling $templateCache.get() is more efficient than calling $templateRequest(), once the cache has been populated.

From my perspective, I see two reasonable approaches to this issue:

1. Provide a synchronous option for $templateRequest() that calls $templateCache.get() and converts the internal representation to the expected string or undefined, and does not follow the $http path.
2. Abstract $templateCache into a service that wraps the $cacheFactory('templates') object and converts between the internal representation the string representation.

[caitp]

Did you profile your app and decide that you needed to use the cache api incorrectly in order to avoid waiting for the next tick? It really looks like you're trying to solve the wrong problem here

[pkozlowski-opensource]

@zoowar retrieving a template is a asynchronous operation, by definition. Trying to "work-around" it by using $templateCache is a bad idea as effectively you try to turn asynchronous operation into a synchronous one which will "work" only in certain cases.

You wouldn't notice any problem if you would be using $templateRequest or $http + $templateCache to retrieve templates.

Once again, we can easily revert the change done 8c3a42c, this is really not a big deal. But we want to make sure that we do it for good reasons.

[zoowar]

The documentation leads me to believe that $templateRequest and $templateCache are public APIs and that $templateRequest is a utility service to populate $templateCache when the template string is not local to the client application. 8c3a42c breaks Semantic Versioning by making an incompatible API change in a patch version.

[pkozlowski-opensource]

@zoowar this is not really correct statement, really, for multiple reasons:

• $templateRequest is not a utility service to populate $templateCache - it is an utility service to retrieve templates content
• AngularJS doesn't strictly follow semantic versioning but even if it would we wouldn't mark it is as a breaking change since no public contract was broken here - this is more like a side effect.

Anyway, I think we've spent more time arguing about it than it is worth it so I'm going to send a patch. We just wanted make sure that you are not shooting yourself in a foot with a way you use those APIs together...