fix: fix TypeScript type definitions for commonjs

[remcohaszing]

This is done by duplicating index.d.ts into index.d.cts, and modifying it for CommonJS. The same was done for type tests.

Unfortunately I was unable to find a way to re-use types without drastically changing the code base.

To test this, a minimum TypeScript version of 4.7 is needed, so it has been updated. The old types still work with older TypeScript versions.

[remcohaszing]

Everything reported by CodeQL was literally copied from the ESM type tests. 🙈

Also the lines it reported make sense for type tests.

[jasonsaayman]

Thanks this looks awesome!!

[jasonsaayman]

@remcohaszing seems that a test failed, it seems un-related but can you have a look and confirm?

[remcohaszing]

Yes, it looks unrelated. Could you retrigger the failed jobs?

[luisdeka]

4.8?

[remcohaszing]

This signals the minimal TypeScript version these types are tested against. At the moment of writing a value of 4.7 means these types are tested using TypeScript 4.7, 4.8, and 4.9 (next).

[sacummings91]

Upgrading from Axios 1.1.2 -> 1.1.3 results in multiple "Can't resolve x library" errors for us. I was combing through the github issues but struggling to find anything related. Is this related to that?

[remcohaszing]

No, those are build errors. This PR only touches type definitions.

[sacummings91]

Are the build errors going to be fixed? Changing moduleResolution to node16 results in other breaking changes in our app. So we can't really do that either

[jasonsaayman]

Adding this PR and then we will cut a 1.2.0 pre-release

[DigitalBrainJS]

@jasonsaayman The way this is implemented seems to make the project very difficult to maintain. Now we have completely separate types and type tests - this doesn't guarantee consistency and forces us to duplicate definitions and test code. Support for types definitions in such conditions will be too exhausting.
Here we need to find a way to reuse definitions for both entry points as much as possible. In addition, the commonjs module is just a fallback.
Personally, I think there is no point in adding such complexities to an almost obsolete thing after a decade of project existence. It's not a big deal that users will see a named export in IDE for a CJS module that doesn't actually exist because they've always used the default export to work with it and this can be covered by Readme.
But if we really need it, we should have generic types for the CJS entry point and another one for ESM that just re-exports the types as a named export like it was done in the JS code, but not totally independent files.

[remcohaszing]

I agree this is cumbersome. Unfortunately this problem comes with dual packaging.

Because this package defines "type": "module", TypeScript considers .ts files to be ESM. .cts files can’t import ESM TypeScript files.

I think this could be resolved by removing "type": "module" from package.json. Then instead of a .cts and .ts file for CommonJS, we get a .ts and .mts file. The .mts file can import from the .ts file and some logic could be reused.

Note that simply removing the CJS type definitions and keeping "type": "module" tells TypeScript this package can’t be imported in CommonJS at all. For example:

1. clone https://github.com/remcohaszing/axios-test-instance (Doesn’t include these updated types at the moment of writing)
2. Run npm install
3. In index.ts, remove the @ts-expect-error comment from the axios import.
4. Observe the error.

The simplest solution would be to just remove CJS support altogether, but that would be a massive breaking change.

[DigitalBrainJS]

@remcohaszing It seems that axios@1.2.0-alpha.1 can already be used in the TS project with import and require, this is covered by tests:

• https://github.com/axios/axios/blob/v1.x/test/module/ts-require-default/index.ts
• https://github.com/axios/axios/blob/v1.x/test/module/ts-require/index.ts
• https://github.com/axios/axios/blob/v1.x/test/module/ts/index.ts

Currently, both entry points have a factory export that has consistent typings for TS, but only ESM module has named export as an alternative.
The problem is not so significant - TS users will see the named export in autocompletion even for the commonJS module, where it is not present. That's the entire problem, so all this PR is about autocompletion, not importing Axios in TS context.
I think this is acceptable, especially considering that trying to fix this inconvenience would result in a significant overengineering of the project design.

I think this could be resolved by removing "type": "module" from package.json.

The package is an ESM JS module that requires "type": "module" in order for the import statements to work natively in JS. CJS is just a fallback for require.

The simplest solution would be to just remove CJS support altogether, but that would be a massive breaking change.

Removing the entire cjs module just because TS users will have some inaccuracies in their types, this is definitely not a solution.

[remcohaszing]

@remcohaszing It seems that axios@1.2.0-alpha.1 can already be used in the TS project with import and require,

Yes, that’s because this pull request is part of that release.

