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

Cannot import some packages when tsconfig.json specifies "module": "nodenext" #46770

Closed
ide opened this issue Nov 11, 2021 · 6 comments · Fixed by #47893
Closed

Cannot import some packages when tsconfig.json specifies "module": "nodenext" #46770

ide opened this issue Nov 11, 2021 · 6 comments · Fixed by #47893
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@ide
Copy link

ide commented Nov 11, 2021

Bug Report

When a TypeScript project specifies "module": "nodenext" (or "node12") in tsconfig.json, tsc emits errors saying it can't find imported modules or that the exports aren't the right types.

I've put together a demo that shows this with the "ip" and "nullthrows" packages. "ip" doesn't ship with TypeScript declarations and needs "@types/ip", while "nullthrows" comes with declarations. The precise errors are included below but in summary, tsc can't find "ip" and it thinks "nullthrows" is not callable despite the default export being a function.

Playing around, when I add "type": "module" to nullthrows's package.json, its error goes away. When I change ip's package.json from "main": "lib/ip" to "main": "lib/ip.js", its issue goes away. Writing this out, these are likely two different issues with the shared symptom of an ESM package not being able to import either of them.

🔎 Search Terms

esm module nodenext

🕗 Version & Regression Information

[email protected]

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about modules. The issue is present in the 4.5.0 betas as well, though I understand that ESM support via nodenext/node12 is being deferred.

⏯ Playground Link

Minimal repo: https://github.com/ide/typescript-esm-demo. Dependencies are needed to show the effect.

💻 Code

import * as ip from 'ip';
import nullthrows from 'nullthrows';

export function getAddress(): string {
  return nullthrows(ip.address());
}

🙁 Actual behavior

tsc outputs:

index.ts:1:21 - error TS2307: Cannot find module 'ip' or its corresponding type declarations.

1 import * as ip from 'ip';
                      ~~~~

index.ts:5:10 - error TS2349: This expression is not callable.
  Type 'typeof import("/private/tmp/typescript-esm-demo/node_modules/nullthrows/nullthrows")' has no call signatures.

5   return nullthrows(ip.address());
           ~~~~~~~~~~


Found 2 errors.

🙂 Expected behavior

tsc should compile without errors.

@ide ide changed the title Cannot import some packages that use "export" in their .d.ts files but don't have "type": "module" in package.json when tsconfig.json specifies "module": "nodenext" Cannot import some packages that use "export" in their .d.ts files when tsconfig.json specifies "module": "nodenext" Nov 11, 2021
@ide ide changed the title Cannot import some packages that use "export" in their .d.ts files when tsconfig.json specifies "module": "nodenext" Cannot import some packages when tsconfig.json specifies "module": "nodenext" Nov 11, 2021
@andrewbranch
Copy link
Member

Ok, so the ip issue is because the types field of the @types/ip package.json is missing a file extension. We started making the published packages include the .d.ts extension quite a while ago, but I guess that package hasn’t been updated since. In ESM module resolution, extensions are required on import paths, and it seems that behavior spilled over into our lookups under the "types" field as well. I’m not sure if that’s intentional or not—@weswigham? Either way, we may want to consider doing a onetime fixup of all existing DT packages that don’t have an extension in their "types" field (cc @sandersn).

The issue with nullthrows looks like a different bug that I’m surprised we haven’t seen before now? Even though you’re using a default import, the type you’re getting is the module type with the default member.

