Skip to content

fix: replace moment with dayjs #737

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,11 @@
"@postlight/ci-failed-test-reporter": "^1.0",
"browser-request": "github:postlight/browser-request#feat-add-headers-to-response",
"cheerio": "^0.22.0",
"dayjs": "^1.11.7",
"difflib": "github:postlight/difflib.js",
"ellipsize": "0.1.0",
"iconv-lite": "0.5.0",
"jquery": "^3.5.0",
"moment": "^2.23.0",
"moment-parseformat": "3.0.0",
"moment-timezone": "0.5.37",
"postman-request": "^2.88.1-postman.31",
"string-direction": "^0.1.2",
"turndown": "^7.1.1",
Expand Down
30 changes: 19 additions & 11 deletions src/cleaners/date-published.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import moment from 'moment-timezone';
import parseFormat from 'moment-parseformat';
// Is there a compelling reason to use moment here?
// Mostly only being used for the isValid() method,
// but could just check for 'Invalid Date' string.
import dayjs from 'dayjs';
import customParseFormat from 'dayjs/plugin/customParseFormat';
import utc from 'dayjs/plugin/utc';
import timezonePlugin from 'dayjs/plugin/timezone';
import advancedFormat from 'dayjs/plugin/advancedFormat';

import {
MS_DATE_STRING,
Expand All @@ -16,6 +16,11 @@ import {
TIME_WITH_OFFSET_RE,
} from './constants';

dayjs.extend(customParseFormat);
dayjs.extend(utc);
dayjs.extend(timezonePlugin);
dayjs.extend(advancedFormat);

export function cleanDateString(dateString) {
return (dateString.match(SPLIT_DATE_STRING) || [])
.join(' ')
Expand All @@ -27,21 +32,24 @@ export function cleanDateString(dateString) {

export function createDate(dateString, timezone, format) {
if (TIME_WITH_OFFSET_RE.test(dateString)) {
return moment(new Date(dateString));
return dayjs(new Date(dateString));
}

if (TIME_AGO_STRING.test(dateString)) {
const fragments = TIME_AGO_STRING.exec(dateString);
return moment().subtract(fragments[1], fragments[2]);
return dayjs().subtract(fragments[1], fragments[2]);
}

if (TIME_NOW_STRING.test(dateString)) {
return moment();
return dayjs();
}

return timezone
? moment.tz(dateString, format || parseFormat(dateString), timezone)
: moment(dateString, format || parseFormat(dateString));
if (timezone) {
return format
? dayjs.tz(dateString, format, timezone)
: dayjs.tz(new Date(dateString), timezone);
}
return format ? dayjs(dateString, format) : dayjs(new Date(dateString));
}

// Take a date published string, and hopefully return a date out of
Expand Down
32 changes: 16 additions & 16 deletions src/cleaners/date-published.test.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import assert from 'assert';
import moment from 'moment-timezone';
import dayjs from 'dayjs';

import cleanDatePublished, { cleanDateString } from './date-published';

describe('cleanDatePublished(dateString)', () => {
it('returns a date', () => {
const datePublished = cleanDatePublished('published: 1/1/2020');

assert.equal(datePublished, moment('1/1/2020', 'MM/DD/YYYY').toISOString());
assert.equal(datePublished, dayjs('1/1/2020', 'M/D/YYYY').toISOString());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is how the old tests were too, but can we change all these expected dates into hard-coded strings? I think that would make the functionality we're testing more clear: we expect strings of varying format and quality, and we always return a string in a specific format.

So this line would become:

assert.equal(datePublished, '2020-01-01T00:00:00.000Z');

I think with that change we could probably remove dayjs as a dependency from this test file entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly, i'll have a commit with these changes shortly ✅

});

it('returns null if date is invalid', () => {
Expand All @@ -28,37 +28,37 @@ describe('cleanDatePublished(dateString)', () => {
it('accepts a custom date format', () => {
// The JS date parser is forgiving, but
// it needs am/pm separated from a time
const datePublished = cleanDatePublished('Mon Aug 03 12:45:00 EDT 2015', {
const datePublished = cleanDatePublished('Aug 03 12:45:00 EDT 2015', {
timezone: 'America/New_York',
format: 'ddd MMM DD HH:mm:ss zz YYYY',
format: 'MMM DD HH:mm:ss z YYYY',
});
assert.equal(datePublished, '2015-08-03T16:45:00.000Z');
});

it('can handle dates formatted as "[just|right] now"', () => {
const date1 = cleanDatePublished('now');
const newDate1 = moment(date1)
const newDate1 = dayjs(date1)
.format()
.split('T')[0];
const expectedDate1 = moment()
const expectedDate1 = dayjs()
.format()
.split('T')[0];
assert.equal(newDate1, expectedDate1);

const date2 = cleanDatePublished('just now');
const newDate2 = moment(date2)
const newDate2 = dayjs(date2)
.format()
.split('T')[0];
const expectedDate2 = moment()
const expectedDate2 = dayjs()
.format()
.split('T')[0];
assert.equal(newDate2, expectedDate2);

const date3 = cleanDatePublished('right now');
const newDate3 = moment(date3)
const newDate3 = dayjs(date3)
.format()
.split('T')[0];
const expectedDate3 = moment()
const expectedDate3 = dayjs()
.format()
.split('T')[0];
assert.equal(newDate3, expectedDate3);
Expand All @@ -69,30 +69,30 @@ describe('cleanDatePublished(dateString)', () => {
// "X days ago" will not be accurate down to the exact time
// "X months ago" will not be accurate down to the exact day
const date1 = cleanDatePublished('1 hour ago');
const newDate1 = moment(date1)
const newDate1 = dayjs(date1)
.format()
.split('T')[0];
const expectedDate1 = moment()
const expectedDate1 = dayjs()
.subtract(1, 'hour')
.format()
.split('T')[0];
assert.equal(newDate1, expectedDate1);

const date2 = cleanDatePublished('5 days ago');
const newDate2 = moment(date2)
const newDate2 = dayjs(date2)
.format()
.split('T')[0];
const expectedDate2 = moment()
const expectedDate2 = dayjs()
.subtract(5, 'days')
.format()
.split('T')[0];
assert.equal(newDate2, expectedDate2);

const date3 = cleanDatePublished('10 months ago');
const newDate3 = moment(date3)
const newDate3 = dayjs(date3)
.format()
.split('T')[0];
const expectedDate3 = moment()
const expectedDate3 = dayjs()
.subtract(10, 'months')
.format()
.split('T')[0];
Expand Down
21 changes: 5 additions & 16 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2577,6 +2577,11 @@ date-now@^0.1.4:
version "0.1.4"
resolved "https://registry.yarnpkg.com/date-now/-/date-now-0.1.4.tgz#eaf439fd4d4848ad74e5cc7dbef200672b9e345b"

dayjs@^1.11.7:
version "1.11.7"
resolved "https://registry.yarnpkg.com/dayjs/-/dayjs-1.11.7.tgz#4b296922642f70999544d1144a2c25730fce63e2"
integrity sha512-+Yw9U6YO5TQohxLcIkrXBeY73WP3ejHWVvx8XCk3gxvQDCTEmS48ZrSZCKciI7Bhl/uCMyxYtE9UqRILmFphkQ==

debug@2.6.9, debug@^2.1.2, debug@^2.3.3, debug@^2.6.8, debug@^2.6.9:
version "2.6.9"
resolved "https://registry.yarnpkg.com/debug/-/debug-2.6.9.tgz#5d128515df134ff327e90a4c93f4e077a536341f"
Expand Down Expand Up @@ -6132,22 +6137,6 @@ module-deps@^6.0.0:
through2 "^2.0.0"
xtend "^4.0.0"

moment-parseformat@3.0.0:
version "3.0.0"
resolved "https://registry.yarnpkg.com/moment-parseformat/-/moment-parseformat-3.0.0.tgz#3a1dc438b4bc073b7e93cc298cfb6c5daac26dba"

moment-timezone@0.5.37:
version "0.5.37"
resolved "https://registry.yarnpkg.com/moment-timezone/-/moment-timezone-0.5.37.tgz#adf97f719c4e458fdb12e2b4e87b8bec9f4eef1e"
integrity sha512-uEDzDNFhfaywRl+vwXxffjjq1q0Vzr+fcQpQ1bU0kbzorfS7zVtZnCnGc8mhWmF39d4g4YriF6kwA75mJKE/Zg==
dependencies:
moment ">= 2.9.0"

"moment@>= 2.9.0", moment@^2.23.0:
version "2.29.4"
resolved "https://registry.yarnpkg.com/moment/-/moment-2.29.4.tgz#3dbe052889fe7c1b2ed966fcb3a77328964ef108"
integrity sha512-5LC9SOxjSc2HF6vO2CyuTDNivEdoz2IvyJJGj6X8DJ0eFyfszE0QiEd+iXmBvUP3WHxSjFH/vIsA0EN00cgr8w==

ms@0.7.2:
version "0.7.2"
resolved "https://registry.yarnpkg.com/ms/-/ms-0.7.2.tgz#ae25cf2512b3885a1d95d7f037868d8431124765"
Expand Down