fix: replace moment with dayjs #737
Conversation
|
Nice! There are tons of examples of real-world timestamps in the fixture files which we can test here as well. I think the best we can aim for here is an equal success rate against all those timestamps to what we were getting with moment. |
|
@johnholdun yep i'm running through all the tests now, realized |
|
|
||
| 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()); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
exactly, i'll have a commit with these changes shortly ✅
touchRED
force-pushed
the
fix-remove-moment-js
branch
from
1b4cc52
to
6a5f892
Compare
touchRED
force-pushed
the
fix-remove-moment-js
branch
from
bdda1bb
to
9cf23f1
Compare
|
@johnholdun clearly the updates we spoke about last week took significantly longer than I expected! Now that |
|
Hey is this issue still be worked on? |
#731
Replaces Moment with DayJS, a 2kb library with an almost identical API.
Note:
parseFormatmethod used here. I'm not quite sure how long it could take to recreate this method, and I don't believe the moment-parseformat plugin can function without Moment installed. Thoughts?