search

moribvndvs / ng2-idle

Responding to idle users in Angular (not AngularJS) applications.
https://moribvndvs.github.io/ng2-idle
Apache License 2.0
315 stars 128 forks source link

chore: update library to use angular 14 #169

Closed HarelM closed 1 year ago

HarelM commented 1 year ago

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x")

[ ] Bugfix
[ ] Feature
[x] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[x] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior? (You can also link to an open issue here) There's a warning when using this library when ivy is enabled (by default from angular 13)

What is the new behavior? There should not be a warning

Does this PR introduce a breaking change? (check one with "x")

[x] Yes 
[ ] No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ... I think the movement to Ivy creates breaking changes, but I'm not 100% sure

Other information: I've also migrated from tslint to eslint. package-lock.json now uses version 2 which is the default for node 16 (npm 7 I think). There's a need to update rxjs I think, but it's not part of this PR, I can add it if requested. Most of the migration was done using automation tools, so I didn't do much here... :-)

HarelM commented 1 year ago

Do I need to update the CI to use node 16 as part of this PR? I think angular 14 might not support node 12 anymore...

moribvndvs commented 1 year ago

Yeah, LTS is currently 16. If you don’t mind trying to upgrade to 16, I’d appreciate it.

un1c0rnr1d3r commented 1 year ago

Do I need to update the CI to use node 16 as part of this PR? I think angular 14 might not support node 12 anymore...

Yes, please update the GitHub CI Workflow to target Node versions 14 and 16 (instead of 12 and 14).

HarelM commented 1 year ago

I've updated the github ci stuff, I hope it will be picked up and not use the master branch instead... Otherwise I'll probably need to do it as part of a different PR... I've also removed the hard coded npm version and allowed the CI to use the one that comes with the relevant node.

HarelM commented 1 year ago

The new CI commands are being picked up, which is good :-) I hope fortawesome is the last one that uses outdated angular as dependencies, otherwise I'll add --force to npm ci to make sure everything is installed...

coveralls commented 1 year ago

Coverage Status

Coverage remained the same at 100.0% when pulling d775aa08aa0f9a490d101736b5a71c59b42cba5c on IsraelHikingMap:master into 2ab2d9aa7cfc4e390bce9720ac8c9dcfc5f41d8f on moribvndvs:master.

HarelM commented 1 year ago

Thanks for the super quick response! Please let me know when a new version is available in npm and I'll make sure the ngcc warning is gone.

moribvndvs commented 1 year ago

change looks good, my stupid release script isn’t working right. I’ll give it another shot tomorrow.

Nic-Grunklee commented 1 year ago

Is there any update on releasing this to npm? Thanks

kufuntu commented 1 year ago

@moribvndvs can you please release the version to npm? :)

HarelM commented 1 year ago

@moribvndvs any updates on this? Do you need a help looking at the publish script (wherever that is)? I'll be happy to update this package to angular 15 as well if needed. This is currently the last package I have that has this ivy warning :-)

HarelM commented 1 year ago

I've opened the following issue: https://github.com/moribvndvs/ng2-idle/issues/183 Might be something wrong with the publish CI pipeline, IDK...

gburgalum01 commented 1 year ago

Are there any updates on this change? I'd love to use this library, but unfortunately, I am unable to do so until the changes made by @HarelM are pushed. Thank you!