Incorrect support of `exports` in `package.json` when generating CommonJS files #54263

qraynaud · 2023-05-16T14:32:13Z

Bug Report

🔎 Search Terms

ts1479
exports
package.json

🕗 Version & Regression Information

This is the behavior in every version I tried, and I reviewed the FAQ for entries about 5m

💻 Code

Very simple codebase:

import { divide, round } from "mathjs"

Using the following tsconfig.json :

{
  "compilerOptions": {
    "moduleResolution": "nodenext",
    "module": "commonjs",
  }
}

🙁 Actual behavior

Typescript generates the following error message:

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("mathjs")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field "type": "module" to '/path/to/package.json'.

🙂 Expected behavior

This should work. The important parts of the package.json file are:

{
  "type": "module",
  "main": "./lib/cjs",
  "types": "./types/index.d.ts",
  "exports": {
    ".": {
      "types": "./types/index.d.ts",
      "import": "./lib/esm/index.js",
      "require": "./lib/cjs/index.js"
    },
  } 
}

Here is the reference package.json.

Even though the root directory of this file is marked as "type": "module", it has a require exports. And this require exports point to a file in a directory containing a package.json looking like:

{
  "type": "commonjs"
}

Thus, it is a perfectly valid module to be required.

I’m pretty sure this should work. node requires this module perfectly well without giving an error.

Note: remove "type": "module", from the package.json using a patch fix the issue, but it is a real bother to maintain a patch for each module using this structure (there are a lot more out there).

The text was updated successfully, but these errors were encountered:

Andarist · 2023-05-16T16:22:51Z

TypeScript won't understand that you have the require condition pointing to a file in a different directory.

You are telling it to always use /types/index.d.ts regardless of what loader is being used (cjs vs esm). This is essentially equivalent to ./types/index.js and thus this ".js" file is interpreted according to the nearest package scope. However, that is not defined by ./lib/cjs/package.json, nor by ./types/package.json, it's finding ./package.json and that has { "type": "module" } in it.

package.json#exports are always matched from the top to the bottom (it's an order-sensitive field despite being defined by JSON object). TypeScript reaches the types condition first and doesn't even look "past it".

qraynaud · 2023-05-16T16:25:50Z

@Andarist I understand what you are saying. But it’s not sensitive to use the types definition to know if a module is loadable using require or not. Even applying type to a .d.ts file is not making any sense. Those file are totally agnostic to the module type.

Plus I’m not those package maintainers. There are dozens out there that have similar looking package.json. And those makes sense. It’s a logical mistake to make.

qraynaud · 2023-05-16T16:36:38Z

Maybe the simpler choice would be to remove ts1479 altogether from the codebase. It’s not very useful to check this in TS since node do at runtime. It’s always nice to have more checks to prevent mistakes. But maybe this time it’s a little too involved a check to make?

Andarist · 2023-05-16T17:18:44Z

Those file are totally agnostic to the module type.

I'm afraid that's not exactly true. Some things might actually be allowed in one module system while not being allowed in the other one. For example, this is an error in the ESM file because it corresponds to module.exports =:

// @moduleResolution: node16
// @module: esnext

export = number

There are probably more subtle differences that I can't list on the spot from the top of my head. Andrew probably could elaborate on this but the bottom line is that - the module format is important for the declaration files.

Plus I’m not those package maintainers. There are dozens out there that have similar looking package.json. And those makes sense. It’s a logical mistake to make.

I hear you here. People jumped the gun on adding types condition to their packages and many have done that incorrectly. That doesn't make their setup correct though.

I hate the current situation as well but it originates outside of TypeScript. TypeScript just adheres here to rules created by other standards and runtimes. It has to offer certain flexibility to make valid runtime setups typeable.

RyanCavanaugh · 2023-05-16T18:50:54Z

There's a practical upper limit on how much we can choose to ignore manifest misconfigurations, of which this is definitely one. The real problem here is that all of this behavior is transitive, and there's also no guarantee that the types returned by the types field are even the same as what you would see in the exports map if you got there. So the only way we could really handle this in a way that would totally hide all these errors is a sort of combinatoric explosion of all the different ways that you might import things and the alternate ways that maybe the package maintainer meant to do. That's not really feasible. The best we can do is just believe the configuration that's given to us and tell you what's wrong, which I think is what's happening.

qraynaud · 2023-05-17T07:14:08Z

What you are telling me is new to me.

I don’t understand much how this can be possible. This is really weird in the sense that you can only specify one types file for both the ESM an CJS modules (at least I don’t see how you could provide 2). Noone would know you should export 2 types files for the same projet, 1 for ESM and 1 for CJS. And there is no serious example anywhere about how this should be done…

It still doesn’t make any sense to me. I’m still thinking that this error should try to find a valide JS file using the same algorithm node uses, ignoring the typings for this.
Or maybe I'm still missing something there?

Thanks @Andarist and @RyanCavanaugh for your involvment in this though.

