microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.66k stars 12.44k forks source link

Type definitions overshadowing #37316

Open ghost opened 4 years ago

ghost commented 4 years ago

TypeScript Version: 3.8.3

(also tried 3.9.0-dev.20200310)

Search Terms: "Module '"immutable"' has no exported member"

Code

(not self contained)

import { hash } from "immutable";

export function test(): number {
    return hash(1);
}

Actual behavior:

src/index.ts:1:10 - error TS2305: Module '"immutable"' has no exported member 'hash'.

1 import { hash } from "immutable";
           ~~~~

even though grep "export function hash" node_modules/immutable/dist/immutable.d.ts reveals export function hash(value: any): number;

Because it uses type definitions in node_modules/@types/draft-js/node_modules/immutable/dist/immutable.d.ts instead of node_modules/immutable/dist/immutable.d.ts

Demo:

sadly I did not find any way to put it in a playground, in any case: https://gitlab.com/rawieo/issue_demo_1

Related Issues:

Furetur commented 4 years ago

Had the same issue with create react app.

RyanCavanaugh commented 4 years ago

Someone should probably update https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/draft-js/package.json

RyanCavanaugh commented 4 years ago

@sheetalkamat thoughts on this? It looks like the root cause is that we look up "immutable" from the context of @types/draft-js, resolve to the nested copy under @types/draft_js/node_modules/, and then cache that so that subsequent resolutions from index.ts get the "wrong" copy instead of the version in the local area.

Maybe we need to augment the cache key in the case where we resolve to a nonlocal node_modules (whatever the definition of "nonlocal" is...)

sheetalkamat commented 4 years ago

The issue here is the configuration..

======== Resolving module 'immutable' from 'c:/temp/issue_demo_1/src/index.ts'. ========
======== Module name 'immutable' was successfully resolved to 'c:/temp/issue_demo_1/node_modules/immutable/dist/immutable-nonambient.d.ts' with Package ID 'immutable/dist/immutable-nonambient.d.ts@4.0.0-rc.12'. ========

======== Resolving module 'immutable' from 'c:/temp/issue_demo_1/node_modules/@types/draft-js/index.d.ts'. ========
======== Module name 'immutable' was successfully resolved to 'c:/temp/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable/dist/immutable.d.ts' with Package ID 'immutable/dist/immutable.d.ts@3.7.6'. ========

So there are two files included for same module different version. We do not support two different versions of the modules that well since they can interplay with each other. In this case c:/temp/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable/dist/immutable.d.ts has ambient module definition for immutable so thats going to win when you say import (given the other resolution is not to a file called immutable.d.ts but to immutable-nonambient.d.ts ) and hence the error

ghost commented 4 years ago

Did i get it right, that it prefers node_modules/@types/draft-js/node_modules/immutable/dist/immutable.d.ts over node_modules/immutable/dist/immutable-nonambient.d.ts because the former is ambient and the latter is nonambient? Why doesn't it prefer node_modules/immutable/dist/immutable.d.ts which is also ambient?

sheetalkamat commented 4 years ago

@Rawieo there is typings in package.json that redirects to ambient one .. the immutable.d.ta file has definition of module “imutable” that’s ambient.. you can see more details of which file module resolution picks up using —traceResolutions

Furetur commented 4 years ago

@sheetalkamat What is the best solution to this problem in your opinion? In other words, how can I tell TypeScript to use node_modules/immutable/dist/immutable-nonambient.d.ts instead of node_modules/@types/draft-js/node_modules/immutable/dist/immutable.d.ts

ghost commented 4 years ago

@sheetalkamat

there is typings in package.json that redirects to ambient one

Which one package.json? Does it not consider node_modules/immutable/dist/immutable.d.ts because node_modules/immutable/dist/immutable-nonambient.d.ts was specified in typings?

I examined both packages (node_modules/immutable and node_modules/@types/draft-js/node_modules/immutable) and I still do not understand why it should prefer "fake" definitions (node_modules/@types/draft-js/node_modules/immutable/dist/immutable.d.ts) over "the real" definitions (whichever one it would be node_modules/immutable/dist/immutable-nonambient.d.ts or node_modules/immutable/dist/immutable.d.ts) when "the real" definitions are explicitly specified in "the real" package.json and also exist as immutable.d.ts which should be, like the "fake" definitions (node_modules/@types/draft-js/node_modules/immutable/dist/immutable.d.ts), resolved by default.

If that's really working as intended then it should not be intended, because that would mean that any type definition ever could be accidentally broken.

sheetalkamat commented 4 years ago

Using --traceResolution will show you whats going on with the module resolution.

======== Resolving module 'immutable' from 'c:/temp/issue_demo_1/src/index.ts'. ========
Module resolution kind is not specified, using 'NodeJs'.
Loading module 'immutable' from 'node_modules' folder, target file type 'TypeScript'.
Directory 'c:/temp/issue_demo_1/src/node_modules' does not exist, skipping all lookups in it.
Found 'package.json' at 'c:/temp/issue_demo_1/node_modules/immutable/package.json'.
'package.json' does not have a 'typesVersions' field.
File 'c:/temp/issue_demo_1/node_modules/immutable.ts' does not exist.
File 'c:/temp/issue_demo_1/node_modules/immutable.tsx' does not exist.
File 'c:/temp/issue_demo_1/node_modules/immutable.d.ts' does not exist.
'package.json' has 'typings' field 'dist/immutable-nonambient.d.ts' that references 'c:/temp/issue_demo_1/node_modules/immutable/dist/immutable-nonambient.d.ts'.
File 'c:/temp/issue_demo_1/node_modules/immutable/dist/immutable-nonambient.d.ts' exist - use it as a name resolution result.
Resolving real path for 'c:/temp/issue_demo_1/node_modules/immutable/dist/immutable-nonambient.d.ts', result 'c:/temp/issue_demo_1/node_modules/immutable/dist/immutable-nonambient.d.ts'.
======== Module name 'immutable' was successfully resolved to 'c:/temp/issue_demo_1/node_modules/immutable/dist/immutable-nonambient.d.ts' with Package ID 'immutable/dist/immutable-nonambient.d.ts@4.0.0-rc.12'. ========
ghost commented 4 years ago

Yes. And then it does

======== Resolving module 'immutable' from '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/index.d.ts'. ========
Module resolution kind is not specified, using 'NodeJs'.
Loading module 'immutable' from 'node_modules' folder, target file type 'TypeScript'.
Found 'package.json' at '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable/package.json'.
'package.json' does not have a 'typesVersions' field.
File '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable.ts' does not exist.
File '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable.tsx' does not exist.
File '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable.d.ts' does not exist.
'package.json' does not have a 'typings' field.
'package.json' does not have a 'types' field.
'package.json' has 'main' field 'dist/immutable.js' that references '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable/dist/immutable.js'.
File '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable/dist/immutable.js' exist - use it as a name resolution result.
File '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable/dist/immutable.js' has an unsupported extension, so skipping it.
Loading module as file / folder, candidate module location '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable/dist/immutable.js', target file type 'TypeScript'.
File '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable/dist/immutable.js.ts' does not exist.
File '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable/dist/immutable.js.tsx' does not exist.
File '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable/dist/immutable.js.d.ts' does not exist.
File name '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable/dist/immutable.js' has a '.js' extension - stripping it.
File '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable/dist/immutable.ts' does not exist.
File '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable/dist/immutable.tsx' does not exist.
File '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable/dist/immutable.d.ts' exist - use it as a name resolution result.
Resolving real path for '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable/dist/immutable.d.ts', result '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable/dist/immutable.d.ts'.
======== Module name 'immutable' was successfully resolved to '/home/rawieo/Projects/issue_demo_1/node_modules/@types/draft-js/node_modules/immutable/dist/immutable.d.ts' with Package ID 'immutable/dist/immutable.d.ts@3.7.6'. ========

and forgets completely about node_modules/immutable/dist/immutable-nonambient.d.ts

And that's the whole point of this issue -- is it intentional? Should it do that?

Should it really completely drop the exact required type definitions in favor of something found by accident?

@RyanCavanaugh can this be avoided with some userspace configuration?

typescript-bot commented 4 years ago

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

RyanCavanaugh commented 4 years ago

@sheetalkamat I think we need something that would allow this to not happen, even if it's not feasible to make it work out-of-the-box. Any ideas?

FilipMessa commented 4 years ago

Hi, @Rawieo Did you solve this somehow?

ghost commented 4 years ago

@FilipMessa no, sadly I had no luck searching for solution in user-space. I was considering fixing the compiler myself, but I have no idea how so I didn't even try.