feat($templateFactory): use $templateRequest on 1.3

[nateabele]

Fixes #1882

cc: @matsko

This fails one test which I'm trying to figure out how to resolve.

[kasperlewau]

text/html is not part of the default $http headers being used by $templateRequest and needs to be set explicitly for your test to pass.

$templateRequest does not allow for the passing of HTTP headers by the looks of the source, so my best bet is that you would have to touch the $http.defaults.headers prior to running a $templateRequest and reset it once you're done. yuck.

The following will make your tests and implementation pass (not very happy with how this turned out):

  this.fromUrl = function (url, params) {
    if (isFunction(url)) url = url(params);
    if (url == null) return null;

    if ($templateRequest) {
      // .get is not set as a default object in $http.defaults.headers.
      // note: some consumers probably _have_ set it, so this is a naive 'hack' to showcase the issue.
      $http.defaults.headers.get = { Accept: 'text/html' };
    }

    return ($templateRequest && $templateRequest(url) || $http.get(url, {
      cache: $templateCache,
      headers: { Accept: 'text/html' }
    })).then(function(response) {
      // Cleanup what we did in the above if block.
      delete $http.defaults.headers.get;
      return response.data;
    });
  };

  it('should request templates as text/html', inject(function($templateFactory, $httpBackend) {
    $httpBackend.expectGET('views/view.html', { 'Accept': 'text/html' }).respond(200, '<h1>template</h1>');
    $templateFactory.fromUrl('views/view.html');
    $httpBackend.flush();
  }));

Note that the template has been filled in in the expectGET - if the response is empty, $templateRequest will fail. You can override that by passing true as the second parameter to the $templateRequest call - which might be something that you should consider supporting in the fromUrl method.

All in all - very naive and ugly code in my opinion (at least the edits I ran to make it all work), so I would await more info from matsko on the issue.

edit; With all of that said, is it necessary to specify the { Accept: 'text/html' } header in the case of $templateRequest? I was under the impression that it should just work™ in the case of $templateRequest as it is specifically built for fetching and storing HTML. Could verify it by setting some expectations on the returned data of your request I suppose.

[nateabele]

@kasperlewau Thanks for your efforts. Couple things though. First, note the error:

    EXPECTED: function (headers) {
          return headers.Accept === 'text/html';
        }
    GOT:      {"Accept":"application/json, text/plain, */*"}

Basically, $httpBackend used to accept a function, now it only accepts an object hash. This is the biggest thing I can't get around right now.

The secondary issue is the Accept header. We added that because some people's servers weren't serving templates correctly otherwise... this seems like something $templateRequest should probably handle.

@matsko I can submit a PR for the $templateRequest thing, but do you have any ideas what I could do about $httpBackend? Thanks.

[Narretz]

In 1.5, you can use the https://code.angularjs.org/1.5.0/docs/api/ng/provider/$templateRequestProvider to set the defaults for templateRequests. There's however the chance that this is overwritten in a different config block - but then the user can / must set the appropriate headers anyway. In 1.4.x, you can use an interceptor, which is still a bit hacky, since we can only detect a ui-router request via the url: http://plnkr.co/edit/Q51qlNzdwmaL7knOb4kg?p=preview

Actually, I think if people have a problem with their backends accepting these requests, they should use interceptors to manipulate the headers based on the urls (over which they have control anyway).

[nateabele]

Actually, I think if people have a problem with their backends accepting these requests, they should use interceptors to manipulate the headers based on the urls (over which they have control anyway).

@Narretz Yeah, that's kind of my feeling on it as well.