valeriangalliat / markdown-it-anchor

A markdown-it plugin that adds an `id` attribute to headings and optionally permalinks.
The Unlicense
291 stars 72 forks source link

Types invalid with `"type": "module"` and `"moduleResolution": "Node16"` #118

Closed meteorlxy closed 1 year ago

meteorlxy commented 2 years ago

Description

Current types/index.d.ts does not work well when:

Reproduction

https://github.com/meteorlxy/markdown-it-anchor-types

valeriangalliat commented 2 years ago

Thanks for providing a repo with the repro!

I looked at this and can't figure why this doesn't work. The way we type the fact the anchor function has a permalink property is by doing:

declare function anchor(md: MarkdownIt, opts?: anchor.AnchorOptions): void;

declare namespace anchor {
  // [...]

  export const permalink: {
    headerLink: (opts?: HeaderLinkPermalinkOptions) => PermalinkGenerator
    linkAfterHeader: (opts?: LinkAfterHeaderPermalinkOptions) => PermalinkGenerator
    linkInsideHeader: (opts?: LinkInsideHeaderPermalinkOptions) => PermalinkGenerator
    ariaHidden: (opts?: AriaHiddenPermalinkOptions) => PermalinkGenerator
  };
}

export default anchor;

And this is exactly how TypeScript compiles a TS file containing function anchor(){}; anchor.permalink = { ariaHidden() {} }...

I'm clueless how to fix that, any suggestions welcome! 🙏

valeriangalliat commented 2 years ago

OK, setting "type": "module" in markdown-it-anchor's package.json fixes it, but that'd break many other things so is not an option right now.

I tried configuring the exports property in package.json as with typical hybrid CJS/ESM packages, sadly that didn't help.

  "main": "dist/markdownItAnchor.js",
  "module": "dist/markdownItAnchor.mjs",
  "exports": {
    "./package.json": "./package.json",
    ".": {
      "import": {
        "types": "./types/index.d.ts",
        "default": "dist/markdownItAnchor.mjs"
      },
      "require": {
        "types": "./types/index.d.ts",
        "default": "dist/markdownItAnchor.js"
      }
    }
  },
  "types": "./types/index.d.ts",

Even putting the CJS and ESM outputs in different directories, e.g. esm/markdownItAnchor.js with a esm/package.json containing {"type":"module"} didn't work either.

Putting the index.d.ts in the esm/ directory which is {"type":"module"} didn't work either.

valeriangalliat commented 2 years ago

Interesting: https://github.com/microsoft/TypeScript/issues/49271#issuecomment-1139688063

So you can fix it by using:

import { default as anchorPlugin } from 'markdown-it-anchor'

Still I believe TypeScript shouldn't require the dependency to have "type":"module" in its root package.json for the default import to work, as long as exports.import is defined in package.json too, pointing to an actual ESM file. Might be worth reporting an issue in TS itself?

Edit: reported it here https://github.com/microsoft/TypeScript/issues/50083 but was closed as wontfix

meteorlxy commented 2 years ago

Uh, that's an interesting limitation 🤔 (Apologize that I didn't have enough time to search for related issues yesterday, so I only posted a repro)

Some ideas (might not be 100% correct though):

Anyway, as it's a known limitation (or work as intended) of typescript, I think libraries should try to follow it (at least for now).

wickning1 commented 2 years ago

Facing the same issue trying to provide a hybrid library. I think it can be solved by placing a package.json file in the types directory next to the index.d.ts with its only content { "type": "module" }. That may be a little safer than changing the root package.json.

valeriangalliat commented 2 years ago

OH this seems to work well for both CJS and ESM imports, nice find @wickning1! 🙌

I just released version 8.6.5 with this fix, @meteorlxy let me know if that fixes the issue for you 😄

valeriangalliat commented 1 year ago

I'll consider this one fixed, feel free to reopen if I missed anything!