Repository navigation
Should type-only import resolving to ES module from CJS module be allowed? #52529
Description
Activity
- changed the title
[-]ESM import mistakenly reported as prospect requires when importing types[/-][+]Should type-only import resolving to ES module from CJS module be allowed?[/+]on Feb 1, 2023 Yes, please.
This makes interoperability a mess for module maintainers (like me) and I would like user not to have to use@ts-expect-erroreverywhere.Is it hard to suppress the error when doing
import typeonly?No, it’s not hard, we just have to be sure it makes sense and doesn’t have any other unintended consequences.
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.
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.Arent classes with private members nominal?
Reacted by Bruce Pascoe and Andrew BranchOh, good point; yes, they are. Not sure how I forgot about that!
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>>;
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) 🤔.
Just want to note that this is also true for
import()used in JSDoc, see:
5 remaining items
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 usetypeof 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.
Reacted by David Tanner and Henry ChanYeah. This is what Edward Faulkner (@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.
Thank you Andrew Branch (@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?
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:
- Those that represent some construct that exists in the corresponding JS file
- 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 node16ornodenextare 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 ofnode16andnodenext.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
Reacted by Andrew Branchopenpgp(CJS) currently imports types from@openpgp/web-stream-tools(ESM), usingimport 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-errorseems 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.tsfiles.#53656 is the solution to this problem, which is on the 5.3 iteration plan.
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 usetypesVersions.Andrew Branch (@andrewbranch) I still got a question. The rationale behind import attributes is security. If I use explicit
import typethere 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?Reacted by KittenWithHerbsSecurity 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
Reacted by StriderIs there a solution/workaround for this in the meantime?
Hahnbee Lee (@hahnbeelee) basically
@ts-expect-errorbefore the import. No error, while still having types imported.Reacted by cyan and Cristiano AguzziHahnbee Lee (@hahnbeelee) basically
@ts-expect-errorbefore the import. No error, while still having types imported.This works, thank you!
#53656 is the solution to this problem, which is on the 5.3 iteration plan.
5.3 has now been released. What's the next step toward a more permanent solution for this issue?
Bug Report
🕗 Version & Regression Information
I observed this in TypeScript 4.9.x, couldn't test earlier versions.
💻 Code
tsconfig.json:
package.json:
index.ts:
🙁 Actual behavior
Error reported:
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.