Use CommonJS export for "module": "node16" + ESM

[karlhorky]

Hi there 👋 First of all, thanks for slugify, it's very useful!

The ESM-style export (export default slugify) causes an error when using the "module": "node16" option in tsconfig.json and "type": "module" in package.json:

$ yarn tsc
index.ts:3:1 - error TS2349: This expression is not callable.
  Type 'typeof import("/home/projects/node-wshdsj/node_modules/slugify/slugify")' has no call signatures.

3 slugify('');
  ~~~~~~~


Found 1 error in index.ts:3

error Command failed with exit code 2.

Demo on StackBlitz - run yarn tsc in the Terminal at the bottom to reproduce:

https://stackblitz.com/edit/node-wshdsj?file=tsconfig.json,package.json,index.ts&file=tsconfig.json,index.ts

Background

The change to ESM-style export in #19 seemed like the right solution at the time, because bundlers such as webpack + ts-loader did not understand the CommonJS syntax (export = slugify).

However, module systems have evolved since then and now the export default fails when "module": "node16" is configured in tsconfig.json inside of a project which is an ES Module ("type": "module" in package.json).

Since slugify is a CommonJS module, it seems like using a CommonJS-style export is indeed the right choice for this package.

See more information about the problem of misconfigured declaration files in packages here:

• TypeScript module "Node16" does not resolve types of CJS module microsoft/TypeScript#49160 (comment)
• TS 4.7.1-rc cannot resolve default CJS or ESM exports with module nodenext. microsoft/TypeScript#49189 (comment)

[karlhorky]

cc @andrewbranch in case you can see an error in my logic about "module": "node16" + PR above, would be amazing to have a tip here!

[andrewbranch]

slugify’s JS uses a UMD wrapper that includes

module.exports = factory()
module.exports['default'] = factory()

so technically the export default in the types isn’t wrong, it’s just incomplete. In your repro, you can do slugify.default('') and TypeScript will recognize that as good, and it will work in Node too. But obviously you want to be able to get at the module exports without the unnecessary .default. To do that, off the top of my head, I would write the types like this:

declare module slugify {
  type ExtendArgs = {
    [key: string]: any;
  }

  export function extend (args: ExtendArgs): void;
  const _default: typeof slugify;
  export { _default as default };
}

declare function slugify(
  string: string,
  options?:
    | {
        replacement?: string;
        remove?: RegExp;
        lower?: boolean;
        strict?: boolean;
        locale?: string;
        trim?: boolean;
      }
    | string,

): string;

export = slugify;
export as namespace slugify;

Note the const _default: typeof slugify; export { _default as default }; in the existing merged namespace as well as the export = slugify change. Together, those cover both the module.exports as well as the module.exports.default. The export as namespace slugify just declares that the module is also available as a global in script files, since it does have a browser-compatible UMD wrapper. That can be dropped if global usage is discouraged.

[andrewbranch]

Also, I will note that this kind of problem is not yet detectable by https://arethetypeswrong.github.io, but it’s next on my list.

[karlhorky]

Ok great, added the changes for the UMD wrapper in b443c79 . I checked and this seems to also work in my project with "module": "node16"

@simov @Trott what do you think?

[GabrielDelepine]

Edit: I realized that the issue I described in my comment was not caused by the changes proposed in this PR. Actually, merging this PR will be solving my problem

[remcohaszing]

Is there a reason to use module over namespace here? Also is there any particular reason to define exports in the module instead of an interface?

I think it’s purely stylistic, but I’m curious to know if there are any differences. I would write it like this:

declare namespace slugify {
  type ExtendArgs = {
    [key: string]: any;
  }

  interface Options {
    replacement?: string;
    remove?: RegExp;
    lower?: boolean;
    strict?: boolean;
    locale?: string;
    trim?: boolean;
  }
}

interface Slugify {
  (string: string, options?: slugify.Options | string): string;
  default: Slugify;
  extend: (args: slugify.ExtendArgs) => void;
}

declare const slugify: Slugify;

export = slugify;
export as namespace slugify;

[andrewbranch]

There’s no difference, and namespace is the preferred keyword. I used module only because I copied and pasted the original code.