search

angular / in-memory-web-api

The code for this project has moved to the angular/angular repo. This repo is now archived.
MIT License
1.18k stars 231 forks source link

refactor: remove @angular/http #223

Closed CaerusKaru closed 5 years ago

CaerusKaru commented 5 years ago

BREAKING CHANGE:

CaerusKaru commented 5 years ago

cc @IgorMinar

IgorMinar commented 5 years ago

@wardbell FYI ^

wardbell commented 5 years ago

Looking forward to being able to review this PR this week.

FWIW, if we’re really getting rid of old Http, it’s kind of nutty to keep the abstraction that supported both. I mentioned this I think in our conversation. Much of the muddiness in this library is due to the now pointless support for both http libraries.

I’m particularly unhappy with the dance that lead to RequestCore and HeadersCore (see interfaces.ts). It made life difficult for developers to get at the body and headers in a type-ful way. I don't know if you cleaned this up in this PR.

Not saying we should hold the PR up for those reasons. Just commentary for the moment.

IgorMinar commented 5 years ago

@wardbell I'd prefer any major refactoring should be done separately from the removal of http because otherwise this PR will grow uncontrollably. Let's get this in and 0.8 out. sgty?

wardbell commented 5 years ago

@IgorMinar I'm with you on that. I too would have done this in two steps, the first as in this PR.

I just hate that this project so twisted to accommodate a feature (old Http) it no longer supports, leaving the code in a mysterious, convoluted state that reflects poorly on the author (me). I realize that this is my ego trip.

IgorMinar commented 5 years ago

@wardbell we can do that as a followup step and your contributions are welcome, but in the meantime this removal is blocking a lot of other work in angular/angular so we need to get moving on this.

I asked @brandonroberts to take over this PR and cut the 0.8 release with this change.