Should type-only import resolving to ES module from CJS module be allowed?

[ShogunPanda]

Bug Report

🕗 Version & Regression Information

I observed this in TypeScript 4.9.x, couldn't test earlier versions.

💻 Code

tsconfig.json:

{
  "compilerOptions": {
    "allowJs": true,
    "target": "ESNext",
    "module": "CommonJS",
    "outDir": "dist",
    "lib": ["ESNext", "DOM"],
    "esModuleInterop": false,
    "declaration": true,
    "forceConsistentCasingInFileNames": true,
    "strict": true,
    "skipLibCheck": true,
    "resolveJsonModule": true,
    "sourceMap": true,
    "moduleResolution": "nodenext",
    "baseUrl": "."
  },
  "include": ["index.ts"]
}

package.json:

{
	"dependencies": {
		"got": "^12.5.3",
		"typescript": "^4.9.5"
	}
}

index.ts:

import type { Headers } from 'got'

const foo: Headers = {}
console.log(foo)

🙁 Actual behavior

Error reported:

index.ts:1:30 - 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("got")' call instead.
  To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Volumes/DATI/Users/Shogun/Programmazione/Test/got/package.json'.

1 import type { Headers } from 'got'
                               ~~~~~


Found 1 error in index.ts:1

Note the got is just a quick repro example. It happening with any ESM module.

🙂 Expected behavior

Since I'm just doing import type, the emitted file has no requires at all and thus the error is unnecessary.

[andrewbranch]

This is a duplicate of #46213, just with an updated error message, but I don’t think the analysis that led us to close #46213 was correct. It’s probably worth reconsidering this.

[ShogunPanda]

Yes, please.
This makes interoperability a mess for module maintainers (like me) and I would like user not to have to use @ts-expect-error everywhere.

Is it hard to suppress the error when doing import type only?

[andrewbranch]

No, it’s not hard, we just have to be sure it makes sense and doesn’t have any other unintended consequences.

[Andarist]

One potential risk is that nominal types could accidentally leak into the project in two versions - one coming from the CJS types and one from the module types. This wouldn't exactly be a new risk but this could make it more common.

I'm not saying that this should be a blocker - being able to use types from modules in CJS type definitions would solve some of my problems. It is an important consideration though.

[fatcerberus]

Isn’t the only truly nominal type in TS unique symbol? afaict all other methods to emulate nominal types in TS (e.g. branded primitives, deferred conditional types, etc.) are still more or less structural insofar as if you define the type twice it will refer to the same type in both places.

[Andarist]

Arent classes with private members nominal?

[fatcerberus]

Oh, good point; yes, they are. Not sure how I forgot about that!

[ef4]

Even if one follows the instructions in the error message and tries to use dynamic-import-type syntax, the error persists:

type Terser = typeof import('terser');
// 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("terser")' call instead.

The only workaround I've found is to write a real function that does the dynamic import and infer the types off of that:

async function loadTerser() {
  return await import('terser');
}
type Terser = Awaited<ReturnType<typeof loadTerser>>;

[andrewbranch]

The instructions are telling you to write the latter because the error message doesn’t realize you were writing a type-only import to begin with. But something is still weird with that error being issued on an ImportType (the former) 🤔.

[voxpelli]

Just want to note that this is also true for import() used in JSDoc, see:

[fatcerberus]

The instructions are telling you to write the latter because the error message doesn’t realize you were writing a type-only import to begin with. But something is still weird with that error being issued on an ImportType (the former) 🤔.

For the record, in CommonJS files, even if you write a dynamic import, it will still be transformed to a require() leaving you in the same boat. The suggestion just seems misleading all the way around.

[Andarist]

That doesn't sound correct, perhaps it's sensitive to other compiler options related to target and stuff but with this input:

// @module: commonjs
// @moduleResolution: node16

export const bar = 1

import('foo').then(a => {})

I get this CJS output:

"use strict";
// @module: commonjs
// @moduleResolution: node16
Object.defineProperty(exports, "__esModule", { value: true });
exports.bar = void 0;
exports.bar = 1;
import('foo').then(a => { });

TS playground

[fatcerberus]

Ah, it works with moduleResolution node16, good to know. But the default is node.

[andrewbranch]

Hold up, the module resolution mode affects the emit here?

[fatcerberus]

@andrewbranch Seems to - changing the moduleResolution to node in the code above causes the dynamic import to be emitted as a require.

[patrickshipe]

There are several issues that exist around this - I think the gist that I haven't seen written anywhere is this: dynamic imports of ESM code into a CJS module as supported by Node is essentially crippled in TypeScript at the moment, because you can't use types anywhere. The original await import() infers the appropriate type with no issue, but if you try to either import types directly from the module OR even use typeof import() you will get errors. So you essentially have this hot potato - if you can do everything you want with the ESM in the same code block you are golden, but if you want to create helper functions or what have you with appropriate typings, you are out of luck.

