Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework request to mostly match ng2 Http API #5

Merged
merged 2 commits into from
Dec 23, 2015

Conversation

escardin
Copy link
Contributor

This is almost exactly the work I did when I encountered the problem with not being able to add headers.

I didn't use the authHttp api when I wrote mine.

I broke some of the api for the lesser used stuff (in the Http Api, delete and head have no body).

Considering that I already broke the api, I would suggest going a step further and replacing request with _request, and getting rid of options (not part of the http api).

@escardin
Copy link
Contributor Author

fixes #2

@damienpontifex
Copy link

👍 need this

@troyanskiy
Copy link

I should check pull requests before implementing mine the same fixes.
Definitely need that.
Please merge.

chenkie added a commit that referenced this pull request Dec 23, 2015
Rework request to mostly match ng2 Http API
@chenkie chenkie merged commit 8539932 into auth0:master Dec 23, 2015
@escardin
Copy link
Contributor Author

If you're going to do a breaking change release, I still think the api should be made to fully match http.

@chenkie
Copy link
Contributor

chenkie commented Dec 23, 2015

Can you elaborate a bit more on getting rid of options here just so I'm clear?

Considering that I already broke the api, I would suggest going a step further and replacing request with _request, and getting rid of options (not part of the http api).

@escardin
Copy link
Contributor Author

When I was getting my changes into angular2-jwt I reviewed the http api and it doesn't provide an options call. I was suggesting removing it because it's not in the http api.

see: https://angular.io/docs/ts/latest/api/http/Http-class.html

I don't think it's really necessary. Extending the http api is okay (desired even). The important part is that the angular2-jwt request function is not drop in compatible with the http request function.

@chenkie
Copy link
Contributor

chenkie commented Dec 23, 2015

Makes sense 👍 Thanks

@escardin escardin deleted the requestrework branch January 18, 2016 21:28
@escardin escardin restored the requestrework branch April 5, 2016 16:57
@escardin escardin deleted the requestrework branch May 9, 2016 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants