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" throws ESM related errors #50058

Open
sagargurtu opened this issue Jul 26, 2022 · 28 comments
Open

"moduleResolution": "NodeNext" throws ESM related errors #50058

sagargurtu opened this issue Jul 26, 2022 · 28 comments
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status. Rescheduled This issue was previously scheduled to an earlier milestone

Comments

@sagargurtu
Copy link

sagargurtu commented Jul 26, 2022

Does this issue occur when all extensions are disabled?: Yes

  • VS Code Version: 1.69.2
  • OS Version: macOS 12.4
  • TypeScript Version: 4.7.4

Steps to Reproduce:

  • Create an npm package. Install typescript@4.7.4 and pretty-ms.
  • Set tsconfig.json as:
{
  "compilerOptions": {
    "target": "esnext",
    "lib": ["dom", "esnext"],
    "skipLibCheck": true,
    "esModuleInterop": true,
    "allowSyntheticDefaultImports": true,
    "module": "NodeNext",
    "moduleResolution": "NodeNext"
  },
  "include": ["./src/**/*"]
}
  • Add "type": "module" in package.json
  • Following error messages are displayed by VSCode in a file with .ts extension under src/:
// Cannot find module 'pretty-ms' or its corresponding type declarations. ts(2307)
import ms from "pretty-ms";

// The 'import.meta' meta-property is not allowed in files which will build into CommonJS output. ts(1470)
console.log(import.meta.url);
  • tsc throws no errors.
  • Change below values intsconfig.json
{
  "module": "esnext",
  "moduleResolution": "node"
}
  • VSCode no longer throws any errors.
@mjbvz mjbvz transferred this issue from microsoft/vscode Jul 26, 2022
@mjbvz mjbvz removed their assignment Jul 26, 2022
@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Jul 28, 2022
@RyanCavanaugh
Copy link
Member

It sounds like you had a misconfiguration and fixed it. I don't see a demonstrated bug here.

@fatcerberus
Copy link

Isn't nodenext supposed to treat .ts as ESM when "type": "module" is set in package.json? That looks like what the bug is here, setting module to esnext shouldn't have been necessary.

@sagargurtu
Copy link
Author

Yes, that's what the issue is here. When "module": "NodeNext" and "moduleResolution": "NodeNext" are set, VSCode starts throwing above errors (might be because it starts treating the target as commonjs? not sure). Running tsc however on the .ts files is successful and throws no errors. Is this a bug or am I missing some configuration?

@RyanCavanaugh RyanCavanaugh removed the Question An issue which isn't directly actionable in code label Jul 28, 2022
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jul 29, 2022

@fatcerberus points out that #50086 is likely related FWIW.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jul 29, 2022
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.9.0 milestone Jul 29, 2022
@weswigham
Copy link
Member

@sheetalkamat was this also fixed by your PR?

@sheetalkamat
Copy link
Member

sheetalkamat commented Aug 1, 2022

It probably is but i didnt verify it explicitly (assuming issue is with descipencies between errors reported for vscode and tsc)

@egasimus
Copy link

egasimus commented Sep 19, 2022

The main issue with ESM and nodenext/node16 mode is TypeScript refusing to add .js extensions to the generated import statements in the compiled files.

The officially proposed workaround is to manually add .js extensions in the TypeScript sources, which is particularly graceless. So .ts extension is disallowed, yet .js extension is required in order to point to a .ts file - what the fuck? And where do .d.ts files even fit into this?

It's particularly surprising because TypeScript encourages you to use import and export statements even when compiling to CJS. And when you combine this broken behavior with Node's half-assed effort to push people off CommonJS and onto ESM, this hits real hard if you're using monorepos/workspaces/submodules/writing a library.

Frontend framework users whose build toolchain compiles TypeScript on demand will be hit next as they add the .js extensions and their code stops updating. Frameworks will have to special-case this kludge and it'll become 10x harder for a newcomer to understand how JS modules work.

It makes my team members develop an aversion to TypeScript, not to mention it probably got me looking pretty bad myself, because the only way to perform the otherwise trivial act of delivering an isomorphic library (i.e. one which which works out of the box in all contexts - Node, browser, ESM, CJS, pre-compiled, on-demand, framework, no framework) is walking the compiled code's AST to add the extensions.

@smaye81
Copy link

smaye81 commented Sep 21, 2022

I am seeing an issue similar to this where the addition of NodeNext module resolution throws an incorrect error when using it with default imports.

Basically, when trying to import the long library, if you use a module resolution of Node, it will compile fine. However, if I switch to NodeNext, I get an error that says:

  node_modules/long/index.d.ts:446:1
    446 export = Long; // compatible with `import Long from "long"`
        ~~~~~~~~~~~~~~
    This module is declared with 'export =', and can only be used with a default import when using the 'allowSyntheticDefaultImports' flag

Despite the fact that allowSyntheticDefaultImports (and esModuleInterop) are set to true.

I created a super simple repro repo (reprository?) here: https://github.com/smaye81/ts-repro. You can see this behavior by running the two scripts:

npm run build:node will succeed
npm run build:nodenext will fail with the above error.

I'm not sure if I'm just using a bad configuration, but I would think at the very least this error message should be fixed, right?

@cyberwombat
Copy link

cyberwombat commented Oct 16, 2022

Not sure if same issue but with nodenext my code seems to transpile but shows errors in editor. I made a super simple sandbox here.

Basically this code won't work w moduleResolution set to nodenext.

import Ajv from "ajv"

export const getClient = (): Ajv => { 
    return  new Ajv() 
}

My current workaround is to add dozens of lines such as declare namespace 'ajv' in my code for about half of my modules - presumably cjs ones. Calling - as in my example:

