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

dependency-checks lint rule does not detect peer dependencies #29958

Closed
3 of 4 tasks
statop opened this issue Feb 10, 2025 · 2 comments
Closed
3 of 4 tasks

dependency-checks lint rule does not detect peer dependencies #29958

statop opened this issue Feb 10, 2025 · 2 comments
Assignees

Comments

@statop
Copy link

statop commented Feb 10, 2025

Current Behavior

Dependency-checks does not allow dependencies that are peers of transitive dependencies.

For example -

"dependencies": {
"ng-packagr": "18.2.1",
"typescript": "5.5.4"
}

I use ng-packagr in my project, but not typescript
ng-packagr has a peer dep on typescript
so I need to have it
but it's not directly used, so dependency-checks fails

@meeroslav

Expected Behavior

Dependency-checks properly handles peer dependencies

GitHub Repo

No response

Steps to Reproduce

see above

Nx Report

NX   Report complete - copy this into the issue template

Node           : 20.12.2
OS             : darwin-arm64
Native Target  : aarch64-macos
pnpm           : 9.15.4

nx                 : 19.8.14
@nx/js             : 19.8.14
@nx/jest           : 19.8.14
@nx/eslint         : 19.8.14
@nx/workspace      : 19.8.14
@nx/angular        : 19.8.14
@nx/devkit         : 19.8.14
@nx/eslint-plugin  : 19.8.14
@nx/nest           : 19.8.14
@nx/node           : 19.8.14
@nx/plugin         : 19.8.14
@nx/react          : 19.8.14
@nx/rollup         : 19.8.14
@nx/storybook      : 19.8.14
@nx/vite           : 19.8.14
@nx/web            : 19.8.14
@nx/webpack        : 19.8.14
typescript         : 5.5.4
---------------------------------------
Registered Plugins:
./tools/plugin/plugin.js
---------------------------------------
Community plugins:
@ngrx/effects         : 18.1.1
@ngrx/operators       : 18.1.1
@ngrx/store           : 18.1.1
@ngrx/store-devtools  : 18.1.1
@openadp/tools-plugin : 25.7.6
@storybook/angular    : 8.2.9
apollo-angular        : 7.0.2
ng-mocks              : 14.13.0
---------------------------------------
Local workspace plugins:
	 @openadp/tools-plugin

Failure Logs

Package Manager Version

No response

Operating System

  • macOS
  • Linux
  • Windows
  • Other (Please specify)

Additional Information

No response

@meeroslav
Copy link
Contributor

Hi @statop,

After looking more closely into this issue the problem is deeper than the dependency-checks rule. When we parse the lock file to create a tree of dependencies, we mark typescript as being a static dependency of ng-packagr. That means that for Nx, it's ng-packagr's responsibility to ensure typescript is installed.

Let's say we have a situation where A imports, B imports C, and C has a peer dependency on typescript.
Neither A nor B directly use typescript and should not care about it; however, A being the last (or first) in the chain, would have to have direct dependency on typescript.

Having B being peer dependent on typescript would not be correct, and would lead to potential sync issues if we at some point decide to remove dependency on C but forget to cleanup the peerDependency on typescript.

If you still want to keep typescript as a peer dependency of B,, then the correct way to handle this would be to add typescript to ignoredDependencies in the dependency-checks rule of that package.

@meeroslav meeroslav closed this as not planned Won't fix, can't repro, duplicate, stale Feb 12, 2025
@statop
Copy link
Author

statop commented Feb 12, 2025

How would you do development work on A or B though? You need to declare the peer dep somewhere otherwise npm will complain.

And also I might want to reduce the number of valid versions.

I think this is still an issue, the lint rule should be able detect that it shouldn’t complain about this dependency. It should see the peer dep in the tree and understand it.

I shouldn’t need to ignore it manually. In a huge nx repo it is painful to have to have special rules for each project.

I understand this is probably hard because the peer deps are not encoded in the nx dep graph, but it is an issue none-the-less.

@meeroslav

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants