airgradienthq / map

Other
8 stars 2 forks source link

Replace moment.js #5

Closed TomAdam closed 6 months ago

TomAdam commented 11 months ago

The final build size of the app is relatively large. One thing that could improve it is to replace moment.js as this is a large package. The development team has put it in maintenance mode (see https://momentjs.com/docs/#/-project-status/) and a maintainer has gone on to create Luxon (see https://moment.github.io/luxon/#/why). The immutable aspect of Luxon prevents accidental mutation of date objects, making it significantly easier to work with. Would you consider a PR migrating to Luxon?

TomAdam commented 11 months ago

I've looked a bit further into this. Moment is also required as a dependency of chart.js. The angular module ngx-chartjs that is in use has been abandoned by the author and only supports an older (v2) version of chart.js. Newer versions of chart.js (v4) allow you to choose the underlying date library, and also support tree shaking which will both reduce the vendor bundle size. ng2-charts is an up to date replacement for ngx-chartjs, despite the "ng2" in the name. My proposal is to also replace the chart module, and fully switch to luxon.

airgradienthq commented 11 months ago

@TomAdam I think its a good idea to switch to a newer version of chart.js and use the underlying date library. Would you be able to do work on that?

yegormk commented 6 months ago

After investigation, we've discovered that moment.js has already been replaced with Luxon. Therefore, I'm closing this issue as it's no longer a problem.