Current $templateRequest implementation breaks $templateCache decorators

[djfd]

I'm submitting a ...

• bug report
• feature request
• other (Please do not submit support requests here (see above))

Current behavior:

It is impossible now to decorate $templateCache.put()

Expected / new behavior:

Expected to be working as per documentation

Minimal reproduction of the problem with instructions:
currently the code is

 .then(function(response) {
            $templateCache.put(tpl, response.data);
            return response.data;
}

which should be

.then(function(response) {
            return $templateCache.put(tpl, response.data);
}

instead in order to allow proper $templateCache.put() decoration

AngularJS version: master branch

Browser: [all | Chrome XX | Firefox XX | Edge XX | IE XX | Safari XX | Mobile Chrome XX | Android X.X Web Browser | iOS XX Safari | iOS XX UIWebView | iOS XX WKWebView ]

Anything else: nope

[gkalpak]

It sounds reasonable. Technically this would be a breaking change though, imo (minor one, but still).
@djfd, would you be interested in working on this?

[djfd]

@gkalpak Sorry, possible I did miss something.

Could you please explain what do you mean saying 'breaking change' and what does it mean 'working on this'?

As per my current understanding, there is nothing actually breaking, and all the working on this is a simple patch which I can prepare in a couple of minutes. As for PRs, I never did it, so my best result would be a simple patch file.

[Narretz]

I don't really understand how the implementation of templateRequest "breaks" the ability to decorate templateCache. The templateCache service is simply responsible for putting a template into the cache, it's not documented to return anything to the templateRequest service.

[djfd]

@Narretz There is the code of $templateCache

Look at line no.177

Now imagine that someone implemented $templateCache.put() decorator to (for example) replace all <div /> with <span />, eg.like this

.config('$provide', function($provide) {
  $provide.decorate('$templateCache', ['$delegate', function(delegate) {
    var original_put = $delegate.put;
    function replace(value) {
       /* do a replacement work */
      return modified_value;
    }
    function custom_put(name, value) {
      return original_put(name, replace(value));
    }
    $delegate.put = custom_put;
    return $delegate;
  }]);
})

Thus it is expected that one requesting a template, eg. by means of ng-include will actually get that we have in the $templateCache storage. Currently, a requestor will get raw original http response instead of actually stored template.

[gkalpak]

I don't really understand how the implementation of templateRequest "breaks" the ability to decorate templateCache.

@Narretz, I don't see the current implementation as broken, but changing it as @djfd proposed looks reasonable to me (given the low complexity and impact).
As mentioned in the docs $templateRequest() uses $templateCache to cache the template, so it makes sense to ensure that the same template is cached and returned. Right now, if $templateCache.put does any transformation then the returned template will be different than what is actually in the cache (and what will be returned the next time the same template is requested).

Could you please explain what do you mean saying 'breaking change' [...]

@djfd, I mean that making the change you proposed can possibly break existing apps. E.g. if someone has overwritten $templateRequest.put but doesn't return anything (because they don't care about the return value), $temlateRequest() will suddently start returning undefined templates.
Therefore, such a change can only be introduced in a "breaking version" - i.e. a vrsion that is allowed to have breaking changes (e.g. 1.7.0).

[...] what does it mean 'working on this'?

Whether you would be interested in creating and submitting a pull request (with the proposed change and some tests), which we could then review(, propose changes) and merge into the project.
You can find more info about contributing here and there.

BTW, it is worth mentioning that one could work around this today by decorating either $templateRequest() to retrieve the template from the $templateCache or using an $http interceptor or transformResponse function for templates.

[djfd]

@gkalpak aha, ic, you did mean third party applications... Thank you for the explanation. Your described scenario looks as having very low probability, but not impossible at all. And I disagreed with the 'feature request' classification, it is rather a bug (documentation contradiction), just IMHO

Going to read your the links, thanks _)

BTW, it is worth mentioning that one could work around this today by decorating either $templateRequest()

Yeah, right now I am using $templateRequest decorator like this

$provide.decorator('$templateRequest', ['$delegate', '$templateCache', function($delegate, $templateCache) {
  function fixture(tpl, ignoreRequestError) {
    return $delegate(tpl, ignoreRequestError).then(function() {
      return $templateCache.get(tpl);
    });
  }
  return fixture;
}]);

[Narretz]

Imo, if you want to change the template, there are other points where you can / should do that. templateCache is so simple, to overload it with a the ability to change a template is weird to me.

I also don't see where it's documented that templateRequest will return the templateCache result.

However, I see that it's more logical that templateRequest does return it.

[djfd]

to change the template, there are other points where you can / should do that

@Narretz Could you please expand your statement a few? I only can imagine an HDD of a server ))

$templateCache is the lowest point in the hierarchy where such change can be done, without the need to change upper logic. It can be convenient, e.g. when customizing an application for a customer. World is imperfect, I know ))

[Narretz]

@djfd as @gkalpak has said,

BTW, it is worth mentioning that one could work around this today by decorating either $templateRequest() to retrieve the template from the $templateCache or using an $http interceptor or transformResponse function for templates.

[frederikprijck]

FYI: I'm working on this: #16434

[djfd]

@frederikprijck Good to know that, thank you for the information!

Out of curiosity, not entire world did switch to angular2 yet?