Skip to content
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

Merged
merged 3 commits into from Jul 1, 2020
Merged

Conversation

dsevillamartin
Copy link
Member

@dsevillamartin dsevillamartin commented Jun 29, 2020

Almost exactly the same as d81ccee.
I changed the User.prototype.isOnline method to not use greater than and instead use the dayjs query isAfter.

  • 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 as dayjs.

The only big breaking change I think is that moment.duration is no longer a thing.

In PostStream:
image

Source map comparison before & after

Before:
image

After:
image

@dsevillamartin
Copy link
Member Author

The moment config in language packs should continue functioning for the most part.

With no additional changes, Dutch language pack working just fine.

image

@dsevillamartin
Copy link
Member Author

dsevillamartin commented Jun 29, 2020

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.

@askvortsov1
Copy link
Sponsor Member

Are we using duration?

@dsevillamartin
Copy link
Member Author

dsevillamartin commented Jun 29, 2020

@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 PostStream basically does the same thing as far as I can tell.

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 dayjs().add(<time>, <measure>).fromNow().

@askvortsov1
Copy link
Sponsor Member

Alright I'm fine not adding it then 👍

Copy link
Contributor

@tankerkiller125 tankerkiller125 left a 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

@askvortsov1 askvortsov1 merged commit 8dd5420 into master Jul 1, 2020
@askvortsov1 askvortsov1 deleted the ds/frontend-rewrite-dayjs branch July 1, 2020 00:33
franzliedke added a commit to flarum/statistics that referenced this pull request Aug 28, 2020
franzliedke added a commit to flarum/suspend that referenced this pull request Aug 28, 2020
@franzliedke
Copy link
Contributor

I've replaced moment with dayjs in the two bundled extensions (statistics, suspend) that used it. We should consider deprecating the moment alias for stable.

askvortsov1 pushed a commit to flarum/statistics that referenced this pull request Mar 11, 2022
askvortsov1 pushed a commit to flarum/suspend that referenced this pull request Mar 11, 2022
askvortsov1 pushed a commit to flarum/statistics that referenced this pull request May 10, 2022
askvortsov1 pushed a commit to flarum/suspend that referenced this pull request May 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants