flauc / angular2-notifications

A light and easy to use notifications library for Angular.
https://stackblitz.com/edit/angular2-notifications-example
MIT License
746 stars 163 forks source link

Angular 9 compatibility #355

Closed Guerric-P closed 4 years ago

Guerric-P commented 4 years ago

At the moment, the library does no integrate to Angular 9 with the following error message :

SimpleNotificationsModule.forRoot returns a ModuleWithProviders type without a generic type argument. Please add a generic type argument to the ModuleWithProviders type. If this occurrence is in library code you don't control, please contact the library authors.

romontei commented 4 years ago

Still waiting from #348

Guerric-P commented 4 years ago

If interested, I forked the repo here https://github.com/youkouleley/angular2-notifications, migrated to Angular 9, and moved from gulp to ng-cli. I also published this fork as https://www.npmjs.com/package/angular9-notifications-gpu (Due to a manipulation error from me, the name angular9-notifications is locked for 24h).

Tell me if interested in a pull request (I only tested it in an Angular 9 app though)

flauc commented 4 years ago

Hi @youkouleley if you would be able to send a PR that would be wonderful yes.

csentis commented 4 years ago

Tell me if interested in a pull request (I only tested it in an Angular 9 app though)

@youkouleley I really appreciate this this as a user of angular2-notifications, too!

According to @flauc PR is already in #356 - thanks!

flauc commented 4 years ago

Hi, @csentis. There is a PR in place already

https://github.com/flauc/angular2-notifications/pull/356

but I was yet unable to go over it.

fabioformosa commented 4 years ago

Upgrading to angular 9, I get:

Package "angular2-notifications" has an incompatible peer dependency to "zone.js" (requires "^0.9.1", would install "0.10.2")

RockNHawk commented 4 years ago

I have the same issue

Guerric-P commented 4 years ago

@fabioformosa is that a warning or does it prevent you from compiling?

fabioformosa commented 4 years ago

It's a warning that prevents the angular upgrading, launched from angular cli (ng update @angular/core @angular/cli).

Anyway, I've passed over using the param --force and the upgrade went OK, but now the actual version of zone.js is 0.10.2, not the 0.9.1 expected by angular2-notifications. I'm testing if the behaviour of my app is OK.

Tell me if your lib has any incompatibility with zone.js 0.10.2

Guerric-P commented 4 years ago

I'm just a collaborator, not the owner of the library, the owner is @flauc and I think we should discuss about the strategy we want to adopt, between:

What do you think @flauc ?

flauc commented 4 years ago

@Guerric-P thank you for picking this up so quickly. I think evergreen is the way to do. If someone needs an older version they can always pull an older version.

Guerric-P commented 4 years ago

Ok I'll start working on Angular 9 compatibility, do you agree if I sync the version number of the library with Angular's one?

flauc commented 4 years ago

Totaly, that is a great idea 👍

Guerric-P commented 4 years ago

Thanks for your quick answer by the way 🚀

Twista commented 4 years ago

Hey @Guerric-P - thanks for taking care of that issue, really appreciated.

If you need any help with that, let me know - will be happy to help :)

Guerric-P commented 4 years ago

Hi @Twista help is always welcome, I've not started to work on it yet, if you could provide a PR that provides Angular 9 compatibility without the zone.js warnings, I'll check it out. In accordance with what we just said, it doesn't need to be backwards compatible, so you can build it with the Ivy engine, I guess that will save some build time for the users.

Guerric-P commented 4 years ago

Published Angular 9 compatible version as v9.0.0 (with no warnings on peerDependencies)