Skip to content

TypeScript should not use dependencies from an unrelated node_modules directory in a sibling directory #44362

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

Closed
bluenote10 opened this issue Jun 1, 2021 · 2 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@bluenote10
Copy link

Bug Report

🔎 Search Terms

wrong dependency unrelated node_modules sibling directory

Related: #30124 (Declarations are unexpectedly used from node_modules in parent directory) but this issue is about unexpectedly using node_modules from a sibling directory.

🕗 Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ, finding nothing related.

⏯ Playground Link

Not reproducible on Playground because it requires a sibling folder containing a node_modules.

A fully self-contained minimal reproduction example is here on GitHub.

💻 Code

Originally I have posted this issue as a question on StackOverflow, but after several days of experimentation it feels more like a bug. Refer to the question for some more details if needed.


Consider a repository structure that contains two TypeScript projects:

  • common: Some common that is used by the frontend as well.
  • backend: A code NodeJS server app.

common/src/common.ts

// an example dependency
import daysjs from "dayjs";

export function helperFunction() {
  console.log(daysjs());
}

The backend uses this common code like this:

backend/src/backend.ts

import { helperFunction } from "../../common/src/common";

console.log("Running backend");
helperFunction();

Further:

  • In common/package.json we have listed dayjs as a dependency, and we have done an npm install in common.
  • In backend/package.json we forgot to list dayjs as a dependency!

🙁 Actual behavior

The backend compilation silently succeeds, ignoring the fact the dependency is missing. Apparently TypeScript simply picks the dependency from the sibling folder common/node_modules (deleting it makes the compilation fail).

This is not in line with nodejs' behavior of resolving node_modules in parent directories only. Therefore, running the compilation result with nodejs crashes.

We have brought down our production server because of the false positive compilation.

🙂 Expected behavior

Compilation should fail, complaining about the missing dependency instead of picking it up from a sibling node_modules folder.

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Jun 1, 2021
@RyanCavanaugh
Copy link
Member

TypeScript doesn't read your package.json files, so the dependency list there is not germane.

Anyway we're never really sure how your code is going to be laid out at runtime so if it looks like you have something that might work, we err on the side of allowing it rather than disallowing it. In this case you have a valid transitive dependency through common, and taking a dependency (possibly unintentionally, possibly not) on transitive dependencies is extremely common and people actually complain when this doesn't work.

@bluenote10
Copy link
Author

bluenote10 commented Jun 1, 2021

TypeScript doesn't read your package.json files, so the dependency list there is not germane.

Agreed, it doesn't know about package.json, but the docs suggest that module resolution is relative to the baseUrl defined by tsconfig.json, suggesting that it is somehow possible to influence where TS is looking for dependencies relative to tsconfig.json.

Anyway we're never really sure how your code is going to be laid out at runtime so if it looks like you have something that might work, we err on the side of allowing it rather than disallowing it.

Erring on the permissive side can have the consequence of bringing down a productive deployment due to the false positive compilation. Wouldn't it be better to err on the explicit side: By default TS doesn't pick dependencies from "arbitrary" node_modules, and if the user wants that behavior they add an explicit search path to tsconfig.json?

Even if it is not the default, it would at least be nice to have an opt-in mechanism for protecting picking dependencies from a wrong place.

... and taking a dependency (possibly unintentionally, possibly not) on transitive dependencies is extremely common and people actually complain when this doesn't work.

I'd be curious to see such common project structures. I'd understand that people commonly assume dependencies being picked up in parent folders because that's in line with runtime resolution of nodejs, but I can't imagine that it is common to assume that dependencies are picked up from a node_modules directory that is a sibling directory to the tsconfig.json, right? Wouldn't all these use cases be cleanly solvable as well with an explicit list of node_modules paths in tsconfig.json?

In fact my very first idea to solve the problem was to change the paths in tsconfig.json

  • from the unspecific { "*": ["node_modules/*"] }
  • to an explicit path like { "*": ["./node_modules/*"] },

which I would read as "only look in <baseUrl>/node_modules/*" and nowhere else. If I'd have an exotic use case of looking up node_modules in multiple places I could specify ["./node_modules/*", "../../some_other_place/node_modules/*"] etc. But that doesn't work as one would expect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

2 participants