dspace-angular icon indicating copy to clipboard operation
dspace-angular copied to clipboard

Replace Moment.js with date-fns

Open tdonohue opened this issue 2 years ago • 3 comments

Description

In an attempt to start to shrink the size of our main.js, this PR replaces moment.js with date-fns (which is much smaller in size)

  • Why? Because Moment.js doesn't work with "tree-shaking" and it can expand the size of your bundle. See https://github.com/you-dont-need/You-Dont-Need-Momentjs/blob/master/README.md

This PR decreases the GZipped size of main.js by ~66KB. This measurement was determined by running the following on main vs this PR:

  • yarn build:stats
  • webpack-bundle-analyzer .\dist\browser\stats.json

Instructions for Reviewers

  • Review code
  • Run some basic tests with date-based functionality to ensure no changes in behavior.
    • For example: Saving date in submission, filtering by date in search results, etc.

List of changes in this PR:

  • Created new specs for date.util.ts to verify the current behavior using momentjs is validated. These tests pass with momentjs.
  • Refactored code in date.util.ts to use date-fns instead, verifying the same tests still pass
  • Refactored search-range-filter.component.ts to parse the necessary year via a new yearFromString() method in date.util.ts and added tests to verify it behaves the same as the old momentjs code.
  • Removed ngx-moment which didn't appear to be used anywhere in our codebase.

Include guidance for how to test or review your PR. This may include: steps to reproduce a bug, screenshots or description of a new feature, or reasons behind specific changes.

tdonohue avatar Oct 14 '22 18:10 tdonohue

This pull request introduces 1 alert when merging 502d4bbdb11fe0c132f0f5c3aea98d93d05d0df4 into 273c7543707ccdad44c7fcafcfa967c1fd0b98e9 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

lgtm-com[bot] avatar Oct 14 '22 18:10 lgtm-com[bot]

This PR is highly appreciated. Thank you @tdonohue.

pnbecker avatar Oct 18 '22 11:10 pnbecker

@artlowel : Assigning to you for review as you volunteered to look at shrinking main.js. Please feel free to delegate if you wish.

tdonohue avatar Oct 20 '22 15:10 tdonohue

Merging, as this is at +1. Minor feedback was addressed and tests still pass.

tdonohue avatar Oct 27 '22 20:10 tdonohue