import Ajv from "ajv"`

export const getClient = (): Ajv.default => { 
    return  new Ajv.default() 
}

Makes the editor code go away but now code doesn't transpile correctly (results in things like ajv['default']['default'])

What is baffling is IF editor says error then its correct and if it says no error then its not. I kept thinking that my TS linter and my tsc code were using different config but they do not - the fact that the sandbox also doesn't work seems to indicate some weird error.

@Sec-ant
Copy link

Sec-ant commented Oct 23, 2022

I encountered this similar problem when I want to import the default create function from zustand/vanilla while setting NodeNext for both module and moduleResolution in tsconfig.json. For now I use default-import as a temporary fix.

this doesn't work

import create from "zustand/vanilla";
export const store = create(() => ({ foo: "bar" }));

this works

import zustand from "zustand/vanilla";
import { defaultImport } from "default-import";
const create = defaultImport(zustand);
export const store = create(() => ({ foo: "bar" }));

A minimal repo to reproduce this problem: https://stackblitz.com/edit/node-vj368k?file=src/index.ts

@weswigham
Copy link
Member

@zustand/vanilla has incorrect types, again, it only declares types for a cjs entrypoint for everything, and that is the result of that mismatch.

@Sec-ant
Copy link

Sec-ant commented Oct 23, 2022

@weswigham Thanks for the quick response, so what's the correct way to declare types for a mjs entrypoint? Forgive me if I sound stupid.

Update:
If someone like me bumped into this issue and encountered similar problems, have a check on these discussions:
hayes/pothos#597
pmndrs/zustand#1381

There're generally two things that worth noting:

  • If you have import and export statements in an esm type file, you have to check whether you have added .js extensions in those statements.
  • If the type field in package.json is commonjs or not set, typescript will recognize .js or .d.ts as cjs modules, even though they may appear in the exports: import field. You can use a lower level scoped package.json in the esm dist folder to make the files in it be recognized as es modules, or use .mjs and .d.mts extensions to explicitly set them as es modules.

@kachkaev
Copy link

kachkaev commented Oct 31, 2022

I’m facing the same issue in these two PRs:

Here are some third-party packages that have problems with default exports when "moduleResolution": "NodeNext" or "moduleResolution": "Node16":

  • ajv
  • styled-components
  • algoliasearch
  • next/link, next/document, next/image, etc.
  • react-slick
  • @mui/icons-material/*
  • react-gtm-module
  • @emotion/cache
  • react-html-parser
  • slugify

As a temp solution, I’ve managed to do this:

-import x from "x";
+import _x from "x";
+const x = _x as unknown as typeof _x.default;

Although it‘s true that the problem is within the packages, I wonder if it is possible to create a new compile option which could be used during the transition period? Something like allowSyntheticDefaultImports, but for a different use case.

When switching a project to "moduleResolution": "NodeNext", it’s much easier to negotiate one extra line in tsconfig.json than a lot of import _x from "x" hacks. Waiting for upstream fixes may take a while and slow down migration to ESM.

@smaye81
Copy link

smaye81 commented Nov 1, 2022

I’m facing the same issue in these two PRs:

Here are some third-party packages that have problems with default exports when "moduleResolution": "NodeNext" or "moduleResolution": "Node16":

  • ajv
  • styled-components
  • algoliasearch
  • next/link, next/document, next/image, etc.
  • react-slick
  • @mui/icons-material/*
  • react-gtm-module
  • @emotion/cache
  • react-html-parser
  • slugify

As a temp solution, I’ve managed to do this:

-import x from "x";
+import _x from "x";
+const x = _x as unknown as typeof _x.default;

Although it‘s true that the problem is within the packages, I wonder if it is possible to create a new compile option which could be used during the transition period? Something like allowSyntheticDefaultImports, but for a different use case.

When switching a project to "moduleResolution": "NodeNext", it’s much easier to negotiate one extra line in tsconfig.json than a lot of import _x from "x" hacks. Waiting for upstream fixes may take a while and slow down migration to ESM.

I would also add long to your above list. See my comment here. It seems like in addition to the issue in the package, the error message is not correct from the compiler.

@kachkaev
Copy link

kachkaev commented Jan 26, 2023

Thanks @RyanCavanaugh! Yep, you are right – the problem is with incorrect upstream typings. In an ideal world, the ecosystem would be ESM-ready and we would not have this problem at all. In practice, we have quite a few npm packages that won't be quick to patch, which complicates the migration to "moduleResolution": "NodeNext" in a general case.

This specific tsc error does not affect the runtime – otherwise const X = _X as unknown as typeof _X.default; would not work. Because of that, I wonder if it's possible to implement something like allowSyntheticDefaultImports, but for this very case. A new copiler option would allow folks to benefit from other features of "moduleResolution": "NodeNext" while waiting for fixes in typings upstream. WDYT?

@RyanCavanaugh
Copy link
Member

"moduleResolution": "bundler" (possibly plus some extra config flags?) should basically already do what you're asking for

@kachkaev
Copy link

kachkaev commented Jan 26, 2023

Looking at the comparison table between bundler and NodeNext in #51669, I’m not sure if bundler is as desired as NodeNext in a bunch of cases. It still enables extensionless imports, *.ts imports and directory index imports, which need to be banned for compatibility with ESM. When starting a greenfield project, it’s best to avoid these from the onset.

The only problem with "moduleResolution": "NodeNext" is the ugliness of const X = _X as unknown as typeof _X.default, which is an artifact of imperfect community typings. What downsides do you see in adding a flag that would allow switching to NodeNext but avoid using as typeof hack? It can be removed in a couple of years when the community has caught up.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jan 26, 2023

Maybe I'm a bit confused on what your actual runtime environment is. If it's actually Node 16+, then this code legitimately does not work and the _X as unknown as typeof _X.default workaround is just going to result in runtime errors, because the top-level object won't have the function behavior that is only present at X.default. I guess the implicit proposal is to bring over the properties, but not the call signatures?

What downsides do you see ...

It's going to be a nightmare if this flag gets common usage, because every single day someone will see a module declaration that says

export default function() { }
export function foo() { };

import it with

// OK, good
import { foo } from "my-module";
foo();

// Fails, TypeScript is clearly broken beyond repair,
// this is obviously a bug
import theFunction from "my-module";
theFunction();

Maybe if it's named allowImportingObjectsFromCommonJSESMInteropModuleButNotFunctionSignatures I'd feel comfortable shipping it, but the behavior just appears broken on its face and it's awkward to explain why we'd ship something that just helps module authors paper over their definitions being misconfigured instead of surfacing that problem right away.

Certainly there are going to be modules which don't have top-level function behavior and thus aren't particularly affected, but I'd argue this actually makes the situation worse, since it effectively 100% hides a configuration error. In the future I think someone might reasonably ask for a --noInteropImports flag and discover that npm is full of modules with configuration errors that we could be stamping out today instead by correctly identifying these problems.

The community wouldn't catch up because they wouldn't even realize they're behind.

@andrewbranch
Copy link
Member

andrewbranch commented Jan 26, 2023

Ryan and I have investigated two specific instances of this double-default problem in the last two days. One was ajv; the other was react-use-websocket. Both had users reporting that after default-importing the library, they had to access .default to get at the class (ajv) or function (react-use-websocket) they expected to be the simple default import. Both type definition files use export default, both libraries only ship CommonJS, and both use the __esModule marker. Lots of similarities.

Now for the differences. ajv’s JS uses this pattern:

module.exports = exports = Ajv;
Object.defineProperty(exports, "__esModule", { value: true });
exports.default = Ajv;

which means the Ajv is both the module.exports and the module.exports.default. Its typings indicate that it’s only the module.exports.default. So a user importing from an ESM file in Node will see this:

import Ajv from "ajv";
Ajv;                // Actually [class Ajv], but TypeScript thinks { default: [class Ajv] }
Ajv.default;        // [class Ajv], Node and TypeScript agree!
Ajv.default.default // Still [class Ajv], TypeScript doesn't know this exists

So, the typings aren’t exactly wrong, they’re just incomplete in a rather painful way. In this case, because of the interop strategy present in ajv’s JS, the sort of flag @kachkaev suggested would be safe to use.

react-use-websocket, on the other hand, only has this in their JS:

Object.defineProperty(exports, "__esModule", { value: true });
// ...
Object.defineProperty(exports, "default", { enumerable: true, get: function () { return use_websocket_1.useWebSocket; } });

useWebSocket only exists at module.exports.default, not at module.exports. So again, for a Node ESM importer:

import useWebSocket from "react-use-websocket";
useWebSocket;         // { default: [Function: useWebSocket] } Node and TypeScript agree!
useWebSocket.default; // [Function: useWebSocket] Node and TypeScript agree!

Here, the types are exactly right! Obviously, the author of react-use-websocket didn’t anticipate this function being loaded under Node’s ESM interop algorithm (and probably with good reason, since it’s intended for front-end use), but that’s a bit beside the point. A flag that pairs this default property with a Node ESM default import would not be safe to use here, and there are surely other libraries that fit this same pattern.

The problem is that we can’t tell the difference between these two cases without inspecting the actual JavaScript, which would completely defeat the point of having declaration files in the first place, and be very costly in terms of memory and compile time. Like Ryan, I’m sympathetic to this problem, but I 100% believe that if we added a flag to enable this kind of loose behavior, everyone would turn it on, library authors would use it as an excuse not to fix their definitions, and we’d never be able to remove it and fix the source of the problem.

@RyanCavanaugh RyanCavanaugh added the Rescheduled This issue was previously scheduled to an earlier milestone label Feb 1, 2023
tommy351 added a commit to tommy351/kosko that referenced this issue Apr 6, 2023
Fix the issue that `@kosko/env` cannot be resolved correctly
when `moduleResolution=nodenext` is set in `tsconfig.json`.

microsoft/TypeScript#50058
tommy351 added a commit to tommy351/kosko that referenced this issue Apr 7, 2023
Fix the issue that `@kosko/env` cannot be resolved correctly
when `moduleResolution=nodenext` is set in `tsconfig.json`.

microsoft/TypeScript#50058
jamesgpearce added a commit to jamesgpearce/ts that referenced this issue Jul 14, 2023
@jstasiak
Copy link

Leaving a note for the users of these two libraries as they seem to trigger this kind of error too with "moduleResolution": "NodeNext": big.js and fuse.js.

Sample code:

import Big from 'big.js'
import Fuse from 'fuse.js'

Big(1)
new Fuse(...)

The errors:

  Type 'typeof import("/project/node_modules/fuse.js/dist/fuse")' has no construct signatures

  Type 'typeof import("/project/node_modules/@types/big.js/index")' has no call signatures. 

@andrewbranch
Copy link
Member

andrewbranch commented Jul 24, 2023

This kind of error pretty much always indicates that the types use export default when they should use export = instead. That’s the case for fuse.js: https://arethetypeswrong.github.io/?p=fuse.js%406.6.2. I put up a fix: krisk/Fuse#731.

I guess big.js is on DefinitelyTyped but probably has the same problem. Will take a look at that next.


Edit: big.js fix is up DefinitelyTyped/DefinitelyTyped#66163

@throrin19
Copy link

throrin19 commented Dec 13, 2023

I have same problem for module @types/serverless if I try to use with nodeNext, I have this error :

Impossible de localiser le module 'serverless/aws' ou les déclarations de type correspondantes.ts(2307)

image

EDIT : I found how to fix. You have to do this :

import type { Functions } from 'serverless/aws.d.ts';

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status. Rescheduled This issue was previously scheduled to an earlier milestone
Projects
None yet
Development

No branches or pull requests