Dependencies linked with a symbolic link (by lerna) break the transpilation when using rxjs

[markvincze]

Setup
The issue happens in a repository containing two npm packages

.
|-- lerna.json
|-- package.json
|-- packages
    |-- lerna-typescript-demo-base:
          A project implemented in plain JavaScript, exporting a class (constructor function) with two functions.
          Also contains an index.d.ts with the typings.
    |-- lerna-typescript-demo-ts:
          A TypeScript project referencing lerna-typescript-demo-base.
          And lerna will set up the dependency in node_modules as a symlink to the other folder.

The class exported from lerna-typescript-demo-base, Foo has two functions, one is returning Bar[], and the other is returning an rxjs Observable, specifically Observable<Bar[]>.

In lerna-typescript-demo-ts we try to use both functions, and the transpilation of the usage of the one returning the Observable fails.

Code
A repro is uploaded here: https://github.com/markvincze/lerna-typescript-demo
The steps to reproduce are in the README.

Expected behavior:
Transpilation should succeed.

Actual behavior:
Transpilation fails with

error TS2345: Argument of type 'UnaryFunction<Observable<Bar[]>, Observable<Bar[]>>' is not assignable to parameter of type 'UnaryFunction<Observable<Bar[]>, Observable<Bar[]>>'.
  Types of parameters 'source' and 'source' are incompatible.
    Type 'Observable<Bar[]>' is not assignable to type 'Observable<Bar[]>'. Two different types with this name exist, but they are unrelated.

Comments:
I tried also with typescript@next, but get the same error.
I suspect this is related to the symlink dependency, because if I replace that with the ordinary in-place dependency, the error goes away. Also, this is only happening with the function returning an Observable, so it might also be related to rxjs somehow.
(I've seen a number of other issues about symlinks, but they were either closed as fixed, outdated, or just didn't have a clear repro.)

[mhegazy]

@andy-ms can you take a look.

[ghost]

Simplified repro:

// @noImplicitReferences: true

// @Filename: /node_modules/a/node_modules/foo/package.json
{
    "name": "foo",
    "version": "1.2.3"
}

// @Filename: /node_modules/a/node_modules/foo/index.d.ts
export class C {
    private x: number;
}

// @Filename: /node_modules/a/index.d.ts
import { C } from "foo";
export const o: C;

// @Filename: /node_modules/foo/use.d.ts
import { C } from "./index";
export function use(o: C): void;

// @Filename: /node_modules/foo/index.d.ts
export class C {
    private x: number;
}

// @Filename: /node_modules/foo/package.json
{
    "name": "foo",
    "version": "1.2.3"
}

// @Filename: /index.ts
import { use } from "foo/use";
import { o } from "a";

use(o);

The problem is that foo/use.ts imports from ./index, which isn't a global import and doesn't get a packageId. Then when we import from "foo" for real we don't realize we've already done so via ./index.

[markvincze]

Thanks for looking into this @andy-ms!
I'm still trying to get my head around this repro 😃, but just a quick question: so does this seem to be a bug, or is this the expected behavior, and something is set up incorrectly in my repro?

[markvincze]

@andy-ms,
Actually I've just found this issue (#6496), in which many people are complaining about—I think—the same issue, either caused by using lerna, or by using npm link.
The surprising thing to me is that in your repro, you're not using any symlinks, right? Because in my repro the problem specifically happens if the dependency is symlinked. If it's actually physically there, then the error goes away.

[markvincze]

A bit more information:
In my repro project, in the lerna-typescript-demo-ts package the node_modules is set up by lerna like this:

 ▲ lerna-typescript-demo-ts ls -la node_modules
