$templateFactory (0.3.1) is not using $templateRequest of AngularJS for templateUrl

[anantanandgupta]

With the use of $templateRequest it is possible to have additional configuration while making the requests for the template. But since the ui-router's $templateFactory is not using that, it is not possible to have additional configuration for the ui-view's templateUrl.

In my project (ASP.Net MVC), all the views are coming from acition views and we need to pass the headers to all the requests for view. But it seems it is not possible if I am using templateUrl for the ui-view.

I have overcome the issue by decorating the factory and overwriting the implementation.

[christopherthielen]

If you can get all the tests passing, Ill merge a PR. The code should be compatible with angular 1.2 and fall back to $http if $templateRequest is not avilable, or the user chooses to disable it (via $templateRequestProvider)

See this previous PR #1900

[anantanandgupta]

Hi Chris,

I will take some on that and will post a pull request. But I am not sure that I will be able to make it compatible to Angular 1.2.x.

One question though what do you mean by disabling $templateRequestProvider

[christopherthielen]

Wonderful news!

ui-router supports ng 1.2. The code should be compatible with 1.2. It should fall back to $http if $templateRequest is not available (but use $templateRequest if it is available). To achieve this, we can use $injector.has function to check if $templateRequest exists. https://code.angularjs.org/1.2.0/docs/api/AUTO.$injector#methods_has

One question though what do you mean by disabling $templateRequestProvider

I meant to type $templateFactoryProvider. The user should also be able to opt-in to the old behavior (fetching templates using $http) if they choose. We should create a templateFactoryProvider and add a config option.

[anantanandgupta]

Hi @christopherthielen,

I have spent some time on this. The only restriction I face that there is a requirement of sending the requests with header "Accept : text/html". I am not sure why that restriction is set. with $templateRequest there is no option to pass the configuration option via service call. It can only be configured in the config block as the global setting. So if we somehow move the responsibility of setting that header to the config section, as that if user requires it, he can set that, rather enforcing it. We can do this modification and make it support all the AngularJS versions (with of course fallback to $http).

For the changes I have done in my local were simply replacing the $http#get with $templateRequest as I am on Angular 1.5.8. and it works flawlessly.

Please suggest!

[christopherthielen]

@anantanandgupta

So if we somehow move the responsibility of setting that header to the config section, as that if user requires it, he can set that, rather enforcing it.

I think that is OK. If we can't send a specific header in $templateRequest, then it's up to the user to set that header in the $templateRequestProvider config (if they require the header)

[anantanandgupta]

Thats a great news. I will do the required modifications and remove the test case validating that the header Accept: text/html is set. Once done, will raise the pull request.

[anantanandgupta]

Hi @christopherthielen I have create a pull request and would like you to have a look at that.

Here is the PR number #3155.

[rjamet]

Hi, interested bystander here :) this seems like a very positive change IMO, but keep in mind that $templateRequest will make template URLs go through the $sce.getTrustedResourceUrl code with it, while it doesn't right now. In the current state of things, ui-router bypasses the usual security checks Angular does on templates (for ng-include, for instance).

These security policies might block some resources from loading: by default, it allows anything same-origin. You might have users breaking because of that (they'll get https://docs.angularjs.org/error/$sce/insecurl), and in these cases, the way to force template loading is to tell them to either adjust their whitelist or to use $sce.trustAsResourceUrl on the template url.

So, going back to your PR, I'd strongly advise to test what happens with trusted objects as inputs, instead of just string-based URLs, as they have caused issues in similar refactoring I've worked with. If you provide externs for your library, they'll probably need to be updated too: the template URL sinks should also accept !Object.

Finally, I'm glad to help if you need to :)

[anantanandgupta]

Hi @rjamet,
I understand your point, but i guess whatever you are saying is achievable with in the current feature list. It is just extras step of work they have to do.

I kind of lost you there in 3rd para. Can you please explain it to me.

[rjamet]

It's totally achievable, but I meant that this is worth testing (at least informally).

The security is there, and will throw if you try to do something that Angular considers dangerous, like loading a template from another domain. You can bypass that by wrapping URLs with $sce.trustAsResourceUrl, same as you'd do with trustAsHtml when binding HTML that shouldn't be sanitized, and that turns the string into an opaque Object.

The last paragraph is just my experience doing these refactorings: there's very often small assumptions that the thing is a string somewhere in the flow, and that's the kind of stuff which is really easy to test but hard to review for. But yes, it doesn't sound like much work.

[anantanandgupta]

I know it is much of a work to ask, but will that be possible for you to do the tests you are talking about. I am just a 3 months old personal on AngularJS. Also,
I don't have experience in testability of JavaScript frameworks. This is my very first contribution in any open source projects. So the success of this PR matters a lot for me.

[rjamet]

I have a little work backlog right now, but I'll take a look and send your branch a PR by the end of the week.

[anantanandgupta]

Thanks @rjamet

[christopherthielen]

Closed by #3182