turbo icon indicating copy to clipboard operation
turbo copied to clipboard

peerDependencies are ignored while creating dependency graph

Open mkermani144 opened this issue 2 years ago • 3 comments

What version of Turborepo are you using?

1.4.0

What package manager are you using / does the bug impact?

Yarn v1

What operating system are you using?

Mac

Describe the Bug

The peer dependencies are not considered part of the dependency graph. This issue has roots in #813 where they are ignored while creating the graph because of some dependency cycle issues mentioned in the PR.

Expected Behavior

Construct dependency graph including peer dependencies. Peer dependencies are one of the complementary tools which are used in monorepos, and simply ignoring them doesn't seem a good solution.

To Reproduce

  1. Suppose the following monorepo structure:
/app
  /some-app
/packages
  /dependent-a
  /dependent-b
  /c
  1. Suppose that dependent-a and dependent-b has c as peer dependency, and some-app has all packages (dependent-a, dependent-b and c) as dependency.

  2. Suppose turbo.json looks like this:

{
  "$schema": "https://turborepo.org/schema.json",
  "pipeline": {
    "build": {
      "dependsOn": ["^build"]
    }
  }
}
  1. When you run turbo run build on some-app (using --filter=some-app), it triggers build for all of the packages concurrently, completely unable to build c first and clearly causing issues.

I think this is a common scenario for plugin based apps, as the plugins may include other plugins as peer dependency (and not a regular dependency).

mkermani144 avatar Jul 30 '22 13:07 mkermani144

Would it make sense to always copy the peer dependencies to also be dev dependencies? It seems like if you have the dependency it should at least be part of your test suite?

dobesv avatar Aug 10 '22 02:08 dobesv

No. If peerDependencies were meant to be copied to devDependencies or dependencies, they were useless. They have their own use case. They are just peer dependencies.

mkermani144 avatar Aug 10 '22 03:08 mkermani144

peerDependencies that are mirrored in devDependencies are not useless. If your code directly references the dependencies you will have to ensure they are installed during development in order to test your code. You have to add them to devDependencies to do that, no?

If you are just passing through the peerDependencies of another package without using them in your own package you can skip adding them to devDependencies, that makes sense. But they would be in devDependencies for the other package, so they would still get build by turbo when that other package was built.

I suppose there might be some odd cases where you want to be able to import something by name but you don't have any automated tests that reference it, so you add it to peerDependencies but not devDependencies ? What are the details of your use case here?

dobesv avatar Aug 10 '22 17:08 dobesv

Still, no, in my humble opinion. 😄 I think you are missing the point of peer dependencies utterly.

If you want to test a package containing has some peer dependencies in isolation (without using it in the host package), you can do something like forcing your package manager to install peer dependencies for you somehow.

I haven't seen a real project which duplicates peer dependencies to dev dependencies. It may have some use cases in some special scenarios, but I don't know one.

mkermani144 avatar Aug 14 '22 04:08 mkermani144

React libraries with unit tests have to put react as a dev dependency for the tests, and a peer dependency for users of the library.

dobesv avatar Aug 14 '22 17:08 dobesv

I think @dobesv is correct - std practice is to make peer dependencies also dev dependencies.

sppatel avatar Aug 25 '22 12:08 sppatel

Confirmed, peerDeps mirrored in devDeps is common practice.

mattpocock avatar Aug 25 '22 12:08 mattpocock

I'm still not getting the point. I have seen many (standard) monorepos whose packages don't copy their peer dependencies to dev dependencies. I think it's not correct to ignore them just because the mirroring is a common practice.

mkermani144 avatar Aug 25 '22 15:08 mkermani144

IIUC peer dependencies are intended to be provided by the environment / platform where the code is running. As such, turbo doesn't consider them to be dependencies that need to be built. The typical example is plugins, they expect whatever environment they are in to already have the package that they plug into, rather than depend on it directly.

If you don't want to mirror peer dependencies in dev dependencies, you can make the dependency explicit in turbo.json by adding my-peer-dep#build to dependsOn for your package that has a peer dependency on my-peer-dep.

gsoltis avatar Aug 25 '22 19:08 gsoltis

I think it's not correct to ignore them

If they aren't marked as a dependency or dev dependency of that specific project, then they won't affect it's build or execution, so they are not a dependency of THAT package. The project that provides the dependency would be the one that should rebuild if the package changes.

dobesv avatar Aug 25 '22 23:08 dobesv

May I please ask what would happen if a 3rd party dependency (i.e:react), is a dev dependency under root/package.json of the monorepo but a peer dependency under /root/apps/example/package.json,

Would updating react to the latest version on the root, not trigger a new build since the child repo only uses react as a peer dependency rather than a dev dependency?

Under /root/apps/example/package.json, I would believe that semantically, if we don't want to keep versioning (hence, duplicating) dependencies, which are then managed by the root, it makes sense to add it in the format of

"peerDependencies": {
   "react": "*" // uses any version available: inherited by root (not-installed by default)
}

while the following would mean

"devDependencies": {
   "react": "*" // install latest version of react regardless what's declared on the root
}

-- update: Perhaps I missed the point where any changes to the main package.json (root) already updates the dependency graph, so the comment above would be invalidated by that factor?

zanona avatar Oct 12 '22 08:10 zanona

Just to weigh in in this old ticket and to urge to reopen this:

Duplicating dependencies in both devDependencies and peerDependencies will lead to inconsistencies and user errors. Currently it's possible to install only from peerDependencies with pnpm with it's auto install peers configuration option.

Sure, there are additional tooling like Syncpack to try to address versioning inconsistencies but fixing the root cause would be preferable.

jazmon avatar Nov 10 '23 11:11 jazmon

is this documented?

glitch-txs avatar Mar 28 '24 17:03 glitch-txs