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

Support TS moduleResolution: "node16" #66

Closed
wants to merge 1 commit into from

Conversation

antitoxic
Copy link

First issue fixed

"moduleResolution": "node16" in TS will cause:

TS7016: Could not find a declaration file for module postcss-normalize. `postcss-normalize/index.mjs` implicitly has an any type.

There are types at `postcss-normalize/index.d.ts`, but this result could not be resolved when respecting package.json exports. The postcss-normalize library may need to update its package.json or typings.

Second issue fixed:

  • Typescript "moduleResolution": "node16" requires different type info when CJS modules like this one are imported from ESM module.
  • To make default exports (export default) work for both CJS and ESM, we need export = X and not export default X.
  • When you have multiple exports you need to create a namespace named exactly X and in there define the exports.

refs:

PS/FYI: npm run test tape was failing even before my changes so i had to commit with --no-verify.

@romainmenke
Copy link
Member

Yeah, not gonna do that.

I am sorry and I truly sympathize with this situation but I am not going to invest in typings for dual cjs and esm packages. This is an unsolvable problem for package maintainers. You should just silence these errors on your end.

I will remove the typings in the next release.
This package doesn't need types.

@romainmenke romainmenke closed this Sep 7, 2024
@antitoxic
Copy link
Author

Totally fine with your decision. Just noting this was a PR with pre-made solution for this. No need to invest into solving it.

I'm ok with maintaining a patch for my projects. Do whatever works best for you. Thank you for having this lib in the first place.

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

Successfully merging this pull request may close these issues.

2 participants