Export module instead of namespace for TS declaration

[sullvn]

The source code uses modules, so now the types will too.

This fixes the issue of not being able to use the default export although it exists.

The existing workaround is to use a project-wide tsconfig.json which works around this issue (see below). It's less than ideal, as it affects the entire project of the user. This PR should resolve that.

    "esModuleInterop": true,
    "allowSyntheticDefaultImports": true,

[codecov[bot]]

Codecov Report

Merging #751 into dev will not change coverage. The diff coverage is n/a.

@@         Coverage Diff         @@
##            dev   #751   +/-   ##
===================================
  Coverage   100%   100%           
===================================
  Files       154    154           
  Lines      1030   1030           
  Branches    162    162           
===================================
  Hits       1030   1030

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update aa0f210...fbe6972. Read the comment docs.

[iamkun]

Should this be a breaking change? It will change all the existing code from import * as dayjs from dayjs to import dayjs from dayjs I think.

[sullvn]

Yep it's a breaking type-checking change -- although I'd be surprised if anyone's code with import * as dayjs from dayjs actually works with the latest emitted ES modules for Dayjs which use export default. Mine didn't, hence investigating this issue.

To elaborate, TypeScript allows you to do import package from 'package' instead of the actual import * as package from 'package'. This is done with the allowSyntheticDefaultImports flag.

There is no setting to allow you to do the opposite -- import * as package from 'package' instead of the actual import package from 'package'.

That's why I'd be surprised if anyone is actually able to use import * as dayjs from dayjs. But if they are somehow, then this would be a breaking change.

[ilisha13]

Hi! I am running into a similar issue with one of the plugins and I'm wondering if it's related.

I am trying to add the utc plugin to my dayjs in typescript.

import utc from 'dayjs/plugin/utc';
dayjs.extend(utc);

This fails with the error:

Uncaught TypeError: t is not a function
    at Function.g.extend (dayjs.min.js:1)

If I change to import * as utc from 'dayjs/plugin/utc';, it works.

[iamkun]

https://day.js.org/docs/en/installation/typescript This might help.

And we will update this in the next major release.

[kirillgroshkov]

Yes, would be nice! Meanwhile we're forking/patching Dayjs to achieve that. Would be nice to have it by default.

Thanks for the great library!

[iamkun]

is "remove "module" field as a work-around https://github.com/NaturalCycles/time-lib/commit/d5f06aeacf8a439c9f653e8d2891a3dc226b6276" the key to this issue ?

[kirillgroshkov]

is "remove "module" field as a work-around NaturalCycles/time-lib@d5f06ae" the key to this issue ?

No, it's just a workaround.

I think there are 2 issues to fix:

1. Make Dayjs work with both es-style imports (import dayjs from 'dayjs') and typescript-style imports (import * as dayjs from 'dayjs'). In "js world".
2. Make Dayjs export correct types ("ts world"), so it works for both types of imports.

Right now I cannot say if this PR fixes both, or one, or none.

But if you're positive about fixing this issue (even if it becomes a Breaking change for Typescript users) - I can try to put together my version of a fix as a separate PR.

[danawoodman]

Related to #475 I believe

[sullvn]

Hey everyone, to add some more context for this type of change:

The TypeScript definitions should reflect the actual JavaScript API. They should be the same; not different. That's what TS is designed for.

One of the core issues is that dayjs is implemented with a monkeypatched, object-based API, but is exported as a default value in an ES-module. It's hard to write types for this in-between API, so there's no suprise there are issues from incompatibility.

API object and monkey-patching:

https://github.com/iamkun/dayjs/blob/14ab808a7b7e226f2eb2cbe894916a18ed5d967d/src/index.js#L31

https://github.com/iamkun/dayjs/blob/14ab808a7b7e226f2eb2cbe894916a18ed5d967d/src/index.js#L399-L415

And then exported as ES module default:

https://github.com/iamkun/dayjs/blob/14ab808a7b7e226f2eb2cbe894916a18ed5d967d/src/index.js#L417

The alternative is moving to typical ES module exports, without monkeypatching. This would play nicer with bundlers and tooling like TypeScript. Here is the above, mechanically converted:

export const extend = (plugin, option) => {
  plugin(option, Dayjs, dayjs)
  return dayjs
}

export const locale = parseLocale

export const isDayjs = isDayjs

export const unix = timestamp => (
  dayjs(timestamp * 1e3)
)

export const en = Ls[L]
export const Ls = Ls

export default dayjs

In that case, the correct TypeScript definition is trivial. It's basically the same code, but without the implementations.

Then both JS and TS would have the same ES module API that allows both styles:

//
// Usage style - 1
//
import dayjs, { isDayjs } from 'dayjs'

// Now undefined, unless monkeypatched object is still used
dayjs.isDayjs

//
// Usage style - 2
//
import * as dayjs from 'dayjs'

// Defined, even without monkeypatched object
dayjs.isDayjs

// Error, not a function
dayjs()

// Not an error, is a function
dayjs.default()

[mohamedrchalal]

This would be the correct way to export this library. Typescript explicitly follows ES standards, and as such, the "ES Module" way of doing things is the "right" way. Check out the answer to this stack overflow question https://stackoverflow.com/questions/37565709/how-to-use-namespaces-with-import-in-typescript If this PR could be fast tracked, my colleagues and I would be eternally grateful! And also, fantastic library!