search

allianz / ng-aquila

Angular UI Component library for the Open Insurance Platform
https://allianz.github.io/ng-aquila/
Other
213 stars 38 forks source link

Get rid of moment.js dependency #55

Closed jonrich92 closed 1 year ago

jonrich92 commented 1 year ago

day.js should be enough.

Eliminate the moment.js dependency from the package.json file. Day.js is already introduced and should be the only dependency for date/time functionality.

🎯 Goals

Clean lean dependency set up.

jonrich92 commented 1 year ago

Please explain shortly, why you support moment.js and not just day.js or what your future strategy will be? Do you plan to drop moment.js at some point?

Phil147 commented 1 year ago

Hi @jonrich92

there is no urgent need to get rid of moment.js for us. The library is built with secondary entry points exactly for the reason that most third party dependencies that are only needed for specific components only need to be installed if you actually use them. So as long as you don't import from the moment date adapter you don't have to worry about it. Same goes for other peer dependencies like ibanjs, which only needs to be installed when you use the iban mask. Now currently dayjs was added to the dependencies and not peerDependencies, that was a mistake and not intended and will be fixed in the next version. Also removing the moment date adapter is a sensible change. The library is used internally by a lot of projects and we can't know how many still use the moment adapter.

The only trade off which we decided on was to add all these dependencies as peerDependencies. Unfortunately the concept of secondary entry points is a bundler concept that is not known to package managers. Historically we didn't have these possible dependencies listed as dependency or peerDependency. So we had the problem that we got issues opened because people used e.g. the datepicker with the moment adapter and then got the error that moment was missing. So the trade off was we add all these third party dependencies that might be needed by some components as peerDependencies and also with the ng add command, so that at any time the code examples you find in the documentation will work.

I hope that clears your concerns regarding moment.js

jonrich92 commented 1 year ago

@Phil147 thank you very much vor the clarification. My line of reasoning was like you have described. I came from the package manager only. Now I see that moment.js is no concern for me in that context. Agree there is no need to get rid off it immediately. Probably eventually through since it is "near" end of life. I close the ticket.