Private members cause issues when same class is used between dependencies

[octogonz]

TypeScript Version:

1.8.10

Actual behavior:

Suppose that library-a is an NPM package that exports a TypeScript class called MyClass, and two other packages consume this class. If those packages find the MyClass.d.ts definition via two different file paths on disk, and if MyClass contains a private member, then the compiler reports an error because it does not trust that the implementations are interchangeable.

This makes sense in general, however there are two everyday NPM usage scenarios where provably equivalent definitions must unavoidably have different file paths. In this situation the TypeScript error is not reasonable and may make life difficult for developers.

Code

Scenario 1: NPM link (similar to #7828, #6365, and #4800)

Developers commonly use npm link to test fixes before officially committing/publishing their NPM package. In my NpmLinkRepro from my TscNpmBug.tar.gz (see file attachment), the different file paths arise as follows:

• library-a exports MyClass

• library-b exports MyLibrary like this:

  import { MyClass } from 'library-a';
  
  export default class MyLibrary {
   public static getMyClass(): MyClass {
     return undefined;
   }
  }

• application tries to do this:

  import { MyClass } from 'library-a';
  import { MyLibrary } from 'library-b';
  
  let myClass: MyClass = MyLibrary.getMyClass();

• The error looks like this:

  src/index.ts(5,5): error TS2322: Type 'MyClass' is not assignable to type 'MyClass'.
  Types have separate declarations of a private property '_data'.
  

• The two symlinked filenames are:
  NpmLinkRepro\application\node_modules\library-a\lib\MyLibrary.d.ts
  NpmLinkRepro\application\node_modules\library-b\lib\MyLibrary.d.ts

• They both point to this physical file:
  NpmLinkRepro\library-b\lib\MyLibrary.d.ts

Scenario 2: NPM install

This scenario does not involve symlinking at all. In my NpmInstallRepro from my TscNpmBug.tar.gz (see attachment), the different file paths arise as follows:

• library-a 1.0 exports MyClass

• library-d exports MyLibrary like this:

  import { MyClass } from '@local/library-a';
  
  export default class MyLibrary {
   public static getMyClass(): MyClass {
     return undefined;
   }
  }

• library-e exports YourLibrary like this:

  import { MyClass } from '@local/library-a';
  
  export default class YourLibrary {
   public static setMyClass(myClass: MyClass): void {
   }
  }

• application's package.json imports both libraries and also a library-b which indirectly depends on version 2.0 of library-a

   "dependencies": {
     "typescript": "^1.8.10",
     "@local/library-b": "1.0.0",
     "@local/library-d": "1.0.0",
     "@local/library-e": "1.0.0"
   }

• As a result, the node_modules folder must unavoidably create two copies of library-a@1.0.0 (even after you run "npm dedupe" with npm version 3.0!):

  C:\NpmInstallRepro\application>tree /a
  +---lib
  +---node_modules
  |   +---.bin
  |   +---@local
  |   |   +---library-a
  |   |   |   \---lib
  |   |   +---library-b
  |   |   |   \---lib
  |   |   +---library-d
  |   |   |   +---lib
  |   |   |   \---node_modules
  |   |   |       \---@local
  |   |   |           \---library-a
  |   |   |               \---lib
  |   |   \---library-e
  |   |       +---lib
  |   |       \---node_modules
  |   |           \---@local
  |   |               \---library-a
  |   |                   \---lib
  |   \---typescript
  |       +---bin
  |       \---lib
  \---src
  

• application tries to do this:

   import { MyLibrary } from '@local/library-d';
   import { YourLibrary } from '@local/library-e';

   YourLibrary.setMyClass(MyLibrary.getMyClass());

• The error looks like this:

  src/index.ts(5,24): error TS2345: Argument of type 'MyClass' is not assignable to parameter of type 'MyClass'.
  Types have separate declarations of a private property '_data'.
  

• There are two physical files (containing equivalent definitions):
  NpmInstallRepro\application\node_modules@local\library-d\node_modules@local\library-a\lib\MyLibrary.d.ts
  NpmInstallRepro\application\node_modules@local\library-e\node_modules@local\library-a\lib\MyLibrary.d.ts

Running the Repro

Extract the attached TscNpmBug.tar.gz on a Windows PC and follow the steps in Instructions.txt. Note that the repro for scenario 2 requires you to install sinopia, which is a private NPM server where you can publish the test packages. (Also note that the batch files are very simple and intended to be run multiple times, so the first time they will report some errors when they try to delete output folders that don't exist yet.)

Expected behavior:

In both of these scenarios, I believe the compiler should determine that the MyLibrary.d.ts files are equivalent and treat them as the same class. If we're going to use classes in our public API at all, it seems that these scenarios must both work smoothly, otherwise developers will have to do weird workarounds when they randomly happen to hit a certain edge case for "npm install" or "npm link."

• For Scenario 1, the TypeScript compiler could follow symlinks and determine whether the physical file is the same
• For Scenario 2, I think the compiler will have to examine the containing package.json for each file path, and check whether the version numbers are exactly the same. If there is a concern about a package being locally modified, "npm install" injects other fields in package.json that the compiler could use to determine whether the package is pristine or not.

@mhegazy suggested that we might also be able to solve this using tsc path mapping. I will investigate that next and report whether it is a workable solution.

Failing that, we would also be okay with completely suppressing TS2345 and TS2322 as a temporary workaround.

[octogonz]

(Adding file attachment using .tar.gz because .zip is apparently not supported.)

TscNpmBug.tar.gz

[octogonz]

I renamed the title to be less dramatic sounding :-)

[octogonz]

I investigated the tsc path mapping feature. I tried adding "paths" to my "compilerOptions" in tsconfig.json, but I got this error:

error TS5023: Unknown compiler option 'paths'.

It seems that we can't use this feature because it's not supported by the current TypeScript compiler; it is a new feature in TypeScript 2.0 which is not released yet.

I worked around this by installing 1.9.0-dev.20160427. Then I modified my NpmInstallRepro\Application\tsconfig.json to look like this:

{
    "compilerOptions": {
        "target": "es5",
        "module": "commonjs",
        "jsx": "react",
        "declaration": true,
        "sourceMap": true,
        "experimentalDecorators": true,
        "outDir": "lib",
        "listFiles": true,
        "baseUrl": ".",
        "paths": {
          "library-a": [ "node_modules/@local/library-d/node_modules/@local/library-a/lib/index.d.ts" ]
        }        
    },
    "files": [
      "src/index.ts"
    ]
}

The error still occurs as before. I'm not sure this "paths" option is the right approach. We're not trying to remap the application project; we actually need to remap its indirect dependency "node_modules/@local/library-e" to find MyClass.d.ts in the "node_modules/@local/library-d" directory. Even if there was an option that would let us micromanage the module resolution in this way, it feels wrong. Basically our build scripts would be completely circumventing the module resolution algorithm that the compiler is supposed to provide for us.

So far it seems like our best solution is to patch the TypeScript compiler to completely disable the TS2345/TS2322 errors until there is an official fix.

[vladima]

@pgonzal #8486 addresses npm link part of the issue. For the second part can you please open another bug so it also can be tracked

[octogonz]

Will do, coming soon.

THANK YOU VERY MUCH for fixing this!