Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
meteorlxy opened this issue Jul 21, 2022 · 7 comments
Closed

Comments

@meteorlxy
Copy link
Contributor

Description

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

  • setting "type": "module" in package.json.
  • setting "moduleResolution": "Node16" in tsconfig.json's compilerOptions.

Reproduction

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

@valeriangalliat
Copy link
Owner

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
Copy link
Owner

valeriangalliat commented Jul 21, 2022

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
Copy link
Owner

valeriangalliat commented Jul 21, 2022

Interesting: microsoft/TypeScript#49271 (comment)

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 microsoft/TypeScript#50083 but was closed as wontfix

@meteorlxy
Copy link
Contributor Author

meteorlxy commented Jul 22, 2022

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):

  • "type": "module" should be OK if we configured exports.require correctly
  • What about using export = instead of export default? The previous one should work with both cjs and esm.

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
Copy link

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
Copy link
Owner

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants