Convert common time helpers to Typescript

[lhsazevedo]

See #3533

[askvortsov1]

This mostly makes sense, thanks!

[clarkwinkelmann]

Why does one import uses import * as dayjs from 'dayjs'; while the others use import dayjs from 'dayjs'; ?

Do we actually want to import global variables or could we just type-hint them somewhere? Using the global variable instead of an import will save up on the file size, as webpack won't have to add the additional code that resolves each import.

[dsevillamartin]

@clarkwinkelmann We can test locally if the bundle size increases if they're just used for typings. Haven't done so myself yet.

[lhsazevedo]

I removed the import * as dayjs variant and now all three changed files use the default export

[askvortsov1]

Looks good to me! I'm approving this as long as

Do we actually want to import global variables or could we just type-hint them somewhere? Using the global variable instead of an import will save up on the file size, as webpack won't have to add the additional code that resolves each import.

doesn't prove to be an issue

[lhsazevedo]

@askvortsov Thanks, I'll test this as soon as possible.

[lhsazevedo]

Test

So, apparently webpack doesn't inject the module resolving code on each module, but use a global one. Here's the test i did.

Flarum build

Heres src/common/utils/humanTime.ts unminified output:

"use strict";
__webpack_require__.d(__webpack_exports__, "a", function() { return humanTime; });
var dayjs = __webpack_require__(28);
var dayjs_default = /*#__PURE__*/__webpack_require__.n(dayjs);
var dayjs_plugin_relativeTime = __webpack_require__(73);
var dayjs_plugin_relativeTime_default = /*#__PURE__*/__webpack_require__.n(dayjs_plugin_relativeTime);

function humanTime(time) {
  var d = dayjs_default()(time);
  var now = dayjs_default()();

  // ...
}

Dayjs plugins

One thing we could optimize is, even not using directly, we need to import relativeTime dayjs plugin at js/src/common/utils/humanTime.ts. Without this import, typescript can't pick up tipes for the fromNow() method and complains.
The code still works though, because relativeTime plugin is installed at js/src/common/index.js:

https://github.com/flarum/core/blob/2ff46f5a2ebe2532f780f6e60a921beb41876d69/js/src/common/index.js#L12-L16

We can avoid this import if we move this plugin installation process to a separated dayjs library module, something like:

// js/src/common/lib/dayjs.ts

import dayjs from 'dayjs'
import relativeTime from 'dayjs/plugin/relativeTime';

dayjs.extend(relativeTime)

export default dayjs


// js/src/common/utils/humanTime.ts

// no need import relativeTime from 'dayjs/plugin/relativeTime';
import dayjs from '../../common/lib/dayjs'

Should I open a new issue for this? See #2417

[askvortsov1]

So, apparently webpack doesn't inject the module resolving code on each module, but use a global one. Here's the test i did.

Thank you for testing this!

We can avoid this import if we move this plugin installation process to a separated dayjs library module, something like:

Hmm I really like this idea! @clarkwinkelmann @rob006 could this also work for https://discuss.flarum.org/d/25196-beta-14-dayjs-translations?

[dsevillamartin]

@askvortsov1 It could technically work but the config files would need to use flarum.core.compat['flarum/lib/dayjs'] to access it.

We could also just have language packs set the JS objects to an app.* property and have the language logic set the dayjs locale itself with that data.

[lhsazevedo]

It seems that it would be a more drastic change, and the code already works without it, so I created a new issue for this subject so that we can move forward here.

[clarkwinkelmann]

I'm triggered by the imports order ahah (fullTime imports dayjs first, while humanTime imports Mithril first 🙉 )

Looks good otherwise.

[lhsazevedo]

@clarkwinkelmann Fixed following this rule: external imports first, internal last, both alphabetically sorted.