feat(angular): Upgrade to Angular 5 #38
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #38 +/- ##
===========================================
+ Coverage 99.01% 99.13% +0.11%
===========================================
Files 14 14
Lines 408 460 +52
Branches 65 79 +14
===========================================
+ Hits 404 456 +52
Misses 4 4
Continue to review full report at Codecov.
|
| @@ -22,7 +22,7 @@ export class NotifierTimerService { | |||
| /** | |||
| * Timeout ID, used for clearing the timeout later on | |||
| */ | |||
| private timerId: number; | |||
| private timer: NodeJS.Timer; | |||
There was a problem hiding this comment.
I think this is an issue introduced by the NodeJS typings, the timer ID here should be of type number (as before), not NodeJS.Timer. Perhaps we should cast this into a number, if necessary?
There was a problem hiding this comment.
You are right, TypeScript was complaining about this, I didn't check the signature of the method setTimeout which is wrong microsoft/TypeScript#842 I'll use window.setTimeout instead.
package.json
Outdated
| "@angular/platform-browser-dynamic": "4.1.x", | ||
| "@angular/platform-browser": "4.1.x", | ||
| "@types/jest": "20.0.x", | ||
| "@angular/common": "^5.0.3", |
There was a problem hiding this comment.
I usually prefer 5.0.x instead of ^5.0.3 as it is easier to understand for developers
package.json
Outdated
| @@ -48,64 +48,60 @@ | |||
| "ci:coverage": "codecov -f coverage/coverage-final.json", | |||
| "ci:test": "jest --config tools/jest/jest.config.json --runInBand --no-cache" | |||
| }, | |||
| "peerDependencies": { | |||
There was a problem hiding this comment.
Please do not remove the peerDependencies, instead update to >= 5.0.0 < 6.0.0
|
@dominique-mueller I've addressed your comments :) |
|
Thanks again @DavidRouyer for your contribution! I will do some further build-related updates and release v3 of angular-notifier in the next few days. |
BREAKING CHANGE: The upgrade to Angular 5 breaks compatibility with Angular 4.