Fix type declaration bundle(s)

[andrewbranch]

Marked version:

9.1.2

Markdown flavor: n/a

Description

Related: #2997

👋 Hi, I’m the author of https://arethetypeswrong.github.io. I noticed this problem because marked is a dependency of the project, but coincidentally, the purpose of the project is to detect and diagnose issues exactly like this.

marked ships both an ESM and CJS bundle, but attempts to share types generated with tsc --outFile between both. This does not work for a few reasons. The declaration files made with --outFile declare ambient modules that don’t exist:

// lib/marked.d.ts
declare module "rules" {
  // ...
}

allows any user to import from "rules" once they’ve imported from "marked":

import { Marked } from "marked";
import { block } from "rules"; // ???
// TypeScript ok with this due to .d.ts file, but
// crashes at runtime/bundle-time!

Additionally, the .d.ts file is recognized as typing an ES module, which causes --moduleResolution nodenext users to not be able to import marked from CommonJS files, as in #2997 (which doesn’t seem to be related to @types/marked to me).

You can see details about this problem, with a linked explainer, at https://arethetypeswrong.github.io/?p=marked%409.1.2

Expectation

Types work in TypeScript configurations reflecting runtimes where marked works

Result

Types do not work in CommonJS files in --moduleResolution nodenext, even though marked itself works. Additionally, types declare modules that do not exist at runtime.

What was attempted

This PR generates .d.ts bundles for each module format with a tool that was designed to do this: https://github.com/timocov/dts-bundle-generator

It also validates the result with @arethetypeswrong/cli to prevent future regressions of a similar variety.

Contributor

• Test(s) exist to ensure functionality and minimize regression (if no tests added, list tests covering this PR); or,
• no tests required for this PR.
• If submitting new feature, it has been documented in the appropriate places.

Committer

In most cases, this should be a different person than the contributor.

• CI is green (no forced merge required).
• Squash and Merge PR following conventional commit guidelines.

[vercel]

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

Name	Status	Preview	Comments	Updated (UTC)
marked-website	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 31, 2023 0:21am

[UziTech]

It looks like the .d.ts and .d.cts files that are generated are the same file. Why do we need both?

[andrewbranch]

Because the file extension describes something very important. A .d.ts file describes a .js file. A .d.cts file describes a .cjs file. You have a .js file and a .cjs file, so you need a .d.ts file and a .d.cts file. There are more details at https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FalseESM.md

[UziTech]

Thanks for doing this! 💯 I am not a typescript expert so it is great to have the community watching out for things like this.

[styfle]

Is this dependency used?

[andrewbranch]

Here (line 95): https://github.com/markedjs/marked/pull/3038/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R95

It’s optional, but I recommend it any time types and JS are generated by separate tools, as opposed to a single tsc run. As evidenced by the issue this PR fixes, it is quite easy to make JS that disagrees with the types when using a tool like Rollup.

[styfle]

Oh I see, thats the attw bin 👍

Oddly enough, @arethetypeswrong/cli depends on marked 😆

https://www.npmjs.com/package/@arethetypeswrong/cli?activeTab=dependencies

[andrewbranch]

That’s how I noticed this issue! 😄

[styfle]

Will dts-bundle-generator fail when tsc would fail? Usually these bundlers need something like tsc --noEmit first.

[andrewbranch]

Hm, I didn’t notice that nothing else here is running tsc for validation. Do you want me to add that to build:types or another script?

[UziTech]

Ya I think we would want build:types to fail if tsc fails

[UziTech]

We do use tsc in test:types. I'm not sure if that is good enough

[andrewbranch]

Done 👍 (tsc with no arguments uses the file named tsconfig.json by default which is what you want—it already has noEmit and it will reflect exactly the errors you see in-editor)