Skip to content

Remove @angular/http from the dependecies #1842

Closed

Description

@VladimirAmiorkov

Environment

  • NativeScript-Angular: 8.0.0
  • Angular: 8.0.0

Describe the bug
The current 8.0.0 version of the plugin has a dependency for the @angular/http which is deprecated and replaced by the @angular/common/http package.

Activity

  1. charsleysa commented on Jul 18, 2019

    @charsleysa

    The entire NSHttp module should be removed since Angular 8 no longer even has an Http module (only HttpClient).

    IMO nativescript-angular@8.0.0-8.0.2 should be unpublished and 8.0.3 published with this code removed as it should have never been released with an unsupported beta library as a dependency.

  2. VladimirAmiorkov commented on Jul 18, 2019

    @VladimirAmiorkov
    ContributorAuthor

    Hi @charsleysa ,

    Have you experienced any issues related to the use of the @angular/http in the nativescript-angular@8.0.0-8.0.2 ? In all of our tests the latest "@angular/http": "8.0.0-beta.10" version is working correctly no matter that it is a beta version.

    We are planning to remove this as soon as possible but if you feel like contributing to the project we welcome your PR and contribution. Thank you.

  3. charsleysa commented on Jul 18, 2019

    @charsleysa

    Hi @VladimirAmiorkov

    We are seeing compilation errors wherever we include the nativescript-angular library at the root (so import { ... } from 'nativescript-angular'.

    ERROR in nativescript-angular/http/http.module.ts(19,20): Error during template compile of 'NativeScriptHttpModule'
      Could not resolve @angular/http relative to [object Object]..
    node_modules/nativescript-angular/http/ns-http.d.ts(1,96): error TS2307: Cannot find module '@angular/http'.
    

    This seems to be due to the NSHttp module being reference by the root index.ts file.

    I would be happy to contribute a PR, though this is more of a release issue as this code should have been removed before nativescript-angular@8.0.0 was published so removing it now would be deemed a breaking change triggering a major version bump (as per semver).

    My proposed solution effectively rolls in the breaking change into the 8.0 release by unpublishing all prior 8.0.x releases and updating the changelog to note why.

    Thoughts?

  4. VladimirAmiorkov commented on Jul 18, 2019

    @VladimirAmiorkov
    ContributorAuthor

    Does your project have a dependency of @angular/http ? Unpublishing a version of nativescript-angular is not an option because we do not see any issues of using the latest version of @angular/http which was released specifically for Angular 8 during its beta. The reason for not removing it before our 8.0.0 release was that there was no mentioning from the Angular team that this package will not leave the beta stage and that decision was made behind closed doors and we were only made aware at the release day of Angular 8.0.0 at which point it was too late to remove it since we are committed to delivering day one support for major releases of Angular.

    I am asking you if you find any real issues with having @angular/http as a dependency in you project because in all of our tests there is not issues either at runtime and at compilation time of the {N} + Angular project that uses it.

  5. charsleysa commented on Jul 18, 2019

    @charsleysa

    Hi @VladimirAmiorkov

    I can confirm that adding @angular/http@8.0.0-beta.10 to the dependencies allows the compilation to succeed and the app runs without any apparent issues. One thing to note is that the library seems to be included in the vendor bundle, which is less than desirable, but I think its impact on bundle size during a production build mostly reduced thanks to the angular/webpack optimizers.

    Thanks for the help.

  6. VladimirAmiorkov commented on Jul 18, 2019

    @VladimirAmiorkov
    ContributorAuthor

    Yes that is correct, webpack will tree share that dep from the bundle and it will not affect the size of the app. Also in NativeScript 6.0.0 webpack is executed in all builds by default so the tree sharking is also present for local development.

    You are welcome. We are in the process of addressing this and migrating from this deprecated package or deprecating it so we can remove it safely in future version.

  7. charsleysa commented on Aug 30, 2019

    @charsleysa

    @VladimirAmiorkov this can be closed now as it was solved by #1942

  8. VladimirAmiorkov commented on Aug 30, 2019

    @VladimirAmiorkov
    ContributorAuthor

    Resolved via #1942

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions