feat($httpUrlParams): introduce new service abstracting params serialization #10853
Conversation
…ization Closes angular#7429 Closes angular#9224
pkozlowski-opensource
force-pushed
the
http_params_serialization
branch
from
6aae996 to
6728785
Compare
|
maybe we should also move the |
| * @description | ||
| * Use `$httpUrlParamsProvider` to change the default behavior of the {@link ng.$httpUrlParams $httpUrlParams} service. | ||
| * */ | ||
| function $HttpUrlParamsProvider() { |
There was a problem hiding this comment.
Maybe it is better to move this service (and maybe also the relevant unit tests) to a different file?
There was a problem hiding this comment.
Yup, this is the plan as soon as I'm will expand on this service :-)
|
@shahata yes, my bigger plan is to expand this service to handle deserialization in the future and - why not - turn it into a generic URLs params handling service. But I'm taking baby steps with those PRs so those are easier to review and getting merge faster. In short: yes, definitively you remark make sense - it is mid-term plan, but doesn't have to be handled in this PR (I'm planning to send several focused PRs like this one in the coming days) |
|
Thnx @caitp ! |
| * as one can inject `$httpUrlParams` into a test and serialize URL params as {@link ng.$http $http} service. | ||
| */ | ||
| this.$get = function() { | ||
| var HttpUrlParams = {}; |
There was a problem hiding this comment.
nit: make the object literal constant
|
things that I immediately don't like about this:
|
|
I guess that "jQuery" strategy you mean their Regarding support for multiple query string serialization techniques in a single app - it is true, this proposal doesn't address this issue (but also doesn't change anything as compared to the today's situation). Could iterate on this and your proposal to work out something that covers most of the problem people face? Or decide which problem(s) are more important (for now it seems like there is a tension between making testing easier vs. having multiple strategies in one app). I will think of something that tries to cover both testing and multiple strategies in one app... |
|
I'm going to close this one for now and try to implement @caitp suggestion. Will open a new PR when ready. |
|
I've had some ideas, but most likely the simplest / most performant approach would be selecting which method to use in the http config object (using either a boolean flag to flip between "traditional" mode and not, or using a closure or a string to lookup a pre-registered serializer). The boolean flag is the least flexible but by far the simplest and fastest. A slower approach would be having a good "default", and being able to select an alternative strategy based on the domain name of the host you're sending the request to. Yet another one (which is probably not tenable in 1.x, but could be possible in 2.x) is just subclassing Http and setting a new default in the child class. But yeah, I tend to prefer the boolean switch |
|
@caitp thnx for the additional input. Personally I'm leaning towards string flag as I think a boolean one - although the simplest, is probably too restrictive. But yeh, we need to have some default + ability to select an alternative in the config object. |
This is the first step of the changes planned for $http's params serialization. For now a simple refactoring that moves serialization logic to a dedicated service. Even if simple, this first moves already solves some long-opened issues:
Closes #7429
Closes #9224