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

moduleResolution nodenext or node16 causes type errors with a default callable export (PostCSS) #50175

Closed
brainkim opened this issue Aug 4, 2022 · 6 comments
Labels
Duplicate An existing issue was already created

Comments

@brainkim
Copy link

brainkim commented Aug 4, 2022

Bug Report

πŸ”Ž Search Terms

nodenext, node16, ESM, exports, package.json, dependency, postcss

πŸ•— Version & Regression Information

typescript@4.7.4

⏯ Playground Link

Playground link with relevant code

Hilariously, the playground only seems to show this bug intermittently or doesn’t save the config. I can usually force the error to appear by setting the module configuration in the settings.

Screen Shot 2022-08-03 at 9 17 53 PM

πŸ’» Code

index.ts

import postcss from "postcss";

console.log(postcss());

tsconfig.json

{
  "compilerOptions": {
    "target": "esnext",
    "module": "esnext",
    "moduleResolution": "node16",
    "lib": ["esnext", "dom"],
    "strict": true
  }
}

package.json

{
  "name": "typescript-error-example",
  "private": true,
  "type": "module",
  "license": "UNLICENSED",
  "dependencies": {
    "postcss": "^8.4.14",
    "typescript": "^4.8.0-beta"
  }
}

typescript-example.zip

πŸ™ Actual behavior

Running npm i && tsc --noEmit on a directory configured like above produces the following error.

index.ts(3,13): error TS2349: This expression is not callable.
  Type 'typeof import("/Users/briankim/typescript-example/node_modules/postcss/lib/postcss")' has no call signatures.

πŸ™‚ Expected behavior

Running npm i && tsc --noEmit should produce no errors

Deleting "type": "module" from package.json or changing the moduleResolution to node in tsconfig.json makes the error go away. πŸ™ƒ

@Josh-Cena
Copy link
Contributor

The error is working as expected. postcss is "lying" in their type declaration:

declare const postcss: Postcss

export default postcss

If they are honest, then the corresponding JS file should be exports.default = postcss. However, the JS file is actually

module.exports = postcss
postcss.default = postcss

The information that module.exports is a Postcss instance as well is lost. Therefore, TS can only be made to believe that import postcss from "postcss"; in ESM would load { default: postcss }. postcss would need to fix their type declaration to add something like export = postcss and adjust other stuff accordingly (since you can't have other exports with export =).

@brainkim
Copy link
Author

brainkim commented Aug 4, 2022

I’m not sure about the distinction between setting the default property on exports versus setting the default property of the thing which is set to module.exports, or why that is β€œlying” in the ESM world. Especially if things like esModuleInterop are set to true (this doesn’t fix the error). This also doesn’t really explain why it would work when moduleResolution is node and not node16.

Before y’all mark this as an upstream issue, I would like to say that PostCSS is not the only library with this problem. I’ve also seen it with front-matter.

@Josh-Cena
Copy link
Contributor

In moduleResolution: "node", TypeScript basically did not acknowledge the existence of Node ESM and assumes you are always downleveling to CJS. esModuleInterop is an emit flag that causes TypeScript to emit more permissive code that correctly imports the default import both when the module.exports is the default export itself (Node ESM semantics) or when module.exports.default is the default export (Babel/tsc ESM semantics). But if you are not downleveling, you have to play by Node's rules. This has been brought up numerous times in the past, e.g. #49189, #48845, and the answer is always "they need to fix it themselves because it's actually lying".

@RyanCavanaugh
Copy link
Member

This is a bug (to confirm, change the test to import { default as postcss } ... and see that the error goes away). See #49567.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Aug 5, 2022
@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@simonbuchan
Copy link

simonbuchan commented Sep 14, 2022

@RyanCavanaugh - to be clear, you mean it's a bug that calling the default worked, and that the definitions should be updated?

Assuming that's true, what is the suggested approach? export = foo breaks named exports, even type only exports, which are very common in these libraries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

5 participants