(This whole thing is made more confusing by the fact that the editor glitches back and forth between CJS and ESM mode resolution because of #46373/#46396.)

@Silic0nS0ldier
Copy link

Silic0nS0ldier commented Nov 25, 2021

I'm also seeing this issue with [email protected] (includes types), which uses the index.js lookup behaviour (no main needed). Adding "type": "module" seems to allow the package to be seen, as TypeScript correctly reports that there is no default export.

Module resolution trace
======== Resolving module 'get-stream' from '/Users/jordan/source/git-ext/packages/git/src/cli-helpers.ts'. ========
Explicitly specified module resolution kind: 'Node12'.
File '/users/jordan/source/git-ext/packages/git/src/package.json' does not exist according to earlier cached lookups.
File '/users/jordan/source/git-ext/packages/git/package.json' exists according to earlier cached lookups.
Loading module 'get-stream' from 'node_modules' folder, target file type 'TypeScript'.
Directory '/Users/jordan/source/git-ext/packages/git/src/node_modules' does not exist, skipping all lookups in it.
Found 'package.json' at '/Users/jordan/source/git-ext/packages/git/node_modules/get-stream/package.json'.
'package.json' does not have a 'typesVersions' field.
'package.json' does not have a 'typings' field.
'package.json' does not have a 'types' field.
'package.json' does not have a 'main' field.
Directory '/Users/jordan/source/git-ext/packages/node_modules' does not exist, skipping all lookups in it.
Directory '/Users/jordan/source/node_modules' does not exist, skipping all lookups in it.
Directory '/Users/jordan/node_modules' does not exist, skipping all lookups in it.
Directory '/Users/node_modules' does not exist, skipping all lookups in it.
Directory '/node_modules' does not exist, skipping all lookups in it.
File '/users/jordan/source/git-ext/packages/git/src/package.json' does not exist according to earlier cached lookups.
File '/users/jordan/source/git-ext/packages/git/package.json' exists according to earlier cached lookups.
Loading module 'get-stream' from 'node_modules' folder, target file type 'JavaScript'.
Directory '/Users/jordan/source/git-ext/packages/git/src/node_modules' does not exist, skipping all lookups in it.
File '/Users/jordan/source/git-ext/packages/git/node_modules/get-stream/package.json' exists according to earlier cached lookups.
'package.json' does not have a 'main' field.
Directory '/Users/jordan/source/git-ext/packages/node_modules' does not exist, skipping all lookups in it.
Directory '/Users/jordan/source/node_modules' does not exist, skipping all lookups in it.
Directory '/Users/jordan/node_modules' does not exist, skipping all lookups in it.
Directory '/Users/node_modules' does not exist, skipping all lookups in it.
Directory '/node_modules' does not exist, skipping all lookups in it.
======== Module name 'get-stream' was not resolved. ========

It definitely finds the package, but for some reason is writing it off entirely as a valid resolution.

@Silic0nS0ldier
Copy link

I'm not certain, but I think I found the issue.

// esm mode resolutions don't do package `index` lookups
if (!(state.features & NodeResolutionFeatures.EsmMode)) {

This check (which is used in a few places) is causing get-stream to be treated as a ESM package in places (state.features & NodeResolutionFeatures.EsmMode is truthy). As a result no attempt to complete the path .../node_modules/get-stream/index is made.

For the purpose of debugging, I added || candidate.search(/get-stream/g) to the ESM checks to bypass guard just for get-stream. When attempting to compile with that change, get-stream was successfully resolved.

I'm not certain what the purpose of state.features is (to carry forward what Node resolution features are in use in the project, or what node resolution features apply to the package), but it looks like it is either wrong or being misused.

@Silic0nS0ldier
Copy link

I'm not certain what the purpose of state.features is...

This is directly controlled by the set moduleResolution, so the intent is to carry forward what Node resolution features are in use. Given this, its looking like those state.features & NodeResolutionFeatures.EsmMode checks are making a bad assumption about the package being imported when the requested path lacks a file extension in at least once place.

With some more tracing I've isolated the branch causing the issue.

loadNodeModuleFromDirectoryWorker(
extensions,
candidate,
onlyRecordFailures,
state,
packageInfo && packageInfo.packageJsonContent,
packageInfo && packageInfo.versionPaths
);

And further

// esm mode resolutions don't do package `index` lookups
if (!(state.features & NodeResolutionFeatures.EsmMode)) {
return loadModuleFromFile(extensions, indexPath, onlyRecordFailuresForIndex, state);
}

The decision to not include index lookups depends on the package. In this context any kind of node module can be encountered. However loadModuleFromFile also has an extensionless lookup guard that needs to be dealt with.

@Silic0nS0ldier
Copy link

I don't propose this as a "solution" (excuse the comments everywhere, I didn't clean up before committing) but this does fix the resolution issues for me. Silic0nS0ldier@dd8d8d9

otakustay added a commit to otakustay/typanion that referenced this issue Dec 25, 2021
See microsoft/TypeScript#46770 (comment) for detail, in `nodenext` module resolution it requires a full filename including extension
otakustay added a commit to otakustay/clipanion that referenced this issue Dec 25, 2021
See microsoft/TypeScript#46770 (comment) for detail, in `nodenext` module resolution it requires a full filename including extension
otakustay added a commit to otakustay/pretty-bytes that referenced this issue Dec 25, 2021
See microsoft/TypeScript#46770 (comment) for detail, in `nodenext` module resolution it requires a full filename including extension
otakustay added a commit to otakustay/socket.io that referenced this issue Dec 25, 2021
See microsoft/TypeScript#46770 (comment) for detail, in `nodenext` module resolution we need to specify `types` inside `exports` field.
otakustay added a commit to otakustay/socket.io-client that referenced this issue Dec 26, 2021
… resolution

See microsoft/TypeScript#46770 (comment) for detail, in `nodenext` module resolution it requires a `types` field in `exports` with full filename including extension
darrachequesne pushed a commit to socketio/socket.io-client that referenced this issue Dec 28, 2021
…le resolution (#1522)

See [1] for detail, in `nodenext` module resolution it requires a
`types` field in `exports` with full filename including extension.

[1]: microsoft/TypeScript#46770 (comment)

Reference: https://www.typescriptlang.org/tsconfig/#module
darrachequesne pushed a commit to socketio/socket.io that referenced this issue Dec 28, 2021
…le resolution (#4228)

See [1] for detail, in `nodenext` module resolution it requires a
`types` field in `exports` with full filename including extension.

[1]: microsoft/TypeScript#46770 (comment)
sunrise30 added a commit to sunrise30/socket.io-client that referenced this issue Jan 8, 2022
…le resolution (#1522)

See [1] for detail, in `nodenext` module resolution it requires a
`types` field in `exports` with full filename including extension.

[1]: microsoft/TypeScript#46770 (comment)

Reference: https://www.typescriptlang.org/tsconfig/#module
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Feb 14, 2022
@weswigham
Copy link
Member

nullthrows types are wrong. Well, incomplete. They say it's a cjs module which exports a default member which is a function, which is correct, it does, but it also exports that same function as the module itself. It's types should look something more like

declare function nullthrows(x: whatever): whatever;
declare namespace nullthrows {
    export {nullthrows as default};
}
export = nullthrows;

or something else to that effect.

As for @types/ip, yeah, that's an issue. We shouldn't be requiring extensions in main (of cjs packages); we're just applying our esm resolution logic at a step where we shouldn't because we reuse resolution machinery (at least when importing cjs packages - extensionless mains actually aren't supported in node for esm packages!). I have a tiny fix for that up at #47893.

dzad pushed a commit to dzad/socket.io that referenced this issue May 29, 2023
…le resolution (socketio#4228)

See [1] for detail, in `nodenext` module resolution it requires a
`types` field in `exports` with full filename including extension.

[1]: microsoft/TypeScript#46770 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants