feat: add luxon date adapter #1709
Conversation
|
Guess it sure doesn't work, yet. But what does that line in the old moment dateAdapter even do? Just copied it - but actually don't get it. |
There was a problem hiding this comment.
This is really great work, thank you!
Can you also add an example, you can copy from the moment one:
https://github.com/mattlewis92/angular-calendar/tree/main/projects/demos/app/demo-modules/moment
| @@ -113,7 +115,7 @@ function addModuleToImports(options: Schema): Rule { | |||
| const moduleName = `CalendarModule.forRoot({ provide: DateAdapter, useFactory: ${ | |||
| options.dateAdapter === 'moment' | |||
| ? 'momentAdapterFactory' | |||
| : 'adapterFactory' | |||
| : (options.dateAdapter === 'luxon' ? 'luxonAdapterFactory' : 'adapterFactory') | |||
There was a problem hiding this comment.
Let's bake this into the dateAdapters object and avoid the branching logic e.g.
const dateAdapters: Record<Schema['dateAdapter'], {version: string, adapterName: string}> = {
moment: {
version: momentVersion,
adapterName: 'momentAdapterFactory'
},
'date-fns': {
version: dateFnsVersion,
adapterName: 'adapterFactory'
},
luxon: {
version: luxonVersion,
adapterName: 'luxonAdapterFactory'
},
};
Then here we can just pass dateAdapters[options.dateAdapter].adapterName;
| @@ -156,6 +158,26 @@ function addModuleToImports(options: Schema): Rule { | |||
| ); | |||
| } | |||
|
|
|||
| if (options.dateAdapter === 'luxon') { | |||
There was a problem hiding this comment.
Let's add some tests for this based on the moment ones:
| @@ -0,0 +1,55 @@ | |||
| import { adapterFactory as baseAdapterFactory } from 'calendar-utils/date-adapters/luxon'; | |||
There was a problem hiding this comment.
To make this work you'll need to add a new date adapter here:
https://github.com/mattlewis92/calendar-utils/tree/main/src/date-adapters
and add it to the tests here:
https://github.com/mattlewis92/calendar-utils/blob/2f293a291f8fbc4da7b4022a4f562d043772755f/test/calendar-utils-utc.spec.ts#L53-L67
I can't remember the exact reasoning, but I think at the time the Intl API was really buggy in most browsers (which was why angular moved away from it), so I added moment as a "stable" alternative. I think nowadays angular are looking to move back to the Intl api so I guess it got better and having the other date formatters makes less sense. |
|
@mattlewis92 any idea on when this can get added? I just started working on a new project and this feature would make things a lot easier |
Feel free to send another PR based off this one with the review comments addressed :) It's not something I have the time to work on myself |
|
Is there anything that's still blocking this PR from being merged? |
Yep, still waiting on the review comments to be addressed: #1709 (review) |
I'm switching my project from moment to luxon so I needed luxon support in your (great!) calendar control.
I'm not very familiar with package building but hope I've adopted all the necessary files for a correct luxon support. By the way, since your
CalendarNativeDateFormatteruses theIntlobject (and I guess for that reason there is noCalendarDateFnsDateFormatter), why is there even aCalendarMomentDateFormatter? Any reason to use it at all?Thank you for checking, probably some fixing ;) & deploying! :)