Silence “Cannot require an ESM file” error in type-only imports (and ImportTypeNodes)

[andrewbranch]

Closes #52529

In Node, when a require call resolves to an ES module, loading/evaluation will crash because ES modules cannot be loaded synchronously. TypeScript issues a corresponding checker error whenever an import/require statement is written that would result in such a require of an ES module.

However, this checker error was also issued in cases where no require call is emitted at all, such as in type-only imports (import type { T } from "./mod") and import types (type T = import("./mod").T).

The error is only important insofar as it protects against a runtime error in Node, so I don’t think there’s any reason to issue it on constructs that are clearly and unambiguously non-emitting.

Why an ambient context alone does not silence the error

In my first pass at this, I also silenced the error on regular import statements that occur in declaration files, since declaration files themselves do not emit to JS. However, regular (non-type-only) import statements in declaration files ideally represent similar import statements in a JS implementation counterpart, so a user who includes a declaration file containing a suspicious import statement like that probably has reason to believe that the corresponding implementation has a problem, and should be warned about it.

Contrast to resolution-mode

The effect of this PR should not be confused with the effect of the (nightly-only) resolution-mode import assertion. Silencing this error allows CommonJS files to access types from ESM files, but only in cases where the CommonJS resolution algorithm resolves the module specifier to an ESM file, as would happen when writing an import of an ESM-only package—this PR does not change the result of any module resolutions. The resolution-mode import assertion, on the other hand, actually changes which resolution algorithm is used.

[weswigham]

there’s any reason to issue it on constructs that are clearly and unambiguously non-emitting.

Instead, they make adding formats a breaking change where they otherwise aren't. To begin. Suppose we have

// @filename: node_modules/mypkg/package.json
{ "name": "mypkg", "type": "module", "exports": "./main.js" }
// @filename: node_modules/mypkg/main.js
export default "whatever";
// @filename: node_modules/mypkg/main.d.ts
const __default: string;
export default __default;
// @filename: usage.ts
// do note, without a package.json, this file is cjs
import("mypkg").then(p => console.log(p.member)); // JS is all good, cjs `import` resolves like esm
interface MyThing {
  x: typeof import("mypkg").default // currently an error (this `import` resolves like a `require`), in this proposal not an error
}

if this PR is in place, this is not an error. OK. Now suppose mypkg does the supposedly backwards-compatible thing and adds an explicit cjs entrypoint of the same shape, to help its' consumers out:

// @filename: node_modules/mypkg/package.json
{ "name": "mypkg", "type": "module", "exports": { "import": "./main.js", "require": "./main.cjs" } }
// @filename: node_modules/mypkg/main.js
export default "whatever";
// @filename: node_modules/mypkg/main.d.ts
declare const __default: string;
export default __default;
// @filename: node_modules/mypkg/main.cjs
module.exports.default = "whatever"; // output from transforming the esm downlevel
// @filename: node_modules/mypkg/main.d.cts
declare const __default: string;
export default __default;
// @filename: usage.ts
// do note, without a package.json, this file is cjs
import("mypkg").then(p => console.log(p.member)); // JS is all good, cjs `import` resolves like esm
interface MyThing {
  x: typeof import("mypkg").default // meaning suddenly changes! now resolves to the cjs entrypoint, rather than the esm one!
}

If we did this, we make what should be a non-breaking, safe change into a breaking change for TS consumers. That won't be good for the ecosystem. :(

[Andarist]

Are there any caveats related to this in relation to the dual package hazard? Can a type from an inaccessible ESM module leak into the module graph and end up conflicting with its CJS counterpart? It seems somewhat unlikely since, after all, CJS isn't available in this situation... Although, there is probably a convoluted test case to be created here that would involve an ESM-only root entry and dual subentry?

[andrewbranch]

You can always find a way to make adding a format a “breaking change.” Take the reverse scenario: you have an ESM file and a CJS dependency:

// @Filename: main.mts
import ts from "typescript";

Then TypeScript adds an ESM entrypoint. Suddenly the resolution of the import changes! This scenario is way more realistic since it involves a plain old import declaration of a value instead of an import type, and an addition of ESM to existing CJS instead of the reverse (most people who have decided to ship an ESM-only package are not being convinced to add CJS at this point).

It doesn’t make a lot of sense to error on one of these scenarios and not the other, but I don’t see anyone suggesting that we should raise an error every time a (type-only, even) ESM import resolves to a CJS file. Resilience against the structure of your dependencies changing when you update them is not really an invariant we have; just this one corner happened to be guarded against it by a too-conservative error message. And I maintain that the sequence of events that would have to happen to cause a real problem, where it would have been an error before, will be exceedingly rare.

@Andarist the type-level dual package hazard can be affected by using resolution-mode generally, or even just having CJS and ESM files and re-exporting stuff you import in them. Same as the value-land concern, really. I don’t think this PR moves the needle one way or the other on it.

[Andarist]

@Andarist the type-level dual package hazard can be affected by using resolution-mode generally, or even just having CJS and ESM files and re-exporting stuff you import in them. Same as the value-land concern, really. I don’t think this PR moves the needle one way or the other on it.

Hm, I feel like it might move that needle a little bit - as the user might not be aware that at a given moment TS "silently" allows them to access ESM-only files at type-level. I agree that the dual package hazard can always happen in convoluted scenarios but at least in those situations the runtime-level hazard and type-level hazard stay kinda the same as people are only allowed to import/require things from modules of a given type. This introduces a new subtle way to end up in this situation.

I don't necessarily mean that this is a bad thing - perhaps the DX improvement of this change is worth it. I remember my own initial reaction when wanting to import a type from ESM into CJS... I wanted this to be possible :P I think though that the needle is moved here - how big of a move it is? Dunno 🤷

[weswigham]

It doesn’t make a lot of sense to error on one of these scenarios and not the other, but I don’t see anyone suggesting that we should raise an error every time a (type-only, even) ESM import resolves to a CJS file

That's because node acknowledges swapping a cjs dependency to esm almost always is a breaking change and should almost always be a major version bump (because, minimally, it changes the shape of your default exports)? Nobody needs an error on it because the community expects it to break and (mostly) versions appropriately. Migrating from cjs to esm (or just adding esm) can often be a breaking change, and should be done in a major revision, while adding cjs to an esm only package should basically never be a break (since you never had any functioning cjs consumers), so can be shipped in a patch release.

[andrewbranch]

I see your point, I just don’t think it’s worth the poor DX we have today. When we issue an error on some code, it’s generally expected that the code in question is wrong or dangerous at runtime (or creates a type-level inconsistency that could allow wrong or dangerous runtime code to type check down the line), such that fixing it would be worth the user’s time. Erroring on code that might change meaning if they take an explicit action in the future (at which point we could error again if it creates a problem!) is clearly not part of what users think they’re signing up for when they run our type checker. It sounds like a reasonable lint rule to me. For us to justify erroring (with no workaround, for non-nightly users!) over a hypothetical future problem, we’d have to think that problem is quite big and/or likely, and I don’t see any evidence that it is. How many npm packages can you name that shipped ESM first and added CJS later?

[weswigham]

Here's a big question, though: Who's referencing the types of an esm-only package in cjs, but not its' implementation? If you think "well, this is useful alongside dynamic import in cjs, maybe", as the dynamic import will follow esm resolution rules and esm conditions, while the type-only imports will still be following cjs ones... won't that just be confusing? Basically all the reports I've seen have been from people trying to solve our compiler error saying that the type isn't reachable to a non-esm-format import, and trying to make that work without an actual esm-format-searching-import is riddled with pits of failure. Like, yeah, so long as there's no cjs implementation alongside, with this we'd suddenly find something, but when there is? Error's still there. This adds a lot of potential ecosystem burden on upgrades and doesn't seem like a complete or reliable solution to the general problem of reaching esm format types from cjs (which, again, 90% of the time is only a problem because we say it is - if we, say, choose to generate resolution-mode imports in declaration emit, despite how they're not stable yet, this problem would be invisible to most people, since they'd never get the un-nameable type error in the first place)...

[andrewbranch]

@ShogunPanda @ef4 @Andarist have all expressed that they’ve tried to do this; can any of you elaborate on your use case a bit?

[Andarist]

I tried to create a declaration file for https://github.com/gxmari007/vite-plugin-eslint . It's a plugin for Vite and Vite is esm-only. However, Vite is able to load CJS plugins and that plugin annotates its return value with import('vite').Plugin to get contextual typing and validation. This one only relies on structural typing (and possibly on duck-typing in Vite itself) and it seems to be OK.

Later I've thought about cases like:

// .d.cts
import type { Nominal } from 'esm-only'
export declare function accept(input: Nominal): void