qraynaud · 2023-05-17T07:18:49Z

And after thinking this through, it also seems to me that it means it’s impossible to write properly a package.json for a project that is module first for which we want to add support for old nodes through a compilation process. It would always get this error in the end. Or is there a way to fix this package.json to achieve this? I don’t see any…

I thought about adding a { "type": "commonjs" } file in the types folder but it should break the module version instead. One of them is doomed.

Wouldn’t it more sensitive to assume that the types file is compatible with everything that is exported? It’s sensitive to think that if the same module is exported both for CJS and ESM with the same types for both, then the d.ts is compatible for both too.

Andarist · 2023-05-17T08:47:11Z

This is really weird in the sense that you can only specify one types file for both the ESM an CJS modules (at least I don’t see how you could provide 2).

This is totally valid and possible:

{
  exports: {
    import: { types, default },
    require: { types, default }
  }
}

On top of that, you could have lots of nested conditions like:

{
  exports: {
    development: {
      browser: {
        import: { types, default },
      },
    },
  },
}

As you can see, there is truly no 1-to-1 mapping between the files targeting runtime and type-level. You can mix and match this however you want and it's all order-dependent. For example, you could move the types to the front in the example above:

{
  exports: {
    types,
    development: {
      browser: {
        import,
      },
    },
  },
}

Also, note that the types condition is optional. You can rely on "the sibling lookup". If you put ur .d.ts files next to .js files (or .d.mts next to .mjs etc) and if they basenames would match then TS is capable of resolving types like this (as it has always been).

Wouldn’t it more sensitive to assume that the types file is compatible with everything that is exported? It’s sensitive to think that if the same module is exported both for CJS and ESM with the same types for both, then the d.ts is compatible for both too.

It's also not that simple because some things might be nominal - like classes with private fields. This could affect narrowing through instanceof and overall assignability rules. If you try to ship your runtime code twice then it only makes sense to do the same at type-level as well. While it's reasonable for your require and import interfaces to match... they don't have to in practice (i'd call that a code smell but that's beside the point for the conversation here).

qraynaud · 2023-05-17T08:51:12Z

@Andarist oops, I suppose I was unclear about my interrogations. I was talking about keeping support for "moduleResolution": "classic" + old node while making the new system first class. Right now, if I’m understanding properly, using "moduleResolution": "nodenext" still uses the root types by default, ignoring exports… So it is basically useless to specify an exports section since it will be ignored if one wants to keep support for the old mechanism (this is totally unexpected behavior, node prioritize exports).

And even in "moduleResolution": "classic", wouldn’t this fail for either CommonJS or ESM modules since you can only have one types section for both module + commonjs, depending on which type you choose?

Andarist · 2023-05-17T09:03:26Z

Right now, if I’m understanding properly, using "moduleResolution": "nodenext" still uses the root types by default, ignoring exports…

No, if your package has exports then package.json#types should be ignored when using moduleResolution: nodenext/node16/bundler

So it is basically useless to specify an exports section since it will be ignored if one wants to keep support for the old mechanism (this is totally unexpected behavior, node prioritize exports).

Ye, this is also a little bit weird to me that all other runtimes made exports support a default thing - one that u probably can't even disable in many of them. In TypeScript it became an opt-in. I guess that the decision here was based on the fact that it would be quite disruptive for the ecosystem if exports would be handled in moduleResolution: node but right now it only adds to the confusion that it's not handled in that mode. People often don't test against moduleResolution: node16 and ship packages that are broken with it.

And even in "moduleResolution": "classic", wouldn’t this fail for either CommonJS or ESM modules since you can only have one types section for both module + commonjs, depending on which type you choose?

Yes, kinda - the problem here is that with such a setting and a dual package the runtime world doesn't really reflect the type-world and the whole thing is in some weirdo, hard-to-reason-about, state. From TypeScript PoV, with moduleResolution: node/classic your package is always of a single "type" (well, quite frankly - I'm not even sure how classic works, i never used it)

qraynaud · 2023-05-17T09:09:04Z

No, if your package has exports then package.json#types should be ignored when using moduleResolution: nodenext/node16/bundler

Strange, I'm pretty sure I've ran into issues when keeping it in this mode.

One last point before I close this, it means you can’t really use this syntax then:

{
  exports: {
    types,
    ...
  },
}

It’s presented everywhere in the doc and it fails. Why not make it so that it means : "I assume the fact that this type file is correct for all type of packages in the following conditions and assume the responsibility if it does not"? It would be so much easier to work with…

fatcerberus · 2023-05-17T16:43:36Z

It's generally impossible for a single type file to work correctly with both ESM and CJS, because the semantics of how exports work differ: CJS actually just exports a single value (i.e. what require() returns) and named exports are emulated by adding properties to the exported object; ESM on the other hand has completely separate default and named exports. You can't write a single type definition that completely satisfies both situations.

microsoft-github-policy-service · 2023-05-19T17:24:22Z