danielmoncada/date-time-picker

Angular 17

Closed this issue ยท 19 comments

It would be great to add support for Angular 17. I've already tested things by overriding the peer dependencies, and everything seems to work just fine, so I think we just need to update the peer dependencies and publish.

I would be happy to create a pull request, but I'm wondering... instead of just adding || ^17.0.0, can we specify >= 13.0.3?

Also, @danielmoncada, if you're looking for someone to help out on this repo, I'd be happy to take up a contributor or even maintainer role.

polhek commented

@neodescis i was trying to make the library work locally to test if it works. But after "npm install" and "ng serve" in the root directory, I get quite a lot of errors. That is without any changes, just forking the repo and trying it out. Can you point me in the right direction, please.

Node version: v16.20.2

image

@neodescis ah, didn't realize a new ng version was out. yeah, I can try and update it tonight, thanks for testing.

@neodescis i was trying to make the library work locally to test if it works. But after "npm install" and "ng serve" in the root directory, I get quite a lot of errors. That is without any changes, just forking the repo and trying it out. Can you point me in the right direction, please.

Node version: v16.20.2

image

When I say I got things working with Angular 17, I mean I just added an entry to "overrides" in my project's package.json to get past the peer dependency error with installing and running Angular 17 along side this package. I did not do what you attempted.

I will also add that if we update this package to be built with Angular 17, it will break compatibility with previous Angular versions, as built libraries are not backward compatible with earlier versions of Angular. Rather, my point was that updating the peer dependencies is good enough.

polhek commented

@neodescis i was trying to make the library work locally to test if it works. But after "npm install" and "ng serve" in the root directory, I get quite a lot of errors. That is without any changes, just forking the repo and trying it out. Can you point me in the right direction, please.

Node version: v16.20.2

image

When I say I got things working with Angular 17, I mean I just added an entry to "overrides" in my package.json to get past the peer dependency error with installing and running Angular 17 along side this package. I did not do what you attempted.

Also, if we update this package to be built with Angular 17, it will break compatibility with previous Angular versions, as built libraries are not backward compatible with earlier versions of Angular. Rather, my point was that updating the peer dependencies is good enough.

I forked the repo, then 'npm install' and then ng serve or one of the build the library, but none of it worked. Resulting in the errors in the screenshot.

I forked the repo, then 'npm install' and then ng serve or one of the build the library, but none of it worked. Resulting in the errors in the screenshot.

Yes, I understand. I ran npm install, npm run build_lib and npm start and things are working just fine for me. I used node 16.19.0 because I already had it installed via nvm.

Hi Everyone
@danielmoncada
Please check the PR
Added Angular 17 support
#189

@pavlikxor, as I mentioned above, if we switch to actually building with Angular 17, it is no longer backward compatible with earlier versions of Angular. Is that what we want to do?

Also, this is not the only repository that needs updating. There are separate repos for moment and Day.js adapters.

@pavlikxor, as I mentioned above, if we switch to actually building with Angular 17, it is no longer backward compatible with earlier versions of Angular. Is that what we want to do?

Correct,
I see no reasons keeping it backward compatible. For apps developed using older Angular versions (<= 16) appropriate library version should be used

Also, this is not the only repository that needs updating. There are separate repos for moment and Day.js adapters.

Correct,
I'll take care of adapter-libraries update as well. But firstly main library needs to be updated, as adapter-libraries have devDependency on @danielmoncada/angular-datetime-picker

NetWin commented

What's the current state here? Is there an ETA for the release? ๐Ÿ˜„
@danielmoncada @neodescis @pavlikxor

I believe there are two things outstanding in the code review that @danielmoncada had thumbs-up'd:

  1. The change to disableClose in dialog-ref.class.ts
  2. Changing the peer dependencies to be >= 17.0.0

I have some time now.. pulling the PR now

alright; latest has been published to npm. still need to update the adapters.

alright; latest has been published to npm. still need to update the adapters.

I guess we're not going with >=17.0.0 in the peer dependencies then? Bummer.

Hi everyone cc @danielmoncada Moment adapter, please review - danielmoncada/date-time-picker-moment-adapter#15 Dayjs adapter - danielmoncada/date-time-picker-dayjs-adapter#15

I added a few comments to the dayjs adapter. The moment adapter seems like it should have similar tsconfig changes to what you did for dayjs though, no?

closing, both adapters and the picker have now been updated (and published to npm).

thank you for the help / work @pavlikxor and @neodescis