Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace moment.js with date-fns #973

Open
PeterBreen opened this issue May 19, 2023 · 7 comments · May be fixed by #1035
Open

Replace moment.js with date-fns #973

PeterBreen opened this issue May 19, 2023 · 7 comments · May be fixed by #1035
Assignees
Labels
Front End Primarily front-end (react/javascript) javascript Pull requests that update Javascript code

Comments

@PeterBreen
Copy link
Collaborator

PeterBreen commented May 19, 2023

Short version: load date-fns directly, replace functions which use moment.js with date-fns, remove moment from our package.json, double check that all existing datetime functions still work as expected.

Longer version:

Moment.js is quite large (the two biggest pieces of our bundle, per this analysis of package.json, are moment) and the maintainers themselves suggest moving to an alternative.

We use moment in quite a few places and any replacement must be able to handle all of them. The reason I specify date-fns is it's already a dependency of react-datepicker, which we also use and want to keep at this time. Unless there's a critical reason not to use it, loading one datetime library is better than two.

To sum up, in order for this issue to be resolved,

  • add date-fns to our package.json (react-datepicker uses 2.30.0 - let's load this only once, so use the same version)
  • update all our datetime functions which use moment to use date-fns or vanilla js
  • remove moment, moment-timezone, and react-moment from package.json
  • enjoy new, smaller bundle size

resources

These may help with the process:

@PeterBreen PeterBreen added javascript Pull requests that update Javascript code Front End Primarily front-end (react/javascript) labels May 19, 2023
@PeterBreen
Copy link
Collaborator Author

PeterBreen commented May 22, 2023

When tackling this issue, I'd suggest extending the use of our datetime utility component, seen here: https://github.com/DemocracyLab/CivicTechExchange/blob/master/common/components/utils/datetime.js - as you can see it already has a few functions, but there's more we could put in here.

If we can collect all our datetime needs and create reusable functions here and use them in the actual page components, future upgrades or changes to our datetime library will be editing one file in one place, not a dozen files in a dozen places.

@BQingFan
Copy link

BQingFan commented Jun 1, 2023

Could I try to solve this issue? I just finished environment set up.

@PeterBreen
Copy link
Collaborator Author

Could I try to solve this issue? I just finished environment set up.

Absolutely. I'll assign you, feel free to reach out if you're having trouble. There's a lot of components we use moment for, but hopefully it'll make sense how they operate.

@caveman0612
Copy link
Collaborator

I was wondering if I could try to work on this issue?

@PeterBreen
Copy link
Collaborator Author

I was wondering if I could try to work on this issue?

Sure, please do. I checked with Marlon and we think it's open for the taking so it's all yours.

@PeterBreen PeterBreen assigned caveman0612 and unassigned BQingFan Aug 16, 2023
@caveman0612 caveman0612 linked a pull request Aug 31, 2023 that will close this issue
@Kive1ru
Copy link

Kive1ru commented Sep 14, 2023

Is this issue solved? Can I try to work on this issue if not solved?

@PeterBreen
Copy link
Collaborator Author

PR #1035 solves this issue, pending a couple bugfixes. I believe Kyle's still working on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Front End Primarily front-end (react/javascript) javascript Pull requests that update Javascript code
Projects
Status: Ready for testing.
Development

Successfully merging a pull request may close this issue.

4 participants