total 36
drwxrwxr-x  6 mvincze mvincze  4096 jan  5 15:51 .
drwxrwxr-x  4 mvincze mvincze  4096 jan  5 15:51 ..
drwxrwxr-x  2 mvincze mvincze  4096 jan  5 15:51 .bin
lrwxrwxrwx  1 mvincze mvincze    32 jan  5 15:51 lerna-typescript-demo-base -> ../../lerna-typescript-demo-base
drwxrwxr-x 14 mvincze mvincze 12288 jan  5 15:51 rxjs
drwxrwxr-x  4 mvincze mvincze  4096 jan  5 15:51 symbol-observable
drwxrwxr-x  4 mvincze mvincze  4096 jan  5 15:51 typescript

So lerna sets up the reference to the other package in the monorepo as a symlink.
In this case the compilation fails, and indeed, if I do tsc --listFiles, I see this:

...
/home/mvincze/lerna-typescript-demo/packages/lerna-typescript-demo-ts/node_modules/rxjs/Observable.d.ts
...
/home/mvincze/lerna-typescript-demo/packages/lerna-typescript-demo-base/node_modules/rxjs/Observable.d.ts
...

So tsc is separately taking into account all the type definitions coming from rxjs in the two packages.

On the other hand, if I replace the symlink with the normal dependency (simply do rm -rf node_modules && npm install), then the compilation succeeds, and tsc --listFiles only outputs one instance of the rxjs typings.

...
/home/mvincze/lerna-typescript-demo/packages/lerna-typescript-demo-ts/node_modules/rxjs/Observable.d.ts
...

[markvincze]

I spent quite some time today debugging tsc to see what's happening.
The issue is that if the lerna-typescript-demo-base dependency is present as a symlink, then all the rxjs files are going to be processed twice by processImportedModules.

This is what I saw when debugging findSourceFile and processImportedModules.

If the dependency is a symlink

Processing starts with packages/lerna-typescript-demo-ts/index.ts
    Imports:
     - rxjs/operators -> resolves to packages/lerna-typescript-demo-ts/node_modules/rxjs/operators.d.ts
        This brings in all the rxjs d.ts files from lerna-typescript-demo-ts/node_modules/rxjs
     - lerna-typescript-demo-base -> resolves to packages/lerna-typescript-demo-base/index.d.ts (and `originalPath` contains the path of the symlink, packages/lerna-typescript-demo-ts/node_modules/lerna-typescript-demo-base/index.d.ts)
        Imports:
         - rxjs -> resolves to packages/lerna-typescript-demo-base/node_modules/rxjs/Rx.d.ts
            This brings in all the rxjs d.ts files a second time from lerna-typescript-demo-base/node_modules/rxjs
            The files are not recognized as duplicates, because they are in a different folder.
            And the `packageId` also doesn't match, because it's "rxjs/operators@5.5.6" in the first case, and "rxjs@5.5.6" in the second.

If the dependency is there physically

Processing starts with packages/lerna-typescript-demo-ts/index.ts
    Imports:
     - rxjs/operators -> resolves to packages/lerna-typescript-demo-ts/node_modules/rxjs/operators.d.ts
        This brings in all the rxjs d.ts files from lerna-typescript-demo-ts
     - lerna-typescript-demo-base -> resolves to packages/lerna-typescript-demo-ts/node_modules/lerna-typescript-demo-base/index.d.ts (and `originalPath` is undefined)
        Imports:
         - rxjs -> resolves to packages/lerna-typescript-demo-ts/node_modules/rxjs/Rx.d.ts
            This doesn't bring in any new dependencies, because all the file names will match with the already processed rxjs files.

[markvincze]

I found two workarounds for the issue (thanks to @evocateur for the first, and to @fahad19 for the second 😉).

• Instead of doing lerna bootstrap, if we do lerna bootstrap --hoist, it works. This is due to --hoist making lerna unify the common rxjs dependency, and puts it in the node_modules in the root, so there is no duplication any more.
• Add rxjs as a dependency in the root package.json, and move it to the peerDependencies in the actual packages. (This results in the same ultimate setup as what lerna bootstrap --hoist produces.)

So now I can work around the original issue, but I guess this is still possibly a problem with the TypeScript compiler?
cc @andy-ms