this is covered by tests:

• https://github.com/axios/axios/blob/v1.x/test/module/ts-require-default/index.ts

This doesn’t test any types. It uses:

const axios = require('axios').default;

This means require is used as a function, not as an import. This function is typed as a function returning any, so axios is any in that file. The correct way to require modules in TypeScript is

import axios = require('axios')

So all this test does, is the same as https://github.com/axios/axios/blob/v1.x/test/module/cjs/index.js at runtime.

I suggest to remove this.

• https://github.com/axios/axios/blob/v1.x/test/module/ts-require/index.ts

This is the same situation as above, except it tests the axios.default property that was added recently works at runtime.

• https://github.com/axios/axios/blob/v1.x/test/module/ts/index.ts

This does test the axios types, but very minimally. It asserts the types work with these very specific compiler options:

{
  "compilerOptions": {
    "module": "commonjs",
    "moduleResolution": "node",
    "esModuleInterop": true
  }
}

The "node" module resolution predates package.json exports. It uses the old types field in package.json. Because ESM and CJS aren’t properly interoperable, TypeScript 4.7 and greater added the node16 module resolution.

The types are tested using dtslint and these files:

• /test/typescript/axios.cts
• /test/typescript/axios.ts
• /tsconfig.json
• /tslint.json

Currently, both entry points have a factory export that has consistent typings for TS, but only ESM module has named export as an alternative.

I’m not sure what you’re trying to say here. ESM has a default export and a number of named exports. CJS had module.exports.

The problem is not so significant - TS users will see the named export in autocompletion even for the commonJS module, where it is not present. That's the entire problem, so all this PR is about autocompletion, not importing Axios in TS context. I think this is acceptable, especially considering that trying to fix this inconvenience would result in a significant overengineering of the project design.

There seems to be a misunderstanding here! This pull request has nothing to do with autocompletion. This is about correctness of the TypeScript types, and the ability to use axios in type checked CJS. Although this might lead to better autocompletion in editors that rely on TypeScript (such as VSCode).

I think this could be resolved by removing "type": "module" from package.json.

The package is an ESM JS module that requires "type": "module" in order for the import statements to work natively in JS. CJS is just a fallback for require.

The package could still support dual packaging. "type" only changes how .js and .ts files are interpreted. The difference is that what is now published as .js, should be published as .mjs, .ts → .mts, and .cts → .ts.

I’m not saying this should be done, but if it was, the .mts file could import and reuse types from the .ts file to prevent duplication. If you do find a way to deduplicate them, I would love to hear it!

The simplest solution would be to just remove CJS support altogether, but that would be a massive breaking change.

Removing the entire cjs module just because TS users will have some inaccuracies in their types, this is definitely not a solution.

I would seriously consider this for a next major version, but it’s not my call to make. Anyway, thanks to TypeScript 4.7 and this PR, it won’t be necessary for the TypeScript part.

---

TLDR: CJS and ESM are significantly different. Different workarounds in the past have led to more compatibility issues. Modules look easy, but they are hard! There are many aspects to it a lot of people don’t know.

TypeScript makes it even more complex. Before TypeScript 4.7 the best thing we had was "moduleResolution": "node". This implementation assumes CJS is used. TypeScript 4.7 added "moduleResolution": "node16". This adds support for dual publishing using the exports field in package.json, although it is a bit cumbersome for package authors.

[DigitalBrainJS]

Yes, that’s because this pull request is part of that release.

This PR was merged later, so tests don't rely on it, in addition, index.d.cts is currently incompatible with index.d.ts / actual export.

This doesn’t test any types.

And they shouldn't. These tests ensure that the Axios entry points are consistent and the module can be imported into the target environments, so the users can use it.

The package could still support dual packaging.

It already supports this for JS and TS environments, the only problem is any type instead of "AxiosStatic" when loaded with a "require" statement in TS. That's all.

But we can:

• use Axios without explicit typings when loading it with require
• use import statement instead of require in TS and have explicit typings without having separate typing for each entry-point

So the question here is, is it worth adding type support for imports with the require of increasing design and maintenance complexity?

The difference is that what is now published as .js, should be published as .mjs, .ts → .mts, and .cts → .ts.

Yes, but this looks like a hack, and it would be nice to have normal js files for a plain JS project.

I would seriously consider this for a next major version, but it’s not my call to make.

Not everyone uses TypeScript and the current version of Axios works fine in both ESM & CommonJS styles in JS as well as TS with require & import statements.

