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

@nrwl/web|@nrwl/workspace: Package gets all dependencies of its children #8640

Closed
layershifter opened this issue Jan 21, 2022 · 4 comments · Fixed by #9297
Closed

@nrwl/web|@nrwl/workspace: Package gets all dependencies of its children #8640

layershifter opened this issue Jan 21, 2022 · 4 comments · Fixed by #9297
Assignees
Labels
outdated scope: react Issues related to React support for Nx type: bug

Comments

@layershifter
Copy link

layershifter commented Jan 21, 2022

Current Behavior

I have a workspace that has following structure:

               | --------- |
               | suite-pkg |
               | --------- |
               /           \
              /             \
             /               \
            /                 \
           /                   \
  | --------- |             | --------- |
  | core-pkg1 |             | core-pkg2 |
  | --------- |             | --------- |
  • core-pkg1 & core-pkg2 have NPM dependencies (i.e. third party)
  • suite-pkg only re-exports things from core-pkg1 & core-pkg2

After build suite-pkg contains dependencies of its children in package.json:

image

note: changing buildableProjectDepsInPackageJsonType to dependencies does not change the behavior

This happens because @nrwl/web and its Rollup executor use recursivelyCollectDependencies() that actually does recursive collection of dependencies:

function recursivelyCollectDependencies(
project: string,
projGraph: ProjectGraph,
acc: string[]
) {
(projGraph.dependencies[project] || []).forEach((dependency) => {
if (acc.indexOf(dependency.target) === -1) {
acc.push(dependency.target);
recursivelyCollectDependencies(dependency.target, projGraph, acc);
}
});
return acc;
}

Expected Behavior

  • core-pkg1 & core-pkg2 dependencies used in its sources
  • suite-pkg has only core-pkg1 & core-pkg2 in dependencies (again, dependencies used in its sources)

Proposal

I see following options:

  • do not collect dependencies recursively for generation of package.json files
  • make this behavior optional/configurable

Steps to Reproduce

Environment

  Node : 16.13.1
  OS   : darwin arm64
  npm  : 8.1.2
  
  nx : 13.4.6
  @nrwl/angular : undefined
  @nrwl/cli : 13.4.6
  @nrwl/cypress : 13.4.6
  @nrwl/devkit : 13.4.6
  @nrwl/eslint-plugin-nx : 13.4.6
  @nrwl/express : undefined
  @nrwl/jest : 13.4.6
  @nrwl/linter : 13.4.6
  @nrwl/nest : undefined
  @nrwl/next : undefined
  @nrwl/node : undefined
  @nrwl/nx-cloud : undefined
  @nrwl/react : 13.4.6
  @nrwl/react-native : undefined
  @nrwl/schematics : undefined
  @nrwl/tao : 13.4.6
  @nrwl/web : 13.4.6
  @nrwl/workspace : 13.4.6
  @nrwl/storybook : 13.4.6
  @nrwl/gatsby : undefined
  typescript : 4.4.4
  rxjs : 6.6.7
  ---------------------------------------
  Community plugins:
@jaysoo
Copy link
Member

jaysoo commented Jan 21, 2022

We'll address this. It never should have been recursive since npm dependencies are only one level deep.

@FrozenPandaz FrozenPandaz added the scope: react Issues related to React support for Nx label Jan 29, 2022
@egorderg
Copy link

I think it should ignore recursive when the dependency has the 'publishable' flag. Sometimes you need recursive dependencies when you are not publishing some of the libraries or want to collect everything in one library.

@jaysoo
Copy link
Member

jaysoo commented Mar 11, 2022

I think it should ignore recursive when the dependency has the 'publishable' flag. Sometimes you need recursive dependencies when you are not publishing some of the libraries or want to collect everything in one library.

But consumers of that parent library won't be able to use it without also adding the child libraries, or am I missing something here?

For example, I have @acme/parent-lib that consumes @acme/child-lib, I don't want npm deps from child-lib to be in the final package.json of parent-lib. Even if you aren't publishing parent-lib, there is no way to consume it without also adding child-lib`.

The only valid use-case for including deps of deps is if we are bundling child-lib into parent-lib. This bundling isn't currently supported so isn't a concern. If this behavior is needed we should track is as a separate enhancement.

@github-actions
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 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated scope: react Issues related to React support for Nx type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants