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

npm-linked modules (duplicates or not) are not working. #31527

Open
5 tasks done
trusktr opened this issue May 22, 2019 · 18 comments
Open
5 tasks done

npm-linked modules (duplicates or not) are not working. #31527

trusktr opened this issue May 22, 2019 · 18 comments
Assignees
Labels
Needs More Info The issue still hasn't been fully clarified
Milestone

Comments

@trusktr
Copy link

trusktr commented May 22, 2019

Re:

Search Terms

"typescript ignore npm linked dependencies"

Suggestion

I'd like for TypeScript to automatically work with npm linked modules.

Use Cases

Prevents developers from wasting much time.

Examples

npm link anything, and it just works.

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

More details

I'm on TS v3.4.5.

Even if a project's npm linked module has the exact same version of a dependency,

> npm ls react
my-project@2.1.1 /Users/trusktr/src/my-project
├─┬ my-lib@2.0.0 -> /Users/trusktr/src/my-lib
│ └── react@16.8.6
└── react@16.8.6

TypeScript complains. For example:

[0] ERROR in /Users/trusktr/src/my-lib/node_modules/@types/react/index.d.ts(2963,13):
[0] TS2717: Subsequent property declarations must have the same type.  Property 'view' must be of type 'SVGProps<SVGViewElement>', but here has type 'SVGProps<
SVGViewElement>'.

This is quite problematic, especially when needing to link 3, 4, or 5 modules at once during development, and especially more so when those linked modules have other npm linked modules. This issue grinds the dev experience to a halt. This npm link workflow is commonplace in with plain JS projects using NPM.

I've tried all workarounds mentioned in the other four issues, but each workaround brings with it other problems, or just don't work.

One workaround is to remove the duplicate dependency in the npm linked module, and this causes everything to work fine in the project, but then typings don't work while working on the npm linked module. So to get back to developing the npm linked module, we need to reinstall the missing dependencies. Then when we switch back to the project we need to delete the dependencies in the linked package. Then when we switch back to the linked package... etc...

I believe that TypeScript should deeply embrace how npm link works, and provide a built in solution so that TypeScript users never run into this again.

@trusktr
Copy link
Author

trusktr commented May 22, 2019

I think the solution involves TypeScript treating linked modules as if they are not linked modules (if it doesn't already), and use a top-down module resolution (file traversal) so that it will pick the top-most libraries from which to get dependencies (types) from.

If there's a dependency constraint that can not be matched, then it will use a lower-in-the-tree module (just like NPM) to get types from, and get those types only for the package the needs those types from that specific version (regardless if the types are .d.ts or .ts).

This would essentially work the same way as NPM's flattening approach, where it flattens everything unless there are dependency forks (i.e. version ranges can't be satisfied for two or more packages that need incompatible versions of the same lib), except TypeScript won't do any flattening, TypeScript will simply trust the flattened structure that NPM will already have put in place.

@trusktr
Copy link
Author

trusktr commented May 22, 2019

I meant to put I'm on TS v3.4.5 up there. Edited.

@trusktr
Copy link
Author

trusktr commented May 22, 2019

The workaround I mentioned, deleting the dependency in the linked module, doesn't work in this case (I thought it did before, but, now I'm not sure). Now I get

[1] ../my-lib/node_modules/@material-ui/core/styles/withStyles.d.ts:1:24 - error TS2307: Cannot find module 'react'.
[1]
[1] 1 import * as React from 'react';

As in my suggestion, if TypeScript were to treat the linked module as if it were not linked, and continued to traverse up the tree in the main project where tsc is running from, then it would find react higher up (as a dependency of the project that has the linked module). This is how NPM module resolution works at runtime if we delete the dependency in the linked module. TS is not doing the same at compile time.

@trusktr
Copy link
Author

trusktr commented May 22, 2019

However, because we don't want to delete the dependencies in the linked modules (so that we can also go and work locally inside the linked modules when making changes to those modules), this is why I suggested a top-down approach which should ultimately result in the same modules being resolved as with NPM's flattening when no modules are linked).

Note, when modules are linked, NPM's resolution is bottom-up, and that can introduce other problems, which is something I wish NPM would fix.

@trusktr
Copy link
Author

trusktr commented May 22, 2019

EDIT: The workaround does work: I was deleting react instead of @types/react in the linked module. If I delete only @types/react in the linked module, then the project uses the one at the root of node_modules.

The top-down idea still holds though, I think an approach like that would work.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jun 13, 2019
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Jun 13, 2019
@trusktr
Copy link
Author

trusktr commented Jun 18, 2019

I just experienced issues again today. As an example, I have a project-a which depends on project-b. Both project-a and project-b depend on the same version of react.

In project-b, I ran

npm install
npm link
npm run build # generates a dist/ folder

After doing this project-b contains react in its node_modules.

Then in project-a I ran

npm install # installs project-b (and its dependencies) from NPM

Now project-a contains both project-b and react in its node_modules. The folders look like this:

project-a
    node_modules
        react
        project-b

Now I want to work with a local version of project-b which I'm currently working on, so:

npm link project-b # link the local version of project-b into project-a

Now project-a contains react as well as a linked version of project-b. Remember that project-b also contains react. So now, the folder structure of project-a's node_modules looks like this:

project-a
    node_modules
        react
        project-b (symlinked)
            node_modules
                react

Now what will happen is that TypeScript will give errors like the following:

[1] ../project-b/node_modules/@types/react/index.d.ts:2997:13 - error TS2717: Subsequent property declarations must have the same type.  Property 'rect' must be of type 'SVGProps<SVGRectElement>', but here has type 'SVGProps<SVGRectElement>'.
[1] 
[1] 2997             rect: React.SVGProps<SVGRectElement>;
[1]                  ~~~~
[1] 
[1]   node_modules/@types/react/index.d.ts:2998:13
[1]     2998             rect: React.SVGProps<SVGRectElement>;
[1]                      ~~~~
[1]     'rect' was also declared here.
[1] 
[1] ../project-b/node_modules/@types/react/index.d.ts:2998:13 - error TS2717: Subsequent property declarations must have the same type.  Property 'stop' must be of type 'SVGProps<SVGStopElement>', but here has type 'SVGProps<SVGStopElement>'.
[1] 
[1] 2998             stop: React.SVGProps<SVGStopElement>;
[1]                  ~~~~
[1] 
[1]   node_modules/@types/react/index.d.ts:2999:13
[1]     2999             stop: React.SVGProps<SVGStopElement>;
[1]                      ~~~~
[1]     'stop' was also declared here.
[1] 
[1] ../project-b/node_modules/@types/react/index.d.ts:2999:13 - error TS2717: Subsequent property declarations must have the same type.  Property 'switch' must be of type 'SVGProps<SVGSwitchElement>', but here has type 'SVGProps<SVGSwitchElement>'.
[1] 
[1] 2999             switch: React.SVGProps<SVGSwitchElement>;
[1]                  ~~~~~~
[1] 
[1]   node_modules/@types/react/index.d.ts:3000:13
[1]     3000             switch: React.SVGProps<SVGSwitchElement>;
[1]                      ~~~~~~
[1]     'switch' was also declared here.
[1] 
[1] ../project-b/node_modules/@types/react/index.d.ts:3000:13 - error TS2717: Subsequent property declarations must have the same type.  Property 'symbol' must be of type 'SVGProps<SVGSymbolElement>', but here has type 'SVGProps<SVGSymbolElement>'.
[1] 
[1] 3000             symbol: React.SVGProps<SVGSymbolElement>;
[1]                  ~~~~~~
[1] 
[1]   node_modules/@types/react/index.d.ts:3001:13
[1]     3001             symbol: React.SVGProps<SVGSymbolElement>;
[1]                      ~~~~~~
[1]     'symbol' was also declared here.
[1] 
[1] ../project-b/node_modules/@types/react/index.d.ts:3001:13 - error TS2717: Subsequent property declarations must have the same type.  Property 'text' must be of type 'SVGProps<SVGTextElement>', but here has type 'SVGProps<SVGTextElement>'.
[1] 
[1] 3001             text: React.SVGProps<SVGTextElement>;
[1]                  ~~~~
[1] 
[1]   node_modules/@types/react/index.d.ts:3002:13
[1]     3002             text: React.SVGProps<SVGTextElement>;
[1]                      ~~~~
[1]     'text' was also declared here.
[1] 
[1] ../project-b/node_modules/@types/react/index.d.ts:3002:13 - error TS2717: Subsequent property declarations must have the same type.  Property 'textPath' must be of type 'SVGProps<SVGTextPathElement>', but here has type 'SVGProps<SVGTextPathElement>'.
[1] 
[1] 3002             textPath: React.SVGProps<SVGTextPathElement>;
[1]                  ~~~~~~~~
[1] 
[1]   node_modules/@types/react/index.d.ts:3003:13
[1]     3003             textPath: React.SVGProps<SVGTextPathElement>;
[1]                      ~~~~~~~~
[1]     'textPath' was also declared here.
[1] 
[1] ../project-b/node_modules/@types/react/index.d.ts:3003:13 - error TS2717: Subsequent property declarations must have the same type.  Property 'tspan' must be of type 'SVGProps<SVGTSpanElement>', but here has type 'SVGProps<SVGTSpanElement>'.
[1] 
[1] 3003             tspan: React.SVGProps<SVGTSpanElement>;
[1]                  ~~~~~
[1] 
[1]   node_modules/@types/react/index.d.ts:3004:13
[1]     3004             tspan: React.SVGProps<SVGTSpanElement>;
[1]                      ~~~~~
[1]     'tspan' was also declared here.
[1] 
[1] ../project-b/node_modules/@types/react/index.d.ts:3004:13 - error TS2717: Subsequent property declarations must have the same type.  Property 'use' must be of type 'SVGProps<SVGUseElement>', but here has type 'SVGProps<SVGUseElement>'.
[1] 
[1] 3004             use: React.SVGProps<SVGUseElement>;
[1]                  ~~~
[1] 
[1]   node_modules/@types/react/index.d.ts:3005:13
[1]     3005             use: React.SVGProps<SVGUseElement>;
[1]                      ~~~
[1]     'use' was also declared here.
[1] 
[1] ../project-b/node_modules/@types/react/index.d.ts:3005:13 - error TS2717: Subsequent property declarations must have the same type.  Property 'view' must be of type 'SVGProps<SVGViewElement>', but here has type 'SVGProps<SVGViewElement>'.
[1] 
[1] 3005             view: React.SVGProps<SVGViewElement>;
[1]                  ~~~~
[1] 
[1]   node_modules/@types/react/index.d.ts:3006:13
[1]     3006             view: React.SVGProps<SVGViewElement>;
[1]                      ~~~~
[1]     'view' was also declared here.
[1] 

So it appears that when TypeScript is looking up type definitions, it picks up the ones in project-a's version of react and in project-b's version of react, where both react's even have the same version number.

The problem can be exacerbated when linking more than one package, and even more so when for example linking a project-c to project-b, linking project-b to project-a, and linking project-c to project-a.

The npm link workflow is a common workflow in many projects. However, I haven't used too much in the context of TypeScript projects until the last year where all my projects have been in TypeScript, not plain JavaScript.

solution thoughts:

I believe TypeScript should default to picking the types from a package which is the top-most package in node_modules.

So, from perspective of project-a, when TypeScript is compiling project-a code, it'd be nice if it chose the top-most react, and ignore the one in project-b's node_modules.

@trusktr
Copy link
Author

trusktr commented Jun 20, 2019

I'm struggling with this. This makes npm link workflows very difficult and time consuming.

Can I ask, have any of you used npm link workflows in TypeScript projects? Have you had any issues?

@trusktr
Copy link
Author

trusktr commented Jun 20, 2019

Is this a regression of #16274 ?

@sheetalkamat
Copy link
Member

Are you using typescript@3.5? I think #31541 should have fixed this. If not can you create sample repro so we can investigate this.

@trusktr
Copy link
Author

trusktr commented Jun 25, 2019

Alright, I'll report back when I give it a go.

@sheetalkamat sheetalkamat added Needs More Info The issue still hasn't been fully clarified and removed Needs Investigation This issue needs a team member to investigate its status. labels Aug 14, 2019
@sheetalkamat sheetalkamat assigned trusktr and unassigned sheetalkamat Aug 14, 2019
@shcarrico
Copy link

@trusktr I have faced what I think is this same issue.. I was able to work around it by providing a paths configuration for the module in question, and setting preserveSymlinks to true in tsconfig. Would be curious if this worked for you. Similar to the solution for webpack via the "resolve.alias" configuration property (comes up when one has multiple versions of react introduced via npm-link).

...
"preserveSymlinks": true,
    "baseUrl": ".",
    "paths": {
      "office-ui-fabric-react": ["node_modules/office-ui-fabric-react"]
    }

I am using office-ui-fabric react, and have an imported library with customized components.

@trusktr
Copy link
Author

trusktr commented Dec 11, 2019

