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 are CJS, but implementation is ESM #307

Closed
neurosnap opened this issue May 25, 2023 · 3 comments
Closed

Types are CJS, but implementation is ESM #307

neurosnap opened this issue May 25, 2023 · 3 comments

Comments

@neurosnap
Copy link

Greetings! This project is awesome, thanks for all the work that has gone into it!

I recently stumbled across a project that attempts to scan an npm package for typescript type issues. Since I'm using dnt to generate the npm package for me, I was wondering if you had any guidance on how to fix these issues:

Screenshot 2023-05-25 at 3 34 07 PM

I'm mostly concerned about node16 (Masquerading as CJS). It looks like we need to have specific esm types .d.mts in our package.exports.

From the github: https://github.com/arethetypeswrong/arethetypeswrong.github.io

Types are CJS, but implementation is ESM. In the node16 resolution mode, TypeScript detects whether Node itself will assume a file is a CommonJS or ECMAScript module. If the types resolved are detected to be CJS, but the JavaScript resolved is detected to be ESM, this problem is raised. It is often caused by a dependency’s package.json doing something like:

{
  "exports": {
    "types": "./index.d.ts",
    "import": "./index.mjs",
    "require": "./index.js"
  }
}

Because there is no "type": "module" setting here, the .js and .d.ts file will always be interpreted as a CommonJS module. But if the importing file is an ES module, the runtime will resolve to the .mjs file, which is unambiguously ESM. In this case, the module kind of the types misrepresents the runtime-resolved module. This tends to present the most issues for users when default exports are used. In the future, this tool may detect whether an export default might make this problem more severe and give a full explanation of why. In the meantime, you can read the explanation in microsoft/TypeScript#50058 (comment). The simple fix for the example above would be to add an index.d.mts file dedicated to typing the .mjs module, and remove the "types" condition.

Thanks again!

@dsherret
Copy link
Member

dsherret commented May 25, 2023

Upgrade to https://github.com/denoland/dnt/releases/tag/0.36.0 or set declaration: "inline" -- that should fix the issue.

I'm not sure about the node 10 issues, but node < 16 no longer has LTS so people should be upgrading anyway https://nodejs.dev/en/about/releases/

@neurosnap
Copy link
Author

Ahh! I should have looked at releases, thanks so much!

@dsherret
Copy link
Member

dsherret commented May 27, 2023

No worries! It's good to have an issue for this.

neurosnap added a commit to thefrontside/effection that referenced this issue May 28, 2023
neurosnap added a commit to thefrontside/effection that referenced this issue May 30, 2023
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

No branches or pull requests

2 participants