Skip to content

Conversation

@hpawe01
Copy link
Contributor

@hpawe01 hpawe01 commented Mar 12, 2018

This pull request tries to fix #95 and to finish parts of the open pull request #96. I used some ideas of this pull request, so thanks @agustinvinao. This pull request does not change public or protected method signatures except the constructor of the DatastoreService (it now needs a HttpClient instead of Http), so it should be easy to integrate for the users of this library.

To replace HttpModule with HttpClientModule we have to do multiple things:

  • Adjust module file
  • Get rid of all HttpModule constructs (like RequestOptions or Response) and use HttpClientModule constructs (like HttpHeaders or HttpResponse) in json-api-datastore.service.ts and json-api.model.ts
  • Major rewrite of json-api.datastore.service.spec.ts to use the new testing concepts of HttpClientModule (https://angular.io/guide/http#testing-http-requests)
  • Update README

Other things should be considered in following pull requests:

  • Remove the deprecated query method (and its test cases)
  • Switch to Angular 5 and repair tests with null response

@coveralls
Copy link

Coverage Status

Coverage increased (+2.02%) to 90.443% when pulling d13964e on hpawe01:fix-95 into ca53dd5 on ghidoz:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+2.02%) to 90.443% when pulling d13964e on hpawe01:fix-95 into ca53dd5 on ghidoz:master.

aenyway added a commit to compuccino/angular2-jsonapi that referenced this pull request Mar 14, 2018
aenyway added a commit to compuccino/angular2-jsonapi that referenced this pull request Mar 14, 2018
@agustinvinao
Copy link

Its possible to review and merge this PL? thanks

@cguilhermef
Copy link

deleteRecord<T extends JsonApiModel>(
    modelType: ModelType<T>,
    id: string,
    headers?: Headers,
    customUrl?: string
  ): Observable<Response> { ... }

In json-api-datastore.service.ts, still using Response as type of return. - I tried to use the forked repository, but I found a lot of workaround to use it.

@hpawe01
Copy link
Contributor Author

hpawe01 commented Apr 8, 2018

@cguilhermef: I didn't want to change the public API, so I kept Response as type of return. But as I see it, this shouldn't be a problem because Response is an abstract typescript interface and not angular specific (at least Visual Studio Code is telling me this).

Can you specify what workaround you had to use to make this work?

@hpawe01 hpawe01 mentioned this pull request Apr 8, 2018
@gustavomcastro
Copy link

Good Solutions. Need Merged Fast. Because Angular 5+ use HttpClientModule and the performance and controls is best than Http

@cguilhermef
Copy link

Sorry @hpawe01, I expressed wrong myself. I tried to use the #161 branch, from forked source, but there deleteRecord still using Response as type of return. Angular CLI and Webstorm are showing incompatible type issues.

@safo6m safo6m merged commit bf3e152 into ghidoz:master Apr 11, 2018
@gustavomcastro
Copy link

When will a new version be generated with this feature?

@safo6m
Copy link
Collaborator

safo6m commented Apr 11, 2018

@gustavomcastro version 5.0.0 has been already published

@cguilhermef
Copy link

Nice work!

We still have some deprecated dependencies, like Response and Headers, but the first step was done!

@safo6m
Copy link
Collaborator

safo6m commented Apr 11, 2018

@hpawe01 thank you for your contribution.

@gustavomcastro
Copy link

Thank you. Nice Work!

@hpawe01 hpawe01 deleted the fix-95 branch April 12, 2018 09:04
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.

Move away from Http and use Httpclient

6 participants