Repository navigation
feat(http): accept string for method in RequestOptions class and interface #2840
Description
Currently, the method must be of the ResponseMethods TypeScript enum, which is really an integer. So to create a request in TypeScript, one would use the enum to describe the method instead of passing in the method string:
var options = new RequestOptions({
url: '/foo.json',
method: RequestMethods.POST
});
var request = new Request(options);This adds some convenience for TypeScript and Dart users to get some IDE/compiler help to make sure the method is a valid value. But plain old ES5 users probably don't want to pass 0 as the method value. Instead, RequestOptions should have the method property be a union type of RequestMethods | string, and should validate the provided string & throw if not a recognized method.
I'm not sure Request class needs to accept method as a string though. Ideally, we could reduce instances of type ambiguity to public APIs.
Activity
mgechev commentedon Sep 21, 2015
Since
Requestis exposed by the public API, it may create confusion if the users can pass both strings andRequestMethodsto thehttpmethods when using object literals and onlyRequestMethodswhen usingRequest.@jeffbcross I added support of passing HTTP method names as strings to
RequestOptionshere. Do you think it looks okay and ready for PR?jeffbcross commentedon Sep 23, 2015
@mgechev I left a couple of comments on the commit, but yeah, looks good! Please PR! I will assign this to myself just to show that someone's working on it (github issues can only be assigned to those with commit access).
jeffbcross commentedon Sep 23, 2015
I agree, please also add the
normalizeMethodtoRequest.Requestactually is a useful public API, especially in tests.mgechev commentedon Sep 23, 2015
I took another look at
Request. It acceptsRequestOptions, however, inside theRequestOptions's constructor we already normalize the method name. Which means the following is valid:However, in case the
Requestconstructor function is used with ES5 it won't work. I added one extra call ofnormalizeMethodNameinsideRequest's constructor.Here is the PR.
jeffbcross commentedon Sep 23, 2015
@mgechev eventually
Requestshould acceptRequestOptionsArgsinterface, but that's not in scope for this PR. I think I removed that when I was makingHttpwork for dart some time ago.mgechev commentedon Sep 24, 2015
@jeffbcross I updated the PR. If declaring that the argument of the constructor of
Requestshould be of typeRequestOptionsArgsis out of the scope of this PR I can open another one?jeffbcross commentedon Oct 8, 2015
Closed via 34518f0
angular-automatic-lock-bot commentedon Sep 7, 2019
This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.
Read more about our automatic conversation locking policy.
This action has been performed automatically by a bot.