Ensure matching declaration file exists for each output bundle format

[andrewbranch]

👋 Hi, I work on the TypeScript team on module-related stuff. I’ve been researching problems with typings in npm packages for a long time, and made https://arethetypeswrong.github.io to help. I’ve noticed that packages generated with tsup often report the FalseCJS or FalseESM problems. If you read the explanations there, it should be clear why:

• using a single declaration file to represent more than one JS module is always a problem;
• declaration file extensions must match the JS file extension (.mjs → .d.mts etc.);
• but tsup only ever produces one declaration file, always with a .d.ts extension, even when multiple outputs with different extensions are generated.

Solving dual emit in a type-safe way is a very hard problem. As you know, tsup doesn’t even know if it’s producing safe runtime code due to module format incompatibilities (#628, #752). With enough effort (and correct typings to rely on), TypeScript can detect these problems, but it can’t really solve them. Tsup, on the other hand, cannot detect them, but can solve them by bundling ESM-only dependencies via noExternal/devDependencies. It would be nice if a tool like tsup could leverage TypeScript to verify that a given output format is going to be valid and produce a guaranteed correct declaration file for that format, but it’s not really possible with the way tsup bundles things and relies on rolling up the declaration files.

This PR is not perfect, but for adding just a few lines, it should produce valid TypeScript declarations much more often. Since the declarations are not guaranteed to compile, I suggested performing additional checks with arethetypeswrong in the README. Basically, all this does is

• generate one declaration file bundle per output format
• ensure the declaration file extension always matches the JS file extension

You can read more in-depth explanation of the way TypeScript treats modules and declaration files at https://gist.github.com/andrewbranch/79f872a8b9f0507c9c5f2641cfb3efa6.

What do you think?

[codesandbox]

This branch is running in CodeSandbox. Use the links below to review this PR faster.


  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders | Preview

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
tsup	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 22, 2023 7:13pm

[github-actions]

🎉 This PR is included in version 7.1.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[andrewbranch]

Thank you @egoist! Excited about this 😄

[WORMSS]

{
  "entry": ["src/index.ts"],
  "format": ["cjs", "esm"],
  "dts": true,
  "clean": true
}

Only needs 1 .d.ts file.. The types doesn't change from cjs to esm.

{
  "type": "module",
  "main": "./dist/index.cjs",
  "module": "./dist/index.js",
  "types": "./dist/index.d.ts",
  "scripts": {
    "build": "tsup"
  },
  "devDependencies": {
    "@types/node": "latest",
    "tsup": "<7.1.0",
    "typescript": "latest",
  }

[andrewbranch]

Perhaps unfortunately, this isn’t how TypeScript works. Declaration files that represent modules are understood by the compiler to be 1:1 to JS files, and have a file-level module kind that represents whether it represents an ES module or CJS module. So even if the contents of the files are the same, the compiler needs to see two separate declaration files with two different file extensions in order to accurately capture the reality of what’s going on at runtime. The example package.json you provided would work for TypeScript users in --moduleResolution bundler, but it would have problems in node16/nodenext. This is documented in detail at https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FalseESM.md.

FWIW, I’ve played with ideas on how to eliminate the need for this content-level duplication within the compiler, and it’s not out of the question that a future version of TypeScript will support something to make it unnecessary for libraries who ship dual packages to do this. But as of the TypeScript that exists today, it is necessary, and the consequences of not doing it are thoroughly documented.

[WORMSS]

The packages that these build work in both require() and import.

I don't have moduleResolution set to anything in code that is consuming the package..
But, just as a quick test.. I tried with Node, Bundler (changed module to esnext), Node16 (though, other packages started to fail, but seen no issue with my own) and NodeNext just wanted me to name things with .mjs which I wont do for a quick test.

If you have an example repo where things are meant to be broken I wouldn't mind testing my packages on that.. because I am obviously not reproducing the scenarios that this change is attempting to fix correctly.

At the very least, if things do start to get terrible for this.. I'll just stop buidling for .cjs and just become module only.. That will get away from the multiple .d.ts files problem completely.

[andrewbranch]

If you have a publicly published package, I can make a repro specific to your use case. (In the future, I think it would be cool to be able to auto-generate repros with https://arethetypeswrong.github.io)

[duclet]

I'm curious how we go about specify that with just the types in our package.json. I really prefer to not having to go full expand mode to using exports and imports definitions.

[andrewbranch]

You have to use exports to make Node aware of both JS files. I put up an example PR taking advantage of the new version at total-typescript/shoehorn#9

[molisani]

Is there any way we could de-duplicate the declaration files by forcing code splitting between the two formats? When there are multiple entrypoints, the dts build will create intermediate declaration files to reference the shared types. This kind of declaration code splitting doesn't happen when there's only a single entrypoint, but multiple formats.

I can open this as a separate issue if that seems like a reasonable ask, since this PR has been merged.