and at that point, I lost my confidence in being able to do this a good thing. OTOH, you could have an API like that 🤷 it's possible to use a dynamic import in CJS to load ESM files. How that should be typed?

[ef4]

Who's referencing the types of an esm-only package in cjs, but not its' implementation?

I'm using its implementation sometimes. Simplified example:

import type { MinifyOptions } from 'terser';

export async function build(code: string, minify?: MinifyOptions) {
  if (minify) {
    let terser = await import('terser'); 
    code = terser.default.minify(code, minify);
  }
  return code;
}

The type is part of my type, the implementation should only load when needed.

Consider also that this pattern can be used to guard code that doesn't even eval in the current environment, so using the dynamic import() over static import is not just an optimization, it can be necessary for correctness.

[weswigham]

I'm using its implementation sometimes.

Great! And via dynamic import, too! Wouldn't it be odd that the only way to access the types of said dynamic import were via a lookup that doesn't follow the same lookup rules as said dynamic import actually has, and, in fact, may in all likelihood still be unable to access those types?

[ef4]

I also tried accessing those types directly through the same mechanism. That is, surely typeof import('terser') should follow the same rules as import('terser'):

type Terser = typeof import('terser');

But this fails in the same way as the static import. Even though it's clear that TS can locate the types, because this workaround actually works:

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

[andrewbranch]

That is, surely typeof import('terser') should follow the same rules as import('terser')

Tragically, it does not, for backward-compatibility reasons. The former syntax existed in TypeScript and had CJS-y resolution semantics long before Node decided dynamic import in CJS files would have ESM-y resolution semantics.

[ShogunPanda]

In my case I was just importing a type (an interface, to be precise) in the file and then write some code. Nothing else.

The problem was that I must use CJS while the referenced module is ESM.

[ef4]

Tragically, it does not, for backward-compatibility reasons.

If the syntax is claimed already then oh well. But what is the recommended workaround then? I can keep using a function that I never call just to infer the type off of:

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

but maybe there is something else?

[ef4]

Actually I guess I should ask: does the type inference example above actually follow the same resolution rules as the implementation would? I hope yes. If not, that seems WAT.

[andrewbranch]

But what is the recommended workaround then?

There is no recommended workaround, hence this PR. The only workaround that exists is currently only available in nightly versions of TypeScript, because it reuses some syntax in type-space that TC39 is still sorting out the meaning of in value-space, and there was significant enough pushback when we adopted it that we made it unavailable outside of typescript@next. So yeah, unless you’re using nightly, there is no other workaround, at least until the future of import assertions / import reflection is known.

Actually I guess I should ask: does the type inference example above actually follow the same resolution rules as the implementation would?

Yes.

[weswigham]

reuses some syntax in type-space that TC39 is still sorting out the meaning of in value-space

The meaning is less important than the syntax 😂

There's consensus to loosen the spec restrictions that we got a complaint the we were "violating" with resolution-mode already - it'd be allowed at runtime according to the new version, so that complaint definitely doesn't hold water anymore. The issue is the syntax changes paired with the relaxation. T.T

TBH, I think we can un-nightly resolution-mode specifically at this point, but we need to go back to erroring on all assertions (nee attributes) until the proposal advances again (and parse with besides)...

[andrewbranch]

The proposal reached conditional Stage 3 the day after I made my comment, so we may be able to just move forward with with soon. Once that’s the case, I’m fine to redo this PR as changing the error messages instead of silencing them. Right now they’re incredibly confusing for import types:

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.

[andrewbranch]

I’m guessing/hoping we’ll be able to land import attributes fairly soon, so I’m going to close this

[slorber]

@andrewbranch just wondering why closing this issue if it's not already implemented 😅 I'd like to be notified once the solution is out personally.

@RyanCavanaugh wasn't as confident it will land soon (#53656 (comment)), anything changed recently regarding this proposal?

Given the Stage 3 -> 2 regression on the original proposal, we might hold off on this until it's more like Stage 3.5

---

When you mean import attributes in the context of this issue's problem, you mean something like this?

import type { Plugin } from 'unified' with { "resolution-mode": "import" };

Related links others might find useful:

• Import Attributes (Stage 3) #53656
• Allow ES Module Type-Only Imports from CJS Modules #49721

[andrewbranch]

The proposal has been progressing positively: tc39/proposal-import-attributes#137. You should subscribe to the other issues you linked to be notified. This PR was an experiment for sidestepping the need for import attributes altogether and it failed.