Type mismatching with @types/markdown-it 14.1.2

Fachep commented 2 months ago

Describe the bug

DefinitelyTyped/DefinitelyTyped#70083 changed type Delimiter. Now npm resolves "@types/markdown-it": "^14.1.1" to 14.1.2, which does not match the type definition exported from vitepress. Adding plugins for MarkdownIt causes tsc error TS2769: No overload matches this call.

Reproduction

package.json:

{
  "type": "module",
  "devDependencies": {
    "vitepress": "1.3.2",
    "@types/markdown-it": "^14.1.1",
    "typescript": "latest"
  }
}

https://stackblitz.com/edit/vite-qlr8wt?file=tsc.out.log Run npx tsc at package root.

Expected behavior

No errors from tsc.

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.20.3 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.2.3 - /usr/local/bin/npm
    pnpm: 8.15.6 - /usr/local/bin/pnpm
  npmPackages:
    @types/markdown-it: ^14.1.1 => 14.1.2 
    vitepress: 1.3.2 => 1.3.2


### Additional context

Specifying version 14.1.1 for `@types/markdown-it` in package.json works for now.
```typescript
// node_modules/vitepress/dist/node/index.d.ts

interface Delimiter {
    marker: number;
    length: number;
    jump: number;
    token: number;
    end: number;
    open: boolean;
    close: boolean;
}

I guess vitepress shouldn't embed external type definitions like this?

Validations

[X] Check if you're on the latest VitePress version.
[X] Follow our Code of Conduct
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.

brc-dd commented 2 months ago

Add ts-ignore for now. It will be fixed when we update deps here.

Trinovantes commented 2 months ago

Is there any reason why VitePress is currently bundling @types/markdown-it in dist/node/index.d.ts?

$ grep "interface MarkdownIt" node_modules/vitepress/dist/node/index.d.ts
1250:interface MarkdownItConstructor {
1259:interface MarkdownIt {
1712:    interface MarkdownItEnv {
1731:interface MarkdownItHeader {
1794:    interface MarkdownItEnv {
1868:    interface MarkdownItEnv {

This problem will keep happening on every future @types/markdown-it version. It would be nice to just extern this dependency so even if VitePress and plugins use different versions, we can use our package manager to override the resolved version.

It seems VitePress is currently using rollup-plugin-dts to bundle type definitions and using respectExternal option

However, I don't think this option actually externs dependencies:

https://github.com/Swatinem/rollup-plugin-dts/blob/115158d50e90e5a3ee0b5cad215fb2c5432c0278/src/index.ts#L216-L222

I think it's treating the case where respectExternal === true and resolvedModule.isExternalLibraryImport === true same as non-external dependency

I was able to remove @types/markdown-it from the bundle by editing VitePress's rollup.config.ts:


const dtsNode = dts({
  // respectExternal: true,
  tsconfig: r('src/node/tsconfig.json')
})

// const originalResolveId = dtsNode.resolveId

// dtsNode.resolveId = async function (source, importer) {
//   const res = await (originalResolveId as Function).call(this, source, importer)
//   if (res?.id) res.id = await fs.realpath(res.id)
//   return res
// }

I have to comment out the resolveId function because it's causing a bunch of ENOENT errors. However I'm not sure what the implications are for removing the modified resolveId function

peterroe commented 2 months ago

This problem hasn't been fixed yet, right?

brc-dd commented 2 months ago

just released in 1.3.3

vuejs / vitepress