FalseCJS problem - severity and suggested fixes?

[markerikson]

(I realize this is really more of a TS question than it is an attw question, but given our recent discussions I figured it was worth filing here. Feel free to close this once you see it.)

I'm working on redoing the redux-thunk build setup atm, and also trying out https://github.com/egoist/tsup as a potential build tool.

My current package setup looks like this:

  "main": "dist/cjs/index.cjs",
  "module": "dist/index.mjs",
  "types": "dist/index.d.ts",
  "exports": {
    "./package.json": "./package.json",
    ".": {
      "types": "./dist/index.d.ts",
      "import": "./dist/index.mjs",
      "default": "./dist/cjs/index.cjs"
    },
    "./extend-redux": {
      "types": "./extend-redux.d.ts"
    }
  },

Using the CLI wrapper I've set up for attw, I get this output:

I'm getting that FalseCJS warning, and I'm guessing it's because I have dist/index.mjs but dist/index.d.ts.

A few questions based on that:

• How critical is that mismatch in general? I know you've mentioned that "CJS and ESM output files can/should have different types". How much of a concern is that actually in practice? How much effort should I spend on trying to resolve this attw warning?
• tsup doesn't currently have a way to generate .d.mts output , per [feature] Generate esm-specific declaration files egoist/tsup#760 . Is it reasonable to copy over index.d.ts -> index.d.mts in this case?
• redux-thunk only has the one output index.d.ts file. For RTK, we have many source files, and thus many .d.ts files:
  • Does only the index.d.mts file need to exist with that extension (ie, there could be index.d.mts and otherFile.d.ts and that would work)? Or does every output declaration file need to have the .d.mts extension for TS to load those properly in the ESM case?
• We've briefly discussed trying to bundle up TS declarations. What is the right tool for that currently?

[andrewbranch]

How critical is that mismatch in general?

Have you read “Types are CJS, but implementation is ESM” in the README? I started writing a response and felt like I had written it before.

If the module has a default export, a default import from an ESM module will resolve to the wrong thing. Really bad. If the module doesn’t have a default export, a default import will still be (incorrectly) allowed from an ES module, binding to an object with all the named exports. Not good, but not expected API usage.

Is it reasonable to copy over index.d.ts -> index.d.mts in this case?

It’s not possible to know that a copy is safe without a type checker. See https://twitter.com/atcb/status/1635356255203229697 for some discussion. It will almost certainly not be safe if the source files didn’t use Node ESM-compatible module resolution (i.e. file extensions required). If they did, it might be reasonable to copy and then check for coherence with tsc?

Does only the index.d.mts file need to exist with that extension (ie, there could be index.d.mts and otherFile.d.ts and that would work)? Or does every output declaration file need to have the .d.mts extension for TS to load those properly in the ESM case?

Any .d.ts file in this situation is a CJS module, so if you only rename index.d.ts to index.d.mts, it will look like you have an ES module that imports a bunch of CJS modules. That is wrong, but will it be wrong in a way that people will typically notice? 🤷

We've briefly discussed trying to bundle up TS declarations. What is the right tool for that currently?

All I know is that tsup does it via a Rollup plugin, api-extractor has an implementation, and Parcel has an implementation. (I also think esbuild and swc may be interested in making one pending community consensus on isolatedDeclarations.) I don’t have direct experience with any of them. Please report your findings if you have good or bad experiences with them 😅

[markerikson]

Ah, no, didn't realize there were further explanations in the README.

That is wrong, but will it be wrong in a way that people will typically notice?

Yeah, that's sorta my question / gut feeling about these CJS/ESM types-vs-modules mismatches.

I think I understand what you're saying about the potential for problems. On the other hand, the ecosystem has apparently managed to get along so far without that being a "real" issue (although that may also be because the moduleResolution: "node16" setting just came out and hasn't had an impact yet). So, it feels like that may be one of those "wrong in theory, potentially not a problem in practice" kinds of things.

The potential fixes for that feel up in the air. I see you said "add an index.d.mts file". The question is how to get that:

• tsup doesn't do that or offer a way to configure the declaration output. It does apparently bundle the types.
• RTK is currently spitting out individual .d.ts files per source file, and not bundling the types. Seems like I'd need to generate an entire duplicate set of type declarations? And to do that I'd have to re-run tsc with a different moduleResolution or output setting of some kind?

[andrewbranch]

Seems like I'd need to generate an entire duplicate set of type declarations? And to do that I'd have to re-run tsc with a different moduleResolution or output setting of some kind?

Bingo. Somewhere in the Twitter thread I linked I described the most seamless way to do this, but it’s still admittedly a pain:

Ultimately, running tsc twice on .js files under nodenext, swapping the package.json type between runs, is the most foolproof way to generate an ESM/CJS pair of declarations.

I am starting to think about how TypeScript can help here. It’s not cool to say “every way of doing this that doesn’t use a type checker is wrong” and then say “yeah we’re not really designed to do dual emit.” 😬 But trying to finish some docs first.

[andrewbranch]

You can track microsoft/TypeScript#54593 for proper dual emit with TypeScript.