Add a flag to ignore local peer dependency declarations so that circular graph doesn't hang-up entire process #407
Comments
thebanjomatic
pushed a commit
to thebanjomatic/turborepo
that referenced
this issue
Mar 1, 2022
I tried this first when attempting to fix my own build issues after discovering vercel#407. It wasn't enough to solve the build hangs with my project, but seems to have its own merrits. If a package has a peer dependency but there is no corresponding devDependency, then it should be presumed that there are no build-time dependency on that package. For example, if there was code that would be executed by one of the scripts that tried to import from the peer package, this would generally speaking fail to run, because node would be unable to resolve the dependency unless another package was pulling it in and it got hoisted, etc. Workspace dependencies are kind of a special case in that they won't be hoisted and can be resolved by node, but if you were to ever move that package out of the monorepo then that same situation as above would fail to build. As a result, my impression is that for any use-case where peerDependencies need to be explicitly part of the dependency graph is actually just a missing dependency declaration bug where that package should actually be part of the dev deps. I have also changed the order in which dependencies, optionalDependencies, and devDependencies are selected to match the order that lerna uses for cases where the package shows up in multiple places. See: https://github.com/lerna/lerna/blob/a47fc294393a3e9507a8207a5a2f07648a524722/core/package-graph/index.js#L53-L58 The attached issues mentions making a flag to control this behavior, but given the reasoning above, I'm not sure if there is ever a situation where you would want to have a peer-dependency impact the dependency graph if its not also a regular or dev (or optional) dependency. Fixes: vercel#407
thebanjomatic
pushed a commit
to thebanjomatic/turborepo
that referenced
this issue
Mar 3, 2022
I tried this first when attempting to fix my own build issues after discovering vercel#407. It wasn't enough to solve the build hangs with my project, but seems to have its own merrits. If a package has a peer dependency but there is no corresponding devDependency, then it should be presumed that there are no build-time dependency on that package. For example, if there was code that would be executed by one of the scripts that tried to import from the peer package, this would generally speaking fail to run, because node would be unable to resolve the dependency unless another package was pulling it in and it got hoisted, etc. Workspace dependencies are kind of a special case in that they won't be hoisted and can be resolved by node, but if you were to ever move that package out of the monorepo then that same situation as above would fail to build. As a result, my impression is that for any use-case where peerDependencies need to be explicitly part of the dependency graph is actually just a missing dependency declaration bug where that package should actually be part of the dev deps. I have also changed the order in which dependencies, optionalDependencies, and devDependencies are selected to match the order that lerna uses for cases where the package shows up in multiple places. See: https://github.com/lerna/lerna/blob/a47fc294393a3e9507a8207a5a2f07648a524722/core/package-graph/index.js#L53-L58 The attached issues mentions making a flag to control this behavior, but given the reasoning above, I'm not sure if there is ever a situation where you would want to have a peer-dependency impact the dependency graph if its not also a regular or dev (or optional) dependency. Fixes: vercel#407
kodiakhq bot
pushed a commit
that referenced
this issue
Mar 8, 2022
Splitting this out of #802. I tried this first when attempting to fix my own build issues in that PR after discovering Issue #407 which suggested peerDependencies might be the problem. It wasn't enough to solve the build hangs with my project, but seems to have its own merits. If a package has a peer dependency but there is no corresponding devDependency, then it should be presumed that there are no build-time dependency on that package. For example, if there was code that would be executed by one of the scripts that tried to import from the peer package, this would generally speaking fail to run, because node would be unable to resolve the dependency unless another package was pulling it in and it got hoisted, etc. Workspace dependencies are kind of a special case in that they won't be hoisted and can be resolved by node, but if you were to ever move that package out of the monorepo then that same situation as above would fail to build. As a result, my impression is that for any use-case where peerDependencies need to be explicitly part of the dependency graph is actually just a missing dependency declaration bug where that package should actually be part of the dev deps. I have also changed the order in which dependencies, optionalDependencies, and devDependencies are selected to match the order that lerna uses for cases where the package shows up in multiple places. See: https://github.com/lerna/lerna/blob/a47fc294393a3e9507a8207a5a2f07648a524722/core/package-graph/index.js#L53-L58 The attached issues mentions making a flag to control this behavior, but given the reasoning above, I'm not sure if there is ever a situation where you would want to have a peer-dependency impact the dependency graph if its not also a regular or dev (or optional) dependency. Fixes: #407
sokra
pushed a commit
that referenced
this issue
Oct 25, 2022
Had added this to run nextest in #341, but because it doesn't work I had reverted part of the change, just not this
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Originally posted by @sharvilak11 in #287
Add a flag to ignore peerDependencies while checking for cyclic graph
The text was updated successfully, but these errors were encountered: