Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

feat($templateRequest): support configuration of accept headers #13188

Closed
wants to merge 3 commits into from

Conversation

petebacondarwin
Copy link
Contributor

It is now possible to configure the Accept header for template requests.
If a value is configured then this will override only the Accept property
of the headers that are passed.
If no value is configured then the request will use the default $http
Accept header.

Thanks to @luckycadow for help on this feature

Closes #11868
Closes #6860

@pkozlowski-opensource
Copy link
Member

@petebacondarwin the code looks good to me, but I wonder if the current design is not too restrictive. Maybe we should allow people to set any headers and not limit ourselves to Accept only?

If all the use-cases we know of are boiling down to setting Accept I'm fine with landing this as-is. But maybe we should make it more generic?

@petebacondarwin
Copy link
Contributor Author

@pkozlowski-opensource thanks for the review. I think that you are right. It might even bring the code size down a touch! I will update.

It is now possible to configure the Accept header for template requests.
If a value is configured then this will override only the Accept property
of the headers that are passed.
If no value is configured then the request will use the default $http
Accept header.

Thanks to @luckycadow for help on this feature

Closes angular#11868
Closes angular#6860
It is now possible to configure the options sent to $http for template requests.
If no value is configured then the request will use the default $http options.

Thanks to @luckycadow for help on this feature

Closes angular#11868
Closes angular#6860
@petebacondarwin
Copy link
Contributor Author

@pkozlowski-opensource - any chance you can take another look?

return $http.get(tpl, extend({
cache: $templateCache,
transformResponse: transformResponse
}, httpOptions))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be other way around? I mean, we should always enforce cache: $templateCache and certain transform, no?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test for a case where someone tries to specify cache?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point of modifying the transformResponse is that the default $http service adds in a JSON transformation, which we don't want in $templateRequest.

If we are going to allow people to configure the options then we should allow them to override everything, including this transformResponse and also to provide their own cache if we really wanted them to; although the same could be achieved by overriding the $templateCache service itself.

I can add a test for overriding the cache but it is really just the same code path as overriding the transformResponse.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doh! I see what you mean. You cannot create a cache object in the provider...

It is now possible to configure the options sent to $http for template requests.
If no value is configured then the request will use the default $http options.

Thanks to @luckycadow for help on this feature

Closes angular#11868
Closes angular#6860
@petebacondarwin petebacondarwin deleted the pr-11868 branch November 24, 2016 09:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants