Dayjs: Add plugins methods to typescript definition

[naulacambra]

Should plugins's api methods be added in the core d.ts file? Or they should have their owns?

If they have their own, they should be merge into one when bundling the package?

I'm trying to use this library at another library, which is written with Typescript, and trying to use plugin methods, it's driving me crazy. Any thoughts?

[suspectpart]

In my opinion it would be the proper approach to place d.ts definitions in the plugin's folder as a plugin should be self-contained. By keeping plugins self-contained it is easy to add or remove plugins by just adding / removing a folder in plugin/, making sure that a plugin does not alter the main bundle in any way whatsoever. I would even suggest to place tests right with the plugin, so every plugin has a structure like this:

└── somePlugin
    ├── index.js
    ├── index.d.ts
    └── somePlugin.test.js

That could be achieved by altering the jest config to something like

"jest": {
    "roots": [
      "test",
      "src/plugin"
    ],
    "testRegex": "(.*?/)?.*test.js$",
    "testURL": "http://localhost",
    "coverageDirectory": "./coverage/",
    "collectCoverage": true,
    "collectCoverageFrom": [
      "src/**/*"
    ]
  }

[iamkun]

@suspectpart Seems good.

[suspectpart]

Should I restructure the plugins that way and make a pull request so we can have the discussion based on the changes?

[iamkun]

We have to consider structure of our release code.

Currently:

plugin
└── advancedFormat.js
└── isBetween.js

And I'm not sure if code editor could support typescript definition file like this:
plugin
└── advancedFormat.js
└── advancedFormat.d.ts
└── isBetween.js
└── isBetween.d.ts

[suspectpart]

I guess it works by making the compiled plugin/ folder look like

└── isBetween
    ├── index.js
    └── index.d.ts

i.e. compiling plugin code into separate folders and copying the corresponding index.d.ts to reside next to the build artifact. Type definitions in those index.d.ts files need to extend the Dayjs class by doing something like this

declare namespace dayjs {
  interface Dayjs {
    isBetween(d1: dayjs.Dayjs, d2: dayjs.Dayjs)
  }
}

I quickly sketched this and could get Typescript to not complain, but I had no time to create a full working prototype.

[xxyuk]

well then this might be a breaking change since it change the file structure && name.

import AdvancedFormat from 'dayjs/plugin/advancedFormat'
import AdvancedFormat from 'dayjs/plugin/advancedFormat/index'

<script src="https://unpkg.com/dayjs/plugin/advancedFormat"></script>
<script src="https://unpkg.com/dayjs/plugin/advancedFormat/index,js"></script>

[suspectpart]

Hmm I see. I am not too familiar with those Typescript concerns, maybe someone else has a good suggestion how to keep plugins self-contained without breaking the existing structure?

[Noeldeveloper]

Since Angular 6, you can create a npm module.

That kind of module contains a file called public_api.ts, that is used to export any file you want.

Doing that, you will be able to import any of your library classes, just pointing to your library name (package.json name)

[suspectpart]

Since Angular 6, you can create a npm module.

That kind of module contains a file called public_api.ts, that is used to export any file you want.

Doing that, you will be able to import any of your library classes, just pointing to your library name (package.json name)

But is that Angular-specific then? Because we want to have general typings here, not tied npm or anything, right?

[bengry]

@Noeldeveloper I'm not sure what's the relation between dayjs and Angular. Sure, you can use dayjs in Angular apps, but the dependency is that way: Angular app depends on dayjs, not the other way around.

@suspectpart What @Noeldeveloper was referring to, I think, is Angular libraries, which are unrelated here.
Anyway, regarding to the issue at hand - there is no "great" solution regarding types I think, here are a few I can think of:

1. Adding a triple slash directive when you want to use a plugin, something like:

// app.ts
import dayjs from 'dayjs';
import relativeTime from 'dayjs/plugin/relativeTime';

dayjs.extend(relativeTime);

// another-file.ts
/// <reference path="./node_modules/dayjs/plugin/relativeTime.d.ts" />
import dayjs from 'dayjs';

dayjs.fromNow(); // no error

Needless to say, this is less than idle.

1. Same as #1, but break up the plugins into their own packages, then you can use the types triple slash directive:

// app.ts
import dayjs from 'dayjs';
import relativeTime from '@dayjs/relative-time';

dayjs.extend(relativeTime);

// another-file.ts
/// <reference types="@dayjs/relative-time" />
import dayjs from 'dayjs';

dayjs.fromNow(); // no error

1. Make the plugins export their own instance of dayjs, complete with the plugin:

// app.ts
No need for anything special here

// another-file.ts
import dayjs from 'dayjs/plugin/relativeTime';

dayjs.fromNow(); // no error;

The issue here is that you can't chain things along, since each plugin exposes their own instance of dayjs + the plugin

1. Make dayjs.extend return a new type, which the app will then re-export to other files:

// dayjs-config.ts
import * as _dayjs from 'dayjs';
import relativeTime from 'dayjs/plugin/relativeTime';

export const dayjs = _dayjs.extend(relativeTime); // returns dayjs.Dayjs & dayjs.relativeTime.Dayjs type

// another-file.ts
import { dayjs } from './dayjs-config'

dayjs.fromNow() // no error

1. Module augmentation (similar to what d3 does)

Personally, I think either 4 or 5 are the best ways to handle this, but any of the above would work, and there are probably other ways to handle plugins with types I didn't think of.

[naulacambra]

As mentioned on #456, an update for the plugins folder structure might be necessary. @iamkun pointed well that since we'll update the plugins folder structure, we could start working on this issue.

I'm not an expert on TS, but as far as I know, the 5th option that @bengry mentioned seems a nice way to go. Any other thoughts?

[iamkun]

Just tested on vscode, if folder structure like this:

└── advancedFormat.js
└── advancedFormat.d.ts
└── isBetween.js
└── isBetween.d.ts

and import the plugin same as before, the typescript definition works well as expected. And if this is ok, we might not have to change plugins folder structure to fix this issue.

Any thoughts?

[bengry]

@iamkun I haven't tested this out, but I'd guess that the compiler would always assume the methods of the plugins are there - meaning that invoking: dayjs.fromNow() would show no compile-time error, even if dayjs.extend(relativeTime) wasn't called.

[iamkun]

the compiler would always assume the methods of the plugins are there

@bengry Tested again, you are right.
Any thoughts?
Problem continues.....

[bengry]

@iamkun I think options #⁠3 or #⁠4 are good alternatives. They can be made with no breaking changes (for #⁠3 - extend(plugin) can return a new type, in addition to making changes to the prototype of daysjs' instance). I also just noticed a typo in #⁠3 I mentioned above, fixed it - so re-read it to get the accurate proposal I made there.

[iamkun]

@bengry I don't like option #3 or #4. It's really a hack way to make it.

What I am looking for is to fix this from tyopescript definition file side, instead of changing our API logic.

[bengry]

@iamkun I think they're actually a bit better, since there's no "magic" behind the scenes. Similar to the route that rxjs took - moving from prototype changes to explicit calls:

// RxJS < 6
import { Observable } from 'rxjs';
const x$: Observable<number>;

x$.filter(...).map(...)

// RxJS >= 6
import { Observable } from 'rxjs';
import { filter, map } from 'rxjs/operators';

const x$: Observable<number>;
x$.pipe(
  filter(...),
  map(...),
)

Anyway, I don't think there's a way to keep type safety together with prototype changes - since [_I think_] you don't know at compile-time if the call that modified the prototype has been made or not. Maybe @DanielRosenwasser can offer some help or insights here.

What you suggested in https://github.com/iamkun/dayjs/issues/364#issuecomment-455433128 is probably the only route without changing the way plugins work, but at the same time - it makes forks like Day.js Extended pop up, which offer more functionality, for sure - but one that couldn't be added with ample type-safety without making modifications to how plugins work. @prantlf might offer some additional insights here. Ideally this repo would contain "just" a handful of plugins that you can import and use with the original dayjs (there are stuff there that can't be added via plugins, like the instanceof check, but those could probably be PRed into this repo).

[t49tran]

@iamkun I agree with @bengry and think option 3 is a really good solution. It is basically making the extend function a generic function, which is what it really is. It doesn't affect the way the plugin currently works and is a complete Typescript only solution.

The type definition for the extend function can be as simple as:

type ExtendFunction = <T>(plugin: T) => DayJS & T;

[iamkun]

@t49tran I agreed that option 3 is a nice solution. But it seems a breaking change, user has to modify their current code to make plugins work properly. Besides, what if we want to use multiple plugins at the same time? (Or may be some of the plugin is relay on some other plugin.)

[bengry]

@iamkun option 3 isn't necessarily a breaking change (though it can be). Consider that extend can still modify the prototype, and also return the new instance. This means that any existing usage will still work, but to get type safety you'll need to re-export the instance created by the extend call.
Regarding multiple plugins that relay on one another - you can chain them together (or even modify the extend method to take in an array of plugins):

import dayjs from 'daysjs';
import AdvancedFormat from 'dayjs/plugin/advancedFormat';
import RelativeTime from 'dayjs/plugin/relativeTime';

const dayJsWithRelativeTimeAndAdvancedFormat = dayjs.extend(AdvancedFormat).extend(RelativeTime); // type is inferred as dayjs.DayJS & AdvancedFormat & RelativeTime;

// or if we change the `extend` to be defined as:
export class DayJS {
  ...
  extend<T extends PluginFunc>(plugin: T): DayJS & T;
  extend<T extends PluginFunc, U extends PluginFunc>(plugin1: T, plugin2: U): DayJS & T & U;
  extend<T extends PluginFunc, U extends PluginFunc, V extends PluginFunc>(plugin1: T, plugin2: U, plugin3: V): DayJS & T & U & V;
}

you can call it as:
const dayJsWithRelativeTimeAndAdvancedFormat = dayjs.extend(AdvancedFormat, RelativeTime); // type is inferred as dayjs.DayJS & AdvancedFormat & RelativeTime;

The latter is similar to how Object.assign is defined and behaves as.

Do note that order is important here, so if AdvancedFormat would depend on RelativeTime being defined, the above would fail (the other way around though would work). Again, somewhat similar to how Object.assign behaves in regards to overriding properties defined in multiple objects.

[iamkun]

@bengry I think I know what you mean, and seems a nice choice.

However, I don't know much about Typescript, and don't really know how to implement this to test if it works.

Can you make a small PR or repo with the modified extend function as well as plugin's d.ts file, if you got some time? That would be a great help to me. Just update one of the plugins will be enough to let me know what I should do later.

Cheers.

[bengry]

@iamkun I'll try to get around to doing something like this this week and either send a PR or make a sample repo with a fork showcasing this, and we can take it from there.

Update: See https://github.com/iamkun/dayjs/pull/463 for reference.

@iamkun I can also help out in migrating dayjs to be written in TypeScript, if that's something that you'd like to pursue. It makes more sense, in some cases, than keeping a separate type definitions file(s), that's not guaranteed to be in sync, or work like the source code (see the example in the PR in regards to UnitType).

[DanielRosenwasser]

I don't know much about this library, but if plugins just add more methods to an existing type, then you probably want a module augmentation. How they're loaded is another story. I don't think I can keep up and help unfortunately, but StackOverflow might be able to give good advice.

[saboya]

I've been adding my own module augmentations to use plugins for now, but I can't seem to find a way to make a proper one for the customParseFormat plugin. Any thoughts?

[bengry]

@saboya I just pushed a commit to #463 that makes typing plugins that modify the namespace possible (static plugins, as I've called them).
See https://github.com/iamkun/dayjs/pull/463/commits/f79c6a985f3e371261759015db462cac7348e934, though it is dependent on other work in the above PR.

[ypresto]

I updated PR #418, another plugin type defs with module augmentation.

I think it is reasonable to stick on import dayjs from 'dayjs' singleton (not re-exporting extend()ed instance of dayjs) for convenience, because we (at least I) don't want to change installed plugins place-by-place.

It is same as Vue's plugin architecture, and official firebase plugin uses module augmentation to extend type declaration of Vue itself. Looks widely accepted way :)

[saboya]

@ypresto Not really possible to correctly define typings for plugins such as customFormatParse though.

[ypresto]

@saboya A format is just a string value so there is nothing to do in TypeScript layer. TypeScript only constrains the type, not the content, of a value, except for enum-like values.

[saboya]

That's not what I meant. I meant the plugin extends the default constructor adding a new signature, but it's impossible to define it with a module augmentation, so the method signature without the plugin is basically incorrect.

[ypresto]

@saboya Ah, I see. It's a missing signature in index.d.ts .

dayjs('date', 'format') is short hand for dayjs('date', { format: 'format' }), and it is in src/index.js not a plugin.
https://github.com/iamkun/dayjs/blob/35e268acf075256e526b72f0a537afe58b5af1cc/src/index.js#L36

Also note that current plugin function (export type PluginFunc = (option: any, c: typeof Dayjs, d: typeof dayjs) => void) cannot override (replace) exported dayjs() function itself.

index.d.ts is fixed in 0bb72c67b235fdbb4acdca913d4b8e634dd28453 of #418!

[iamkun]

Hi all, just released v1.8.8 with plugin definition (module augmentation).

[iamkun]

Everything looks fine.

[beenotung]

Why don't we use static functions taking the dayjs object as first argument,
Then the typing will be easier to work.

If one still prefer to use plugin style to inject methods over composing functions (e.g. for chainable api), rxjs is a good example with plugin and typescript to take reference.

[xxyuk]

@beenotung sounds interesting, some more detail or demo code, please?

[JoepKockelkorn]

Is there a concluded advised way to use dayjs with plugins in a typesafe way?

[iamkun]

@JoepKockelkorn https://day.js.org/docs/en/installation/typescript

[JoepKockelkorn]

@iamkun Thanks for your reply. I already checked that page, but that still leaves me with a compiler error whenever I want to use a method of the Dayjs interface which is added on the prototype by a plugin. Do you have a working example?

[iamkun]

@JoepKockelkorn A reproduction repo, please?

[JoepKockelkorn]

@iamkun Hmm seems to work when I set it up in an isolated codesandbox.

My setup is a bit different though. I'm using nx from nrwl to build a monorepo and I'm extending dayjs in the app.component.ts so I can use it everywhere. Btw, it's not a compiler error, it all works at runtime. VScode just doesn't seem to know that 'isoWeek' exists in my feature component. This feature component is in another library with another tsconfig.json which might explain why Typescript can't deduct that isoWeek exists on the Dayjs interface. If I add this in the feature component it all works fine:

/// <reference path="../../../../../node_modules/dayjs/plugin/isoWeek.d.ts" />

Do you need a working example setup with nx to figure out why I have to add this triple-slash directive?

[iamkun]

In a pure typescript project, there's no other config needed. I don't know nx, it's should be the same I think.