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

JS package build incorrectly includes dependencies of dependencies #9371

Closed
ajwootto opened this issue Mar 16, 2022 · 17 comments · Fixed by #10600
Closed

JS package build incorrectly includes dependencies of dependencies #9371

ajwootto opened this issue Mar 16, 2022 · 17 comments · Fixed by #10600

Comments

@ajwootto
Copy link
Contributor

Current Behavior

A package built with @nrwl/js which uses the "tsc" build executor will not output the correct dependencies in its resulting package.json file if the package depends on another buildable package in the repo which itself has dependencies. Instead, the package.json will include all the dependencies of that sub package as well.

Expected Behavior

The resulting package.json should include only the dependencies it directly uses, not the dependencies of the other packages it depends on

Steps to Reproduce

  1. Generate buildable JS packages A and B
  2. Add some dependency to package B (say lodash)
  3. Add package B as a dependency of package A
  4. nx build package-a
  5. Package A will include lodash in its package.json even if its only directly used by package B

Environment

 >  NX   Report complete - copy this into the issue template

   Node : 16.13.2
   OS   : darwin x64
   npm  : 8.1.2

   nx : 13.8.8
   @nrwl/angular : undefined
   @nrwl/cli : 13.8.8
   @nrwl/cypress : 13.8.5
   @nrwl/detox : undefined
   @nrwl/devkit : 13.8.8
   @nrwl/eslint-plugin-nx : 13.8.8
   @nrwl/express : undefined
   @nrwl/jest : 13.8.8
   @nrwl/js : 13.8.8
   @nrwl/linter : 13.8.8
   @nrwl/nest : undefined
   @nrwl/next : 13.8.5
   @nrwl/node : 13.8.8
   @nrwl/nx-cloud : undefined
   @nrwl/react : 13.8.8
   @nrwl/react-native : undefined
   @nrwl/schematics : undefined
   @nrwl/storybook : 13.8.8
   @nrwl/tao : 13.8.8
   @nrwl/web : 13.8.8
   @nrwl/workspace : 13.8.8
   typescript : 4.3.5
   rxjs : 6.6.7
   ---------------------------------------
   Community plugins:
@AgentEnder
Copy link
Member

Hmmm, yes this was recently addressed for the rollup executor via #8640. It should be addressed here too.

@layershifter
Copy link

calculateProjectDependencies(
projectGraph,
context.root,
context.projectName,
context.targetName,
context.configurationName
);

The fix is a one liner:

 calculateProjectDependencies( 
   projectGraph, 
   context.root, 
   context.projectName, 
   context.targetName, 
   context.configurationName,
+  true
 ); 

@mckramer
Copy link
Contributor

mckramer commented Jun 6, 2022

Opened this recommended fix as PR #10600

@boeto
Copy link

boeto commented Oct 16, 2022

I get the same result(the package.json will include all the dependencies of that sub package as well) and the version currently in use is: "15.0.0"

@npwork
Copy link

npwork commented Oct 19, 2022

Same issue on "15.0.0"

@jgarciaokode
Copy link

Any news on this?

@jgarciaokode
Copy link

@FrozenPandaz @AgentEnder

@tinesoft
Copy link
Contributor

tinesoft commented May 6, 2023

This is still an issue , in latest Nx v16.1.1.... :-(
See tinesoft/nxrocks

The mentioned fix for rollup (#8640) , does no longer work...

Any workaround?

@dmitry-stepanenko
Copy link
Contributor

@tinesoft there's an updateBuildableProjectDepsInPackageJson option for the @nx/tsc executor, setting it to false prevents pinning of the versions of package's dependencies

@tinesoft
Copy link
Contributor

tinesoft commented May 6, 2023

@tinesoft there's an updateBuildableProjectDepsInPackageJson option for the @nx/tsc executor, setting it to false prevents pinning of the versions of package's dependencies

Hi @dmitry-stepanenko Unfortunately, doing so, will also prevent the rightful direct dependency (@nxrocks/common in my case)to be added...

@jgarciaokode
Copy link

This is still a problem in Nx version 16 ...

@ajwootto
Copy link
Contributor Author

any chance of this ever getting fixed? it seems like a pretty big flaw in what I understand to be a core Nx feature

@tinesoft
Copy link
Contributor

Hi,

to anyone interested, I'm managed to fix that issue by using @nx/dependency-checks ESLint rule to manage (almost) automatically the dependencies of your Nx library.

You can follow this great article by my fellow Nx champion 🏆 @LayZeeDK --> https://dev.to/this-is-learning/manage-nx-library-dependencies-with-the-nxdependency-checks-eslint-rule-2lem for more information.

For a real world project where this now recommended approach is implemented, you can have a look at my repo: https://github.com/tinesoft/nxrocks

Hope it helps! ^^

@tefkah
Copy link

tefkah commented Sep 22, 2023

@tinesoft wow, thank you so much for the recommendation!
While ideally i'd hoped that nx could have automatically fixed this, this is a good enough solution for me to finally retire my terrible custom tool which pruned the generated package.json afterwards.

Thanks!

@tinesoft
Copy link
Contributor

I'm glad I could help 😊.

FYI, @tefkah you kind of can automate this, by running the npx nx run-many --target lint --fix command (note the --fix part).

This command can be added as a npm script for example, and ran as a pre-commit hook, using a tool like Husky (git).

@ajwootto
Copy link
Contributor Author

Going to close this now since they just replaced the whole system with the linting rule (which has lots of its own problems I might add)

Copy link

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.