This will be more and more of an issue as developers only release ESM packages and not all codebases are in a place to switch 100% to ESM.

[andrewbranch]

Yeah. This is what @ef4 was saying above with the pretty bananas workaround of using a runtime dyanmic import and fishing out the types returned by it with conditional type helpers.

We’re pretty much putting all our eggs in the #53656 basket to fix this.

[patrickshipe]

Thank you @andrewbranch - I was just thinking about one clarifying point. Ideally, a CommonJS package that dynamically imports an ESM module and consumes the ESM modules' types should be able to be used in a CJS project without the CJS project caring at all that the dependency happens to dynamic-import an ESM. When testing this scenario myself, I had compilation errors in my CJS project if I didn't change "module" to node16. The CJS project shouldn't have to care about the dependency doing a dynamic import/importing types from an ESM, right?

[andrewbranch]

Ideally, a CommonJS package that dynamically imports an ESM module and consumes the ESM modules' types should be able to be used in a CJS project without the CJS project caring at all that the dependency happens to dynamic-import an ESM.

Kind of yes, kind of no? Ideally, there are two kinds of things in declaration files:

1. Those that represent some construct that exists in the corresponding JS file
2. Those that don’t

When you import any dependency, you have reason to care about everything in category (1), because your program options define what runtime constructs are available and what semantics they have. So, to take your example, if you import a CJS dependency, and the declaration file for that dependency indicates that there is a real (category 1) dynamic import of an ESM file, your program options better indicate that your runtime can handle a CommonJS file that contains a dynamic import of an ESM file, and your options likely also need to indicate what the semantics of that dynamic import are—e.g., how the import path is resolved to a file—so that types for the dynamically imported module can be properly acquired.

Things in category (2), like type-only imports, don’t have this property of needing to be scrutinized for runtime compatibility. They simply need to be coherent and unambiguous so your program can find the types that the author intended to reference.

This is the correct mental model to have in the abstract. In your specific example, I can’t think of a real, existing configuration that should have a problem. But the semantics of imports in your dependencies’ declaration files are very much affected by your own program settings, because imports in your dependencies’ implementation files are affected by your own runtime environment.

I had compilation errors in my CJS project if I didn't change "module" to node16

I don’t know what’s going on without seeing the errors, but --module node16 or nodenext are the only correct options if you’re running in Node.js, regardless of who is ESM and who is CJS. The compiler will not even really understand that there is such a thing as ESM and CJS outside of node16 and nodenext.

[patrickshipe]

Thank you for this response! That makes perfect sense. This is exactly what I needed to hear:

but --module node16 or nodenext are the only correct options if you’re running in Node.js, regardless of who is ESM and who is CJS

[hfhchan-plb]

openpgp (CJS) currently imports types from @openpgp/web-stream-tools (ESM), using import type, and is generating this error.

Adding assertions to openpgp is probably a no-go because it's a nightly feature, and they currently support down to node 8, so even if import assertions made it into an upcoming 5.3 release, it would still be a stretch to ask them to update. Adding // @ts-expect-error seems like a yucky solution as well.

No error shows up if I keep using compilerOptions: { module: 'commonjs' }.

I understand wanting to keep TypeScript's behaviour consistent with node, but this is making migration to ESM especially painful, to the point where it seems TypeScript is actively hostile to migrating to ESM, because otherwise, the code works perfectly fine.

Even if it is decided that an error should still keep showing up, at least it shouldn't mention anything about producing 'require' calls, because it isn't, nor should it suggest using a runtime dynamic import, especially in d.ts files.

[andrewbranch]

#53656 is the solution to this problem, which is on the 5.3 iteration plan.

[andrewbranch]

And yeah, aside from that, the error message needs to be overhauled. If they don’t want to break backward compatibility with old TS versions, they can use an import type (type GenericWebStream = import("...").WebStream) or use typesVersions.

[StreetStrider]

@andrewbranch I still got a question. The rationale behind import attributes is security. If I use explicit import type there is no security threats. It is named import, it is statically erased and no code execution involved (and may be even non-web platform). Would it be much easier to just relax rules here and allow this particular case?

[andrewbranch]

Security isn’t an issue here, but there are other reasons not to do it. I tried to relax the rules; the objections were discussed in the PR #53426

[hahnbeelee]

Is there a solution/workaround for this in the meantime?

[StreetStrider]

@hahnbeelee basically @ts-expect-error before the import. No error, while still having types imported.