-
-
Notifications
You must be signed in to change notification settings - Fork 834
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
Switch from 'moment' to 'dayjs' #2219
Conversation
Looks like there is an official plugin for duration. Do we want/need this? It'd add ~4 KB back. Forum goes to 324 KB and admin remains the same. |
Are we using duration? |
@askvortsov1 We were for time lapsed in the post stream. Some other extensions use it as well. Not sure if we want it because it may have differences in some locales... the function I replaced it with in EDIT: I think we can ignore it. All it really adds is the moment-esque API I think. Its a very small file (https://github.com/iamkun/dayjs/blob/dev/src/plugin/duration/index.js). The whole thing basically boils down to |
Alright I'm fine not adding it then 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure this get's added to the Beta.14 meta issue after merge
I've replaced |
Almost exactly the same as d81ccee.
I changed the
User.prototype.isOnline
method to not use greater than and instead use thedayjs
queryisAfter
.forum.js
: 361 KB ----> 318 KB (-12%)forum.js.map
: 1.5 MB ----> 1.3 MB (-13%)admin.js
: 280 KB ----> 240 KB (-14%)admin.js.map
: 1.2 MB ----> 956 KB (-26%)Total bundle size reduction by 85 KB across forum & admin (excluding source maps).
moment
will still mostly work, it now is the same asdayjs
.The only big breaking change I think is that
moment.duration
is no longer a thing.In PostStream:
Source map comparison before & after
Before:
After: