search

CaliStyle / ng-intercom

Angular 7+ Wrapper for Intercom.com
MIT License
54 stars 46 forks source link

Angular 6 support #47

Closed Koslun closed 6 years ago

Koslun commented 6 years ago

Expected/desired Support Angular 6, TypeScript 2.7 and RxJS 6 without errors

Actual Getting this error when starting a newly created angular-cli project with this library added to it:

TypeError: Cannot read property 'events' of null

Here is the project and branch reproducing the above error. Created the project using angular-cli defaults with yarn as the package manager while also adding rxjs-compat, which should smooth over being able to use RxJS 5.x.

Solution A large part of the work might be possible by simply using ng update with @angular/core and rxjs as outlined in the Angular 6 release post.

Given that I think upgrading to support Angular 6 will invariably break everyone using below angular 5 and below I think it would make sense to couple this update with a major version bump. Meaning bumping the version number to 2.x.

scott-wyatt commented 6 years ago

I’d be very open to a PR, otherwise I’ll have to wait till this hits my deployment schedule for angular 6.

Koslun commented 6 years ago

@scott-wyatt ok, will see if I can get some time later this week to try out the automated tooling I mentioned to provide a PR.

Looking at the git commit log I presume it'll be fine to follow the AngularJS git commit message conventions or similar Conventional Commits

Koslun commented 6 years ago

Have a WIP solution in PR #49. Missing some things that I don't think matter for consuming apps.

Noticed however that Angular itself maybe still worked as the above error was due to not including a router even though I used the router config of this library.