@sheetalkamat I've been on TS 3.7.2 lately, and still encounter issues regarding duplicates. Unfortunately, it hasn't happened on a public repo that I can share yet.

@sheetalkamat
Copy link
Member

@trusktr if you need to, you can also share repro code privately at shkamat at Microsoft dot com and after we sign nda. Let me know if that would work to get repro. Unfortunately without repro we cannot investigate this further.

@trusktr
Copy link
Author

trusktr commented Dec 11, 2019

Ok, thanks, let me chat with someone higher up to see if I can get an NDA approved.

In the meantime, I'm getting an error like this, for example:

node_modules/@scope/some-package/src/components/Foo.dom.tsx:325:5 - error TS2717: Subsequent property declarations must have the same type.  Property ''x-foo'' must be of type 'Foo', but here has type 'Foo'.

325     'x-foo': Foo
        ~~~~~~~~

  ../scope/some-package/src/components/Foo.dom.tsx:325:5
    325     'x-foo': Foo
            ~~~~~~~~

From that output, it seems that the type system is referring to the same type in two ways: one way is with node-style module resolution, and the other way is with a relative path.

When I run npm ls @scope/some-package to see what my installation looks like, I only see one instance of the package:

❯ npm ls @scope/some-package
my-project@1.10.10 /Users/trusktr/src/my-project
└── @scope/some-package@4.3.9  -> /Users/trusktr/src/scope/some-package invalid

and it is labeled as "invalid" (not sure why, but maybe that has to do with it). I obfuscated the names for this example.

After checking all of the import statements in the project, they're all of the form import {...} from "@scope/some-package/.../etc/...".

@trusktr
Copy link
Author

trusktr commented Dec 12, 2019

Just to re-confirm, if I install the exact same package from git instead of using npm link (so I made a new branch, and pushed my changes, instead of relying on npm link to have my latest changes), then the issue goes away.

The only difference between the linked module and the git module, is that due to package.json have a file list of

  "files": [
    "dist",
    "src",
    "package.json"
  ],

the git-installed version does not have any of the extra files (f.e. the npm linked version has things like tsconfig.json, and other files normally used in dev mode). Not sure if this has anything to do with it.

I'll do a test...

@trusktr
Copy link
Author

trusktr commented Dec 12, 2019

Alright, after testing, if I manually create a symlink to the package, in node_modules, and then delete all files in it except the ones that are listed in files of the package's package.json (so it has the same content as if I had installed it manually), then I still have the same issue.

The file structure in node_modules/scope/the-package-in-question is identical either way (whether it's a symlink to the package, vs installed normally).

@trusktr trusktr changed the title Support for npm-linked duplicate modules. npm-linked modules (duplicates or not) are not working. Dec 13, 2019
@trickreich
Copy link

you've found a solution or an workaround?

@NicBright
Copy link

NicBright commented Jun 18, 2020

I'm also having problems with "npm link" and TS, however for me, TS seems to not be able to work with the symlinked module at all ...

Anyways, concerning this issue: I don't think it's TS responsibility to manage "duplicates". The thing is, that the concept of "npm link" here already is flawed. It's just not the same thing to have a package being regularly installed with "npm install" and having dependencies flattened correctly, compared with arbitrarily changing the structure of node_modules when doing "npm link". For me, it is perfectly clear that TS just follows the usual node algorithm of including the nearest match (inside node_modules) ... and duplicate types in this case are a problem, because it has to be assumed that the duplicated dependency ends as a duplicate at runtime ... well and then it's really two different types ... e.g. try using instanceof operator on seemingly identical objects, whose constructor prototypes in fact are different objects.

To summarize: npm link should be changed in a way to produce more realistic results matching what would be installed if the package had been regularly installed by means of npm install. Just my two cents ... Update: I just found this link, however I fear it will not help regarding the "duplicate dependencies" issue: https://github.com/npm/rfcs/blob/latest/accepted/0011-npm-link-changes.md

I almost forgot: the workaround I'm using for the whole npm link mess: I don't use npm link any more. Instead, I'm using a tiny script that I run manually to copy the contents of my package from project-b to project-a. Of course, for this to work correctly, I need to run npm install initially in project-a in order to have those dependencies of project-b correctly installed inside its dedicated node_modules folder, that can't be flattened because project-a somehow uses those dependencies with different versions, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs More Info The issue still hasn't been fully clarified
Projects
None yet
Development

No branches or pull requests

6 participants