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

[deps] Replace the use of moment.js with actively maintained library #1736

Closed
3 tasks
yurishkuro opened this issue Aug 27, 2023 · 2 comments · Fixed by #1738
Closed
3 tasks

[deps] Replace the use of moment.js with actively maintained library #1736

yurishkuro opened this issue Aug 27, 2023 · 2 comments · Fixed by #1738

Comments

@yurishkuro
Copy link
Member

yurishkuro commented Aug 27, 2023

Moment.js is "a legacy project in maintenance mode", according to its official website https://momentjs.com/docs/#/-project-status/.

  • do an inventory of features / APIs we are using from moment.js
  • do analysis which replacement is most suitable (the fewer dependencies the better)
  • make the corresponding changes. It's probably worth consolidating all usage of any new 3rd party library into utils/date.tsx
$ grx "from 'moment'" packages/jaeger-ui/src
packages/jaeger-ui/src/utils/date.tsx:15:import moment, { unitOfTime } from 'moment';
packages/jaeger-ui/src/components/common/RelativeDate.tsx:15:import moment from 'moment';
packages/jaeger-ui/src/components/TracePage/TraceSpanView/index.tsx:17:import moment from 'moment';
packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ScatterPlot.jsx:16:import moment from 'moment';
packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItem.tsx:21:import moment from 'moment';
packages/jaeger-ui/src/components/SearchTracePage/SearchForm.jsx:20:import moment from 'moment';
packages/jaeger-ui/src/components/SearchTracePage/SearchForm.test.js:20:import moment from 'moment';
packages/jaeger-ui/src/api/jaeger.js:16:import moment from 'moment';

$ grx "from 'moment'" packages/jaeger-ui/src packages/plexus/src
packages/jaeger-ui/src/utils/date.tsx:15:import moment, { unitOfTime } from 'moment';
packages/jaeger-ui/src/components/common/RelativeDate.tsx:15:import moment from 'moment';
packages/jaeger-ui/src/components/TracePage/TraceSpanView/index.tsx:17:import moment from 'moment';
packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ScatterPlot.jsx:16:import moment from 'moment';
packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItem.tsx:21:import moment from 'moment';
packages/jaeger-ui/src/components/SearchTracePage/SearchForm.jsx:20:import moment from 'moment';
packages/jaeger-ui/src/components/SearchTracePage/SearchForm.test.js:20:import moment from 'moment';
packages/jaeger-ui/src/api/jaeger.js:16:import moment from 'moment';
@prathamesh-mutkure
Copy link
Contributor

prathamesh-mutkure commented Aug 27, 2023

I did some research around this and two of the most popular date libraries are date-fns and dayjs

date-fns

  • Actively maintained (> 32k stars)
  • Rich functionality

dayjs

  • Actively maintained too (44k stars)
  • Similar API to moment.js (almost identical)
  • Small bundle size (2kb)

I feel dayjs will be a very good choice since its API is almost the same as moment.js and it is much smaller in size compared to date-fns and moment.js

What do you say? I'm willing to work on this

@kumarankit999
Copy link

Can I work on this, if no one is working on this issue

yurishkuro pushed a commit that referenced this issue Sep 22, 2023
## Which problem is this PR solving?
- Resolves #1736 

## Description of the changes
- I've removed `moment.js` and replaced it with `dayjs` packages
- I've fixed all the failing tests
- Tested all dates and time formatting

## How was this change tested?
- All the test cases are passing
- All the pages that got impacted were tested visually by me
- Including the `Monitor` tab, `Span` and `Trace` view, `Search` page,
all the charts, etc.

The UI was checked between `jaeger-all-in-one` and changes of this
commit

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [x] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Prathamesh Mutkure <pmutkure009@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants