-
Notifications
You must be signed in to change notification settings - Fork 72.7k
Remove Moment #8348
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
base: dev
Are you sure you want to change the base?
Remove Moment #8348
Conversation
…tings. Intl.supportedValuesOf("timezone") is the same thing, but browser native
|
I tried this earlier and at that point bumped into issues with DayJS having significant issues with date math & time zones. Seems some of this has been fixed, but curious to hear how extensively you've tested this? See iamkun/dayjs#1805 |
|
I've only done preliminary testing- haven't run it through its paces yet, figured it would be useful to gather some feedback prior. Sounds like I was right. The idempotency issue is definitely something I could've missed, I'll let you know how I go with it! |
We have gone through similar issues at work, where we heavily rely on moment for a very date arithmetic and timezone reliant heavy software bundle we maintain. Careful with just swapping this, especially if (like here) there was not just reliance on moment but also moment-tz. |
…tings. Intl.supportedValuesOf("timezone") is the same thing, but browser native
|
Minor question @bewest; currently, the API rejects illegal date formats; it('should reject invalid date - illegal format', async () => {
let res = await self.instance.post(self.url, self.jwt.create)
.send(Object.assign({}, self.validDoc, { date: '2019-20-60T50:90:90' }))
.expect(400);
res.body.status.should.equal(400);
res.body.message.should.equal('Bad or missing date field');
});However, DayJS is a little more lenient than MomentJS is with its parsing- it can parse that as a valid date, |
This pull request removes the MomentJS dependency, and replaces it with the much lighter DayJS, which has a largely identical API.
This has resulted in a package bundle size reduction of 630Kb. DayJS is not tree-shakeable, and we could get an ever so slightly smaller bundle size if we went with date-fns, but I figured that the 1:1 nature of the migration makes DayJS the clear winner.
Instead of completely remove the
.momentapi from the context, I've added.dayjs()in case there are some corners of the project which I've missed- I've Ctrl+F'd and replaced it wherever found it (but did NOT find and replace to change the dependency, as that would overwrite properties inloop.js.