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

Duplicate type declarations with npm link #6496

Closed
kimamula opened this issue Jan 15, 2016 · 147 comments · Fixed by #16274 or #18185
Closed

Duplicate type declarations with npm link #6496

kimamula opened this issue Jan 15, 2016 · 147 comments · Fixed by #16274 or #18185
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue @types Relates to working with .d.ts files (declaration/definition files) from DefinitelyTyped

Comments

@kimamula
Copy link
Contributor

Using TypeScript 1.7.3.

Suppose I have the below npm packages.
The declaration files are generated by TypeScript compiler, and referred to from the other packages by means of the way described here.

package-a

ts src:

export default class ClassA {
  private foo: string;
  bar: number;
}

ts declaration:

declare class ClassA {
  private foo;
  bar: number;
}
export default ClassA;

package-b (depends on package-a):

ts src:

import ClassA from 'package-a';

namespace ClassAFactory {
  export function create(): ClassA {
    return new ClassA();
  }
}
export default ClassAFactory;

ts declaration:

import ClassA from 'package-a';

declare namespace ClassAFactory {
  function create(): ClassA;
}
export default ClassAFactory;

package-c (depends on package-a and package-b):

ts src:

import ClassA from 'package-a';
import ClassAFactory from 'package-b';

let classA: ClassA;
classA = ClassAFactory.create(); // error!!

The last line causes an error during compilation:

error TS2322: Type 'ClassA' is not assignable to type 'ClassA'.
Types have separate declarations of a private property 'foo'.

When I remove the line private foo; from the declaration of package-a, TypeScript does not emit any error.
However this workaround is a bit painful.

I understand that exposing private properties to declaration is by design (#1532).
I think TypeScript should ignore private properties when compiling variable assignment.
Or is there any better workaround for this?

@kimamula kimamula changed the title Private properties defined in an external package interrupt variable assignment Private properties defined in an external npm package interrupt variable assignment Jan 15, 2016
@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Jan 15, 2016
@RyanCavanaugh
Copy link
Member

There's only one root declaration of ClassA here, so this error shouldn't occur.

@kimamula
Copy link
Contributor Author

Well, sorry I found that this is related to npm link.

When I use npm link, packages are installed as below, as it simply creates symbolic links.

package-c
|
-- node_modules
    |
    -- package-a
    |   |
    |   -- index.d.ts
    |   |
    |   ...
    |
    -- package-b
        |
        -- index.d.ts
        |
        -- node_modules
        |   |
        |   -- package-a
        |       |
        |       -- index.d.ts
        |       |
        |       ...
        |
        ...

As shown, it looks like there are two different declaration files for package-a.
If I install packages normally by using npm install, this does not happen because the declaration of package-a is not included in package-b in this case.

I hope there would be some solution for this anyway, but it might be difficult and low priority.

@kimamula
Copy link
Contributor Author

kimamula commented Feb 2, 2016

I ended up not using npm link, and this does not matter any more for me.

@kimamula kimamula closed this as completed Feb 2, 2016
@RyanCavanaugh
Copy link
Member

Fair enough, but someone else might 😉

@RyanCavanaugh RyanCavanaugh reopened this Feb 2, 2016
@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus and removed Bug A bug in TypeScript labels Feb 4, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Feb 4, 2016

there are actually two files on disk with two declarations of ClassA. so the error is correct. but we need to consider node modules when we compare these types. this issue has been reported before in #4800, for Enums we changed the rule to a semi-nominal check. possibly do the same for classes.

@rezonant
Copy link

+1 on this with TS 1.7.5 with all relevant packages NPM-linked. I tried to construct a testcase that exhibits the issue but could not. No matter what I tried, TS was fine with the scenario I see failing with TS2345 in my application, and as far as I could tell, all copies of the problematic .d.ts file were symlinks to the same file, so there should not have been differing declarations within the type. It would be nice however if the error emitted by Typescript referenced the files which declared the two incompatible types, as that might shed light on something I'm not considering. Right now it says there are two definitions but does nothing to help the developer pinpoint the issue.

As a workaround you can use <any> on the conflicting expression to skip the type check. Obviously this might require you to do another type annotation where you might not have had to before. I hope someone can isolate this issue at some point.

EDIT: made it clear that NPM link is at play in my case

@rezonant
Copy link

Noticed TS 1.8 is available, upgraded and the issue still exists in that version as well.

@huerlisi
Copy link
Contributor

Thanks for all the work in analyzing and documenting this issue. We're having the same problem in some of our code bases. We ported some projects to properly use package.json dependencies but are now seeing this when using npm link during development.

Is there anything I can help to solve this issue?

@seansfkelley
Copy link

seansfkelley commented Oct 13, 2016

I'm using Lerna which symlinks packages around and seeing the issue there as well. Typescript version 2.0.3.

Unfortunately Lerna and its symlinks are a hard requirement, so I used this nasty workaround to get this to compile fine while still being properly type-checkable by consumers:

export class MyClass {
  constructor(foo: Foo) {
    (this as any)._foo = foo;
  }

  get foo() {
    return (this as any)._foo as Foo;
  }
}

The class is very small so it wasn't that arduous, and I don't expect it to change really ever, which is why I consider this an acceptable workaround.

@xogeny
Copy link

xogeny commented Oct 14, 2016

FYI, I've also ended up here as a result of using npm link and getting this error. Has anybody found a workaround for this?

@mhegazy
Copy link
Contributor

mhegazy commented Oct 14, 2016

@xogeny can you elaborate on how npm link is causing this issue for you?

@xogeny
Copy link

xogeny commented Oct 16, 2016

@mhegazy Well I started getting these errors like the one above (except I was using Observable from rxjs, i.e., "Type 'Observable' is not assignable to type 'Observable'). This, of course, seemed odd because the two I was referencing Observable from exactly the same version of rxjs in both modules. But where the types "met", I got an error. I dug around and eventually found this issue where @kimamula pointed out that if you use npm link, you'll get this error. I, like others, worked around this (in my case, I created a duplicate interface of just the functionality I needed in one module, rather than references rxjs).

Does that answer your question? I ask because I don't think my case appears any different than the others here so I'm not sure if this helps you.

@mhegazy
Copy link
Contributor

mhegazy commented Oct 17, 2016

We have done work in TS2.0 specifically to enable npm link scenarios (see #8486 and #8346). Do you have a sample where i can look at where npm link is still not working for you?

@xogeny
Copy link

xogeny commented Oct 17, 2016

Huh. I'm running 2.0.3 (I checked). I'll try to create a reproducible case.

By the way, you should follow up on these threads since they imply that this is still an issue as of TS 2.0:

ReactiveX/rxjs#1858
ReactiveX/rxjs#1744

@seansfkelley
Copy link

seansfkelley commented Oct 17, 2016

The issue I'm seeing in my Lerna repo is somewhat involved, so I made a stripped-down version of it at https://github.com/seansfkelley/typescript-lerna-webpack-sadness. It might even be webpack/ts-loader's fault, so I've filed TypeStrong/ts-loader#324 over there as well.

@d-akara
Copy link

d-akara commented Nov 19, 2017

@andy-ms I'm still having this issue. This occurs for me when attempting to use npm link between any vscode extensions I'm building. They all import vscode which causes duplicate identifier errors.
TS Version 2.7.0-dev.20171118

@davidibl
Copy link

For me it worked map for example "rxjs/*" to a specific rxjs folder within node_modules in paths section of tsconfig file.
Now everything works fine with npm link.

@JoshuaKGoldberg
Copy link
Contributor

I've also seen this happen when going from symlinked to non-symlinked from a version update link to a package.

See this SO question too: https://stackoverflow.com/questions/38168581/observablet-is-not-a-class-derived-from-observablet

@ghost
Copy link

ghost commented Dec 13, 2017

@dakaraphi @JoshuaKGoldberg Could you provide instructions to reproduce these scenarios?

With the new behavior we should not include a package if we've already seen another package with the same "version" value in its package.json. If you have multiple installations with different versions, you will get two different copies of the module. That might explain why a version update would break this, if it only affected one of two installations.

@d-akara
Copy link

d-akara commented Dec 13, 2017

@andy-ms The example I have are the following repositories:

  1. https://github.com/dakaraphi/vscode-extension-fold
  2. https://github.com/dakaraphi/vscode-extension-common

I use local npm install to reference vscode-extension-common from vscode-extension-fold

If you checkout these repositories, they currently work because I have a path mapping workaround in the package.json of vscode-extension-fold. However, if I understand correctly, I should not need that workaround.

@ghost
Copy link

ghost commented Dec 13, 2017

@dakaraphi Thanks! It looks like the error is due to vscode.d.ts being written as a global, ambient declaration rather than as an external module. I've created microsoft/vscode-extension-vscode#90.

cvuorinen added a commit to cvuorinen/preact-material-components-typings-test that referenced this issue Dec 13, 2017
Workaround for typescript duplicate identifier errors when using npm link (to get preact-material-components typings branch)
More info: microsoft/TypeScript#6496
Workaround: ReactiveX/rxjs#1858
guw added a commit to forcedotcom/salesforcedx-vscode that referenced this issue Jan 15, 2018
guw added a commit to forcedotcom/salesforcedx-vscode that referenced this issue Jan 16, 2018
guw added a commit to forcedotcom/salesforcedx-vscode that referenced this issue Jan 19, 2018
guw added a commit to forcedotcom/salesforcedx-vscode that referenced this issue Jan 23, 2018
@SamVerschueren
Copy link

This still doesn't work for me when I'm trying to link 2 packages, each having a dependency on rxjs. I'm using rxjs@5.5.6 and typescript@2.6.1. Both packages are using the exact same version. Does anyone have a workaround for this?

@ghost
Copy link

ghost commented Jan 24, 2018

@SamVerschueren Could you provide specific instructions to reproduce the error? Also test with typescript@next.

@SamVerschueren
Copy link

@andy-ms I'll see what I can do!

@SamVerschueren
Copy link

@andy-ms Here's a small reproduction repository https://github.com/SamVerschueren/ts-link-6496. I used yarn@1.0.0 to reproduce this.

  1. Install both the dependencies for mod-a and mod-b
  2. Compile mod-b with yarn build
  3. Compile mod-a with yarn build

Step 3 will fail with the following error

src/index.ts(7,15): error TS2345: Argument of type 'UnaryFunction<Observable<string>, Observable<string>>' is not assignable to parameter of type 'UnaryFunction<Observable<string>, Observable<string>>'.
  Types of parameters 'source' and 'source' are incompatible.
    Type 'Observable<string>' is not assignable to type 'Observable<string>'. Two different types with this name exist, but they are unrelated.
      Property 'buffer' is missing in type 'Observable<string>'.
src/index.ts(7,47): error TS7006: Parameter 'result' implicitly has an 'any' type.

guw added a commit to forcedotcom/salesforcedx-vscode that referenced this issue Jan 29, 2018
guw added a commit to forcedotcom/salesforcedx-vscode that referenced this issue Jan 30, 2018
guw added a commit to forcedotcom/salesforcedx-vscode that referenced this issue Feb 1, 2018
guw added a commit to forcedotcom/salesforcedx-vscode that referenced this issue Feb 2, 2018
guw added a commit to forcedotcom/salesforcedx-vscode that referenced this issue Feb 5, 2018
@kgroat
Copy link

kgroat commented Apr 26, 2018

Still running into this issue with typescript@2.8.1. Also tried typescript@next and had the same issue.

The root of the problem seems to be that linked packages still reference the type definitions in their own local node_modules rather than using the typings from the node_modules into which they are linked, when possible. That combined with the fact that:

  1. globals cannot be re-defined
    • this causes the compiler to complain when the global is defined both in the parent project's node_modules as well as the node_modules in the linked package
  2. classes that are otherwise identical cannot be assigned to one another
    • this causes the compiler to complain when trying to assign a variable whose type is a class from the parent project's node_modules to a value returned by the linked package whose type is defined in its own node_modules

I've been able to work around this issue using the paths config variable. For modules whose definitions come from @types/*, as suggested here, you can simply use:

"paths": {
  "*": ["node_modules/@types/*", "*"]
}

In the case where you run into this issue with a package that comes bundled with type definitions which define classes or globals, you have to add them manually. For example, rxjs:

"paths": {
  "rxjs": ["node_modules/rxjs"],
  "rxjs/*": ["node_modules/rxjs/*"]
}

@Elephant-Vessel
Copy link

Elephant-Vessel commented May 14, 2018

I also experience problems with symlinks when I add a local package, using TS 2.8.3:

},
"devDependencies": {
    "@types/MyLib": "file:../MyLib/bin/npm/@types"
},

Since v3, npm are apparently installing these with symlinks instead of copying the files.

However, when I try to compile, the compiler sees the linked definition file as two separate and conflicting files:

node_modules\@types\MyLib\index.d.ts(3,11): error TS2300: Duplicate identifier 'Foo'.
C:/MySolution/MyLib/bin/npm/@types/index.d.ts(3,11): error TS2300: Duplicate identifier 'Foo'.

If I manually copy the files instead, it works as expected. I can work around this by setting typeRoots: ["./node_modules/**/"]

@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
stephenh referenced this issue in ffMathy/FluffySpoon.JavaScript.Testing.Faking Feb 26, 2019
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue @types Relates to working with .d.ts files (declaration/definition files) from DefinitelyTyped
Projects
None yet