[remcohaszing]

Yes, that’s because this pull request is part of that release.

This PR was merged later, so tests don't rely on it,

I thought you meant you tried this in axios-test-instance. Indeed, the tests don’t rely on it, because as I explained, the CJS tests don’t use the TypeScript types at all.

in addition, index.d.cts is currently incompatible with index.d.ts / actual export.

How do you mean it’s incompatible?

This doesn’t test any types.

And they shouldn't. These tests ensure that the Axios entry points are consistent and the module can be imported into the target environments, so the users can use it.

Then I suggest to remove the build step from these tests. There are only four runtime entry points:

• ESM Node.js
• CJS Node.js
• ESM browser
• CJS browser

The package could still support dual packaging.

It already supports this for JS and TS environments, the only problem is any type instead of "AxiosStatic" when loaded with a "require" statement in TS. That's all.

But we can:

• use Axios without explicit typings when loading it with require

This statement is false! As explained above, the referenced tests don’t use require correctly. The correct way to use require in TypeScript is:

import axios = require('axios')

Let’s put it differently. TypeScript can be used to type check JavaScript code.

// tsconfig.json
{
  "compilerOptions": {
    "checkJs": true,
    "moduleResulution": "node16"
  }
}

// This is a type error
const axios = require('axios')

So the question here is, is it worth adding type support for imports with the require of increasing design and maintenance complexity?

The question you should ask is: Should Axios support type definitions? If yes, this PR fixed CJS support in TypeScript. If no, then it’s better to remove all type definitions from the repository and move them to DefinitelyTyped instead.

---

You seem to be downplaying the severity of this issue based on a lack of knowledge on how TypeScript modules work, especially on how this has changed since TypeScript 4.7. This more complex than your comments, please don’t underestimate it. This makes it hard to have a discussion about this subject.

I suggest you read the following resources:

• https://www.typescriptlang.org/docs/handbook/modules.html (old module system)
• https://www.typescriptlang.org/docs/handbook/esm-node.html (new module system)
• https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/#esm-nodejs (actually seems to do a better job at explaining it than the handbook)

I also suggest you really fiddle with the axios-test-instance source code, as it’s a simple real world example that shows the issue.

---

I do acknowledge it’s not ideal to duplicate type definitions. I have asked if type definitions can be reused between CJS and ESM in the TypeScript Discord help channel and am awaiting a reply.

[jasonsaayman]

Hey, @remcohaszing and @DigitalBrainJS this seems to have maybe become a bit of a warm topic. It is actually quite a terrible flaw that everything can not just work together and be easy, but I guess that is the world we live in.

I know I will need to read a bit more about this all to make sure we are making the right decision, though one question just off the bat @remcohaszing would move to DefinitelyTyped help bring down the overhead of maintaining both files? Also would typing the entire Axios project and going to a full TS approach remove the need for managing two sets of definitions?

[DigitalBrainJS]

Hey @jasonsaayman ! We have to realize here what percentage of users will be affected by the lack of typings for the commonjs module, the possible workarounds for users, and the reasonableness of solving this problem.
Since, in my opinion, this affects only a small percentage of TS users, so I do not understand the importance of solving typings lack of a fallback entry point if there is no simple/cheap solution.

As I see it, there are the following cases in TSConfig:

• module: CommonJS + require statement => axios common js entry point is used => types missing (any)
• module: CommonJS + import statement => axios common js entry point is used => we have proper typings (AxiosStatic)
• module: ESNext + import statement => axios ESM entry point is used => we have proper typings (AxiosStatic)

The common js entry point is mostly there for JS users to use without babel, TS users can just use an import statement and have the correct typings with conversion to CommonJS or ESM. Or I'm missing something? Is there a reason to use require instead of import in actual versions of TS and projects that use it?

[remcohaszing]

We currently live in a situation where the JavaScript ecosystem is slowly transitioning from a CJS to ESM. This means package authors need to make a choice: publish CJS, publish ESM, or publish both CJS and ESM.

Prior to Axios 1, only CJS was supported. Yes, there was a "module" entry point that’s understood by module bundlers, but as far as types are concerned, only "main" mattered.

The correct way to use Axios back then, was:

const axios = require('axios')

The correct type definitions for this were:

declare namespace axios {
  // …
}

declare function axios(/* … */): // /…

export = axios

However, many people do not understand how to properly type CJS modules. Instead, the following types were published:

declare function axios(/* … */): // /…

