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

This expression is not callable for ESM consuming CJS with default export #52086

Closed
unional opened this issue Jan 3, 2023 · 19 comments
Closed
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@unional
Copy link
Contributor

unional commented Jan 3, 2023

Bug Report

The package react-use-websocket is written in TypeScript and transpiled by TypeScript.
It transpiles to CJS and exposes a default export.

When consume by a ESM project, TS errors out: This expression is not callable.

🔎 Search Terms

ESM CJS default export

🕗 Version & Regression Information

4.9.3 (likely from 4.7.0)

5.0.0-dev.20230103 also fails

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about ESM CJS

⏯ Playground Link

Repro link: https://github.com/cyberuni/ts-esm-on-cjs-with-default-export

💻 Code

import useWebSocket from 'react-use-websocket'
// or import { default as useWebSocket } from 'react-use-websocket'
// or import * as ws from 'react-use-websocket'; ws.default(...)

useWebSocket('abc') // <-- This expression is not callable

tsconfig.json:

{
  "compilerOptions": {
    "allowSyntheticDefaultImports": true,
    "module": "Node16", // doesn't matter
    "moduleResolution": "Node16",
    "outDir": "dist",
    "strict": true,
    "target": "ES2019"
  },
  "include": [
    "ts",
    "types"
  ]
}

🙁 Actual behavior

This expression is not callable

🙂 Expected behavior

work

@unional
Copy link
Contributor Author

unional commented Jan 3, 2023

Kind of related issues:

#50466
Azure/azure-sdk-for-js#22804 (comment)

But not directly related because TS transpiles CJS with type definition mismatch in the first place,
expecting the CJS code to be consumed with esModuleInterop but that has no effect in ESM package when moduleResolution is Node16 or NodeNext

@gdostie
Copy link

gdostie commented Jan 17, 2023

I have the same problem and stumbled upon the following workaround from a question about the same exact problem on stackoverflow.

import bar from 'foo';

export const foo = bar as unknown as typeof bar.default;

It works, but it is far from convenient if you have to do this in several files for multiple packages over a large codebase.

@RyanCavanaugh
Copy link
Member

The intended behavior of the Node16 module resolution mode is to follow Node's module resolution logic, and in this case, TypeScript correctly does. This is a correct error. react-use-websocket's type definition, and the runtime definition of its module, do not provide a function at the requested place.

If you load react-use-websocket from a CommonJS module, you can see its public shape (running this code in a .js file from a non-type: "module" repo):

// test.js
console.log(Object.keys(require("react-use-websocket")));

> node test.js
[
  'default',
  'useSocketIO',
  'ReadyState',
  'useEventSource',
  'resetGlobalState'
]

Its .default property is the function, as "expected".

If you load react-use-websocket from an ES module, you can see its public shape as well:

// test.mjs
import * as ws from "react-use-websocket";
console.log(Object.keys(ws));
console.log(Object.keys(ws.default));
console.log(Object.keys(ws.default.default));

> node test.mjs
// Object.keys(ws)
[
  'ReadyState',
  '__esModule',
  'default',
  'resetGlobalState',
  'useEventSource',
  'useSocketIO'
]
// Object.keys(ws.default)
[
  'default',
  'useSocketIO',
  'ReadyState',
  'useEventSource',
  'resetGlobalState'
]
// Object.keys(ws.default.default)
[]

and the .default property isn't a function

import * as ws from "react-use-websocket";
// 'false'
console.log(ws.default instanceof Function);

The CJS<->ESM interop story, per Node and TypeScript (but not necessarily other bundlers, YMMV) is that a CJS module gets wrapped in the default export of the corresponding synthesized ESM entry point. Node and TypeScript agree here that the esModule flag doesn't do anything.

So node and TypeScript 100% agree on this count: If you import this module from an ESM context, the object you get back (from the * import) has a default property with a default property. That's the correct way to refer to that function.

Really, react-use-websocket shouldn't be providing an export named "default" in a CJS module, because it's never (in a cromulent module configuration) going to be consumable from a default import form.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Jan 25, 2023
@unional
Copy link
Contributor Author

unional commented Jan 25, 2023

Really, react-use-websocket shouldn't be providing an export named "default" in a CJS module, because it's never (in a cromulent module configuration) going to be consumable from a default import form.

The problem is, that is exactly what tsc generates when compiling a default export.
I have created https://github.com/cyberuni/typescript-module-resolutions-demo to demonstrate how tsc behaves.

IMO it is just wrong (or put in another way, some bugs through the workflow), that TypeScript cannot consume code generated by TypeScript.

@RyanCavanaugh
Copy link
Member

IMO it is just wrong (or put in another way, some bugs through the workflow), that TypeScript cannot consume code generated by TypeScript.

It absolutely can consume it, just not with the syntax you're trying to use. The TypeScript project emitted a .d.ts file, which was consumed by another TypeScript project, with 100% fidelity with what the runtime allows for. Had TypeScript not issued an error here, you would have gotten a runtime error instead! It's hard to see how that is preferable.

The problem is, that is exactly what tsc generates when compiling a default export.

The project is authored with ESM-style export syntax but targets CJS. So you have a CJS output with export names that align with the ESM export names, which is presumably what you want to happen. I'm open to hearing proposals as to what should happen instead, if this is "wrong".

That CJS module then gets wrapped into an ESM module wrapper by Node, leading to the double-default problem. Your complaint here is effectively with that behavior, which we're not in control of.

@unional
Copy link
Contributor Author

unional commented Jan 25, 2023

There is a lot more than just the double-default problem. I agree that if that is how Node.js handles it, TypeScript should work the same.

The problem is with other cases. There are many situations that TypeScript typings say one way, but the runtime works differently.

Again you can refer to the repo to see which one fails.
I'm still in the process of summarizing and expanding the use cases, any suggestion are welcome.

The bottom line is, I think TypeScript needs to adjust on both how the typings are generated as well as how they are consumed.

e.g. in a Node.js dual package repository, i.e. type: module and compile to both Node16 and CommonJS (or even ES*), the typings generated should be adjusted accordingly,

and when consuming the typings, in moduleResolution: Node|Node16, they need to be handled differently. As you mentioned, Node.js wraps CJS module into ESM module wrapper in ESM package, while it works differently in CJS package.
There is no way for TypeScript to generate a single typing that work on both. So it has to detect the mode and handled differently.

@RyanCavanaugh
Copy link
Member

There are many situations that TypeScript typings say one way, but the runtime works differently.

Sure, but this bug is about a situation where TypeScript is exactly mimicking the runtime.

Again you can refer to the repo to see which one fails.

Please log a specific bug about which line of emit or resolution is wrong.

The bottom line is, I think TypeScript needs to adjust on both how the typings are generated as well as how they are consume

Again, how? What? Be specific.

e.g. in a Node.js dual package repository, i.e. type: module and compile to both Node16 and CommonJS (or even ES*), the typings generated should be adjusted accordingly,

This is handled correctly, but the package linked in this repro is not a dual package. It doesn't have type: "module" and it doesn't have an export map. It's just CJS written in a way that is made to look like an ES module, but it isn't an ES module at all.

We've investigated several dozen "TypeScript is doing modules wrong" issues at this point and, at time of writing, all of them recently have been module misconfigurations rather than bugs in TypeScript. I'm going to be setting the bar higher on future bug reports so we can move on and do other work - if there's a claim that something in this space isn't working, it needs to be clearly demonstrated.

There is no way for TypeScript to generate a single typing that work on both.

Correct; our position is that you should pick one runtime target (CJS or ESM) and write CJS or ESM code. Consumers can use either through well-trod interop or bundling processes. Producing a single artifact for both CJS and ESM is really only possible when all of your dependencies adhere to a set of coincidences that are not widely correctly adopted and we don't think this is a good model for publishing packages in general.

@unional
Copy link
Contributor Author

unional commented Jan 26, 2023

Sure, but this bug is about a situation where TypeScript is exactly mimicking the runtime.

Yes, I actually agree. This bug corresponding to the node16 | cjs case in https://github.com/cyberuni/typescript-module-resolutions-demo/blob/main/tests/node16/test-result.4.9.4.md which are working as expected.

Please log a specific bug about which line of emit or resolution is wrong.

Will do as I'm going through the cases in https://github.com/cyberuni/typescript-module-resolutions-demo
It will take me some time to get to them. But basically the cases marked with ❌ are potentially bugs.
For example in:
https://github.com/cyberuni/typescript-module-resolutions-demo/blob/main/tests/node-es/test-result.4.9.4.md
https://github.com/cyberuni/typescript-module-resolutions-demo/blob/main/tests/node16/test-result.4.9.4.md

Correct; our position is that you should pick one runtime target

That is one of the main problems. There are cases that we want to support both CJS and ESM, especially in corporate environment. I'm sure you know how slow teams can move in those environments.
As a library author, we can't just dump CJS and produce ESM only packages, while we add features to the packages. At the same time, we need to expose ESM code so that we can leverage the benefits of it in newer projects.

Producing a single artifact for both CJS and ESM is really only possible when all of your dependencies adhere to a set of coincidences

That is not the case, and is probably one of the reasons why there are some communication breakdown.
Even without any "dependencies", what we are saying is that TypeScript as it stands simply unable to produce Dual module packages that works for both ESM and CJS consumers.

@RyanCavanaugh
Copy link
Member

There are cases that we want to support both CJS and ESM [...] That is not the case

Again I think we broadly agree. There are definitely places where you want to ship both CJS and ESM. No contention there. It's also possible (though of course by no means trivial) to write a tool which can take a module written in one module system and, with proper configuration, produce a reasonably-similar module in the other system, subject to some constraints which can be met in most cases.

What I'm saying is, the tool that does this transformation is not TypeScript, or at least should not be. The CJS <-> ESM module problem is not one that TypeScript can uniquely solve, nor is it one that can only be solved in a TypeScript codebase, nor is it one that is currently unsolved. It's a problem on the same tier as minification, downleveling, linting, bundling, gzipping, tree-shaking, and so on: those which the JavaScript ecosystem broadly needs, has, and is in no want of an N+1th solution. We're trying to spend as much time as we can writing the world's best static type system for JavaScript and not writing the world's 43rd tool concerned with doing machine translation of JavaScript from one format to another.

That's what "pick one runtime target" means -- not that you can't or shouldn't dual-ship CJS/ESM, but rather than you should go TS (A) -> JS (A) -> JS(A+ B) where A is either CJS or ESM, B is the other one, and the tool doing the second arrow is some tool that isn't TypeScript. If there are things preventing that, those are the things that we want to hear about, not that half-baked mostly-non-working solutions derived from trying to make ill-advised TS (A) -> JS (B) transforms work when really, allowing that transformation in the first place was a really bad idea that we shouldn't have implemented in the first place.

@unional
Copy link
Contributor Author

unional commented Jan 26, 2023

Thanks for the detailed reply (as always). From what you said. My understanding is this:

  1. I created a library originally distributed as CJS.
  2. Now I want to convert the package to distribute both CJS and ESM.
  3. I add tsconfig.esm.json, with module: Node16, moduleResolution: Node16, and type: module.
  4. Now, instead of refactor the existing tsconfig.json to tsconfig.cjs.json, I need to use "another tool" (that does not yet exist, AFAIK) to convert the ESM output from tsc (both the JS file and .d.ts files), and produce a CJS variant.

Why I say that is because while you mentioned A is either CJS or ESM, it's basically ESM because you can't convert TS -> CJS -> ESM.
And the problem is with the current system, There is no satisfactory config + code that allow us to write code in TS that can compile to both CJS or ESM. Mainly because for ESM we should not turn on esModuleInterop and allowSyntheticDefaultImports, while:

  • CJS doesn't work correctly without esModuleInterop, and
  • using esModuleInterop actually requires the code to be different

That's why we are stuck there.

The repo does not demonstrate those cases yet.
Will need to add those so that proper bugs can be filed.

UPDATE: note that "the another tool" needs to consume and transform both JS and typings. IMO that would be best handled by TypeScript, or else "the another tool" will be tightly coupled to a specific version of TypeScript and will break whenever tsc improve its typings generation or when introducing breaking changes.

UPDATE 2: If what I expect is correct, this should be printed in bold in the TypeScript docs as this would trip over so many package authors.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jan 26, 2023

TypeScript has at best the same, and possibly worse, information than any other tool could have about whatever speculative transform ESM->CJS (just for the sake of argument) might exist. The typings as they exist today do not carry sufficient information on their own to inform how to correctly do this transform.

Today in TS you can write

declare module "foo" { export const a = 3 }
import * as x from "foo";

and this all will just happily work. We have no idea whether "foo" actually resolves to an ESM module or a CJS-to-ESM interop'd module, so have no idea whether the CJS equivalent of this program needs to account for the non-interopedness of the CJS equivalent of the "foo" module (assuming there even is one).

Similarly you could write

declare module "foo/bar.js" { export const a = 3 }

and TypeScript has no idea if that means we're going to be directly loading the bar.js file out of the foo CJS module, as it well might be, or if there's a foo ESM module with an export map for bar.js, which it well might be.

This is why we're so insistent on not messing with import paths (and to a lesser degree really regret providing a means to convert ESM to CJS syntax during downleveling without throwing up red flags everywhere) -- the typings that most projects have do not provide enough information to correctly guess the right output syntax, and even if they could, I think the lived experience of the Node16 transition shows that most package authors couldn't write the correct metadata even if it was mechanically possible to do so. The closest true source of "typings data", if it exists, is what's in package.json files on disk, and that's what the Node16 module resolution mode tries to adhere to as closely as possible, and the result is that there's a ton of confusion around what any given package.json means even for the people who authored them.

Either one of two things has to be true:

  • The ESM -> CJS transform is so trivial that anyone could write it, maybe with a sufficiently advanced regex
  • The ESM -> CJS transform is so complex that it's its own build tool space in which people might find different solutions, each with different configurations and trade-offs

Honestly most people here mistakenly think that the first one is true (i.e. you can just slap .js extensions on stuff and it'll all just work out 😜). I suspect you realize the second one is actually true, so the discussion here is just really about whether TS takes on new responsibilities it's never had before just to provide tooling that would actually only be relying on information that would be equally relevant and necessary in JS scenarios.

So again none of this is to say that we don't want to fix bona fide bugs in this space, just that the lack of some cross-targeting functionality is an intentional line in the sand we're drawing. We haven't rewritten esbuild/webpack/parser/terser/rollup/etc and are in fact removing product options which mimic those tools, and in fact are thinking about how we might exit the downleveling space as well. I think that's the right call for all of these spaces, both from our resource manage perspective, and for how TS should slot into the broader ecosystem.

@unional
Copy link
Contributor Author

unional commented Jan 26, 2023

Um.... I can see your points. For you, you have to think about all the possible cases where user "adding" type information through declare module within TS files, or separate typings files along with JS (using allowJs) etc.
It's definitely a very hard problem to solve.

My perspective is that writing only in TypeScript and using the TS file as the source of truth is probably the majority case. And that case, tsc probably has more information then other tools can access (please correct me if I'm wrong on this).

Yes, if you throw in additional typing files and/or dependencies with typings (AND differences between bundled typing files generated by tsc vs typing files from DT), it is a massive headache to deal with.

It is a very difficult task but at the same time we try to figure out some possible solutions to make it works somehow.
Maybe having a strict compile mode that only support a subset of well defined constructs, e.g. no declare module or export =.

I'll continue to investigate about it and probably make some posts or videos about this topic, as I think the right first steps is to have those not working use cases drawn out.

@typescript-bot
Copy link
Collaborator

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

@unional
Copy link
Contributor Author

unional commented Feb 1, 2023

btw, I just realized one thing, to specify a different typings for CJS when using the exports map, it needs to be done like this:

{
  "exports": {
    "require": {
      "types": "./cjs/index.d.ts",
      "default": "./cjs/index.js"
    },
    "import": {
      "types": "./esm/index.d.ts",
      "default": "./esm/index.js"
    }
  }
}

Unless you keep the main and types|typings field at the top-level. i.e. a hybrid approach:

{
  "exports": {
    "types": "./esm/index.d.ts",
    "import": "./esm/index.js"
  },
  "main": "./cjs/index.js",
  "types": "./cjs/index.d.ts"
}

@StreetStrider
Copy link

I'm very sorry, but I've followed through the thread and it looks similar to my issue but I'm still thinking TS behavior is incorrect.

I got pretty straightforward file with sole module.exports =, very simple export.

The accompanied type reflects this default export with export default function.

import emitter from '@streetstrider/emitter'
console.log(typeof emitter)
const e = emitter()

If I try to import this in ESM mode with bare Node I will get the value of the module.exports correctly as per interop mode mentioned in the docs.

When importing CommonJS modules, the module.exports object is provided as the default export.

However, under TS I got:

This expression is not callable

Despite both types and Node intended behavior points to normal function in that position.

If I bypass this typecheck error by workaround mentioned above, the code will work as expected (calling function and go on) without actual code changing. Thus it is a TS typecheck error. The underlying ESM code works normally in pure Node ESM and under ts-node.

This issue got similar repro in softonic/axios-retry#228.

@StreetStrider
Copy link

Is it possible for someone to double-check this issue and maybe my results?
@RyanCavanaugh shall I create new clean repro if I wanna to get some traction here?

@RyanCavanaugh
Copy link
Member

@StreetStrider certainly you have a misconfiguration somewhere. Please log a bug under the "module resolution" template to provide the necessary info.

@RyanCavanaugh
Copy link
Member

https://arethetypeswrong.github.io/?p=%40streetstrider%2Femitter%401.2.1

@StreetStrider
Copy link

@RyanCavanaugh thanks for the help. As it turns out, I had a misconception, thinking that export = is an old way of saying export default. So I always sticked to the export default because it mirrors es modules and also works properly in bundler scenario, thus making things much easier.
(It was the case until node16 esm, as arethetypeswrong states)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

5 participants