export default axios

According to these types, this was correct usage:

const axios = require('axios').default

So to accommodate this, the runtime default property was added.

This is not just the case for Axios. Many packages did this wrong, and many still have wrong type definitions today.

---

Various tools added different ways to attempt interoperability between ESM and CJS. Often people would write ESM syntax, then compile to this:

export default function foo() {}

into this:

function foo() {}

module.exports.default = foo

TypeScript added the esModuleInterop flag to support this when compiling to CJS. This would add some boilerplate code that attempts to resolve compiled default exports. So this:

import foo from 'foo'

Becomes this:

"use strict";
var __importDefault = (this && this.__importDefault) || function (mod) {
    return (mod && mod.__esModule) ? mod : { "default": mod };
};
Object.defineProperty(exports, "__esModule", { value: true });
const foo_1 = __importDefault(require("foo"));

This option is fine when building an end-user library, such as a website or server, but not desirable for libraries.

---

Node.js 12 added native support for ESM. To support compatibility with CJS, the module.exports field would be interpreted as the default export. So in order to consume the prior code in ESM, one would have to type this:

import foo_exports from 'foo'

const foo = foo_exports.default

This is not great, but that’s how many libraries are developed. Now the problem is, TypeScript didn’t understand how to consume this. So TypeScript would show errors for that code.

Not only did Node.js 12 add support for ESM, but also for the "exports" field in package.json. This field allows dual publishing CJS and ESM.

---

Prior to TypeScript 4.7, TypeScript did not support dual publishing. They added an new "moduleResolution": "node16" option to accommodate this. As a shortcut, it’s also possible to use "module": "node16", which sets a couple more options. Also the .cts and .mts extensions were introduced to represent .cjs and .mjs files. This is a huge step forward for ESM and dual publishing support for TypeScript and the npm ecosystem as a whole.

This new module resolution looks at the package.json fields "type" and "exports". Just like the "type" field is used how .js files are interpreted by Node.js, it’s also used to determine how .ts are interpreted by TypeScript.

If a module contains "type": "module" in package.json, this tells TypeScript that .d.ts files are a module. This means the package can’t be consumed using CJS. However, packages can dual publish using the "exports" field. Since there are runtime entry points for ESM and CJS, there also need to be TypeScript entry points for ESM and CJS.

Now I believe that if a package claims to support CJS, and it claims to support TypeScript, it should also support using it with the combination of CJS and TypeScript. That’s what this PR did, it added a CJS entry point for TypeScript.

Module	ESM	CJS
runtime code	Supported	Supported
type definitions	Supported	Support added in this PR

---

Now I agree 100% with @DigitalBrainJS that supporting type definitions for both CJS and ESM is tedious. Unfortunately this responsibility comes with dual support. If someone could point me in a direction how types could be reused, I would love to hear about it.

Older versions of TypeScript don’t know about .cts files. So the .ts file can’t import from the .cts file.

CJS can’t import ESM, so the .cts file can’t import from the .ts file.

One solution is to remove "type": "module", so .js and .ts files are treated as CJS, and publish .mjs files. Personally I don’t love it, but it could allow some reusability of the types.

would move to DefinitelyTyped help bring down the overhead of maintaining both files? Also would typing the entire Axios project and going to a full TS approach remove the need for managing two sets of definitions?

No. This would move the burden and the discussion to the DefinitelyTyped repository. Since the goal of DefinitelyTyped is to provide correct type definitions, they would accept the types as I have provided them.

This would also be a breaking change which users won’t like. A package removing TypeScript support feels like a step backwards for most users.

Just like removing TypeScript support would remove the need to maintain duplicate type definitions, so would removing support for CJS or ESM. I’m not saying you should, but I am saying it’s an option if the type definitions are too much of a burden.

---

I see some value in the tests in https://github.com/axios/axios/tree/v1.x/test/module. I understand it’s valuable to test the CJS and ESM entry points. I don’t really see the point of the tests that are compiles from TypeScript first. As @DigitalBrainJS pointed out, these do not test the types. In fact, the ts-require-default and ts-require tests don’t even use the Axios types at all. They use const axios = require('axios'), which is not how require() should be used in TypeScript.

This doesn’t test any types.

And they shouldn't. These tests ensure that the Axios entry points are consistent and the module can be imported into the target environments, so the users can use it.

This means they are just variations to test the CJS runtime entry point. (package.json and tsconfig.json matter). They might as well be compiled with Babel or ESBuild.

It is important to note TypeScript is not just a build tool. It is also a type checker for both TypeScript code and JavaScript code. To provide TypeScript support, means to provide type definitions.

---

Now for a practical example, let’s create a new project. This example deliberately doesn’t use .js or .ts files, because whether they are ESM or CJS depends on the "type" property in package.json.

// package.json
{
  "dependencies": {
    "axios": "1.1.3",
    "typescript": "^4.9.3"
  }
}

// tsconfig.json
{
  "compilerOptions": {
    // Tell TypeScript to also validate JavaScript files.
    "checkJs": true,
    "module": "node16"
  }
}

// mts.mts
// Uses the ESM types
import axios_mjs from 'axios'

// Uses the CJS types
// Compiles to:
// import { createRequire as _createRequire } from "module"
// const __require = _createRequire(import.meta.url)
// const axios_cjs = __require("axios")
import axios_cjs = require('axios')

// Uses the ESM types
const axios_mjs_dynamic = await import('axios')

axios_mjs('/')
axios_cjs('/')
axios_mjs_dynamic.default('/')

// cts.cts
// Uses the CJS types
// Compiles to:
// const axios_cjs = require("axios")
import axios_cjs = require('axios')

// Uses the ESM types
import('axios').then((axios_mjs) => {
  axios_mjs.default('/')
})

axios_cjs('/')

// mjs.mjs
// Uses the ESM types
import axios_mjs from 'axios'

// Uses the ESM types
const axios_mjs_dynamic = await import('axios')

// cjs.cjs
const axios_cjs = require('axios')

// Uses the ESM types
import('axios').then((axios_mjs) => {})

Now run npm install to install TypeScript and Axios.

From the project root, run tsc --noEmit true to check types. Note how this yields errors for all CJS use cases.

$ npx tsc --noEmit true
cjs.cjs:1:27 - error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("axios")' call instead.

1 const axios_cjs = require('axios')
                            ~~~~~~~

cts.cts:2:28 - error TS1471: Module 'axios' cannot be imported using this construct. The specifier only resolves to an ES module, which cannot be imported with 'require'. Use an ECMAScript import instead.

2 import axios_cjs = require('axios')
                             ~~~~~~~

cts.cts:9:1 - error TS2349: This expression is not callable.
  Type 'typeof import("node_modules/axios/index", { assert: { "resolution-mode": "import" } })' has no call signatures.

9 axios_cjs('/')
  ~~~~~~~~~

mts.mts:5:28 - error TS1471: Module 'axios' cannot be imported using this construct. The specifier only resolves to an ES module, which cannot be imported with 'require'. Use an ECMAScript import instead.

5 import axios_cjs = require('axios')
                             ~~~~~~~

mts.mts:11:1 - error TS2349: This expression is not callable.
  Type 'typeof import("axios/index", { assert: { "resolution-mode": "import" } })' has no call signatures.

11 axios_cjs('/')
   ~~~~~~~~~


Found 5 errors in 3 files.

Errors  Files
     1  cjs.cjs:1
     2  cts.cts:2
     2  mts.mts:5

Now update Axios, so the types from this PR are included.

npm install axios@1.2.0-alpha.1

Now run the type checker again to notice the type erros are gone

$ npx tsc --noEmit true

---

We have to realize here what percentage of users will be affected by the lack of typings for the commonjs module, the possible workarounds for users, and the reasonableness of solving this problem.

I would like to point to this chart, taken from wooorm/npm-esm-vs-cjs.

According to this chart, 77.4% of the most popular packages in the npm ecosystem use CJS (including dual). Now I think Axios has quite a lot users who create websites or backends. Those projects probably use ESM or faux ESM. These are not represented here. Still, a lot of people use CJS.

Also TypeScript 4.7 is relatively new. "module": "node16" is the correct setting for Node.js projects, but many people are not aware of this yet or are blocked by their dependencies not supporting this yet. Most people don’t fully understand modules in the first place and blindly accept any advice to make TypeScript shut up, without understanding what it actually means.

For example: “Just enable "esModuleInterop"”, “Just use const foo = require('foo')”, or “Just add a @ts-ignore comment”.

I believe Axios, being a popular package, can really help the ecosystem move forward by doing it right.

[arthurfiorette]

Hello!

Probably this PR was the one who introduced a type error into arthurfiorette/axios-cache-interceptor#778 related to ESM/CJS typings.

I know this issue should only be related to axios itself, but any help would be appreciated.