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

phantom dependencies in nx and @nrwl packages #12021

Closed
troywweber7 opened this issue Sep 15, 2022 · 10 comments
Closed

phantom dependencies in nx and @nrwl packages #12021

troywweber7 opened this issue Sep 15, 2022 · 10 comments

Comments

@troywweber7
Copy link

troywweber7 commented Sep 15, 2022

Current Behavior

When installing @nrwl/storybook or @nrwl/workspace using pnpm, there appear to be a couple phantom dependencies.

Expected Behavior

If either of these packages publish JS which directly depends on phantom dependencies, those phantom dependencies should be added to the appropriate package.json's dependencies (or peerDependencies, if appropriate) so that Nrwl Nx can better support pnpm as it claims to do. The phantom packages I currently identified are shown below:

  • @nrwl/storybook/src/generators/configuration/configuration.js directly depends on the following
    • tslib
  • @nrwl/workspace/src/utils/ast-utils.js directly depends on the following
    • @angular-devkit/schematics
    • @angular-devkit/core
  • nx/src/adapter/ngcli-adapter.js directly depends on the following
    • @angular-devkit/core
    • @angular-devkit/schematics
    • @angular-devkit/architect

Steps to Reproduce

Here is the original issue that caused me to stumble upon these phantom dependencies. This original issue contains reproduction steps as well as additional information for how to workaround the issue using a pnpmfile.cjs file. In that file, I ultimately identified the above offenders.

Failure Logs

Environment

 >  NX   Report complete - copy this into the issue template

   Node : 16.16.0
   OS   : linux x64
   pnpm : 7.11.0
   
   nx : 14.7.5
   @nrwl/angular : 14.7.5
   @nrwl/cypress : 14.7.5
   @nrwl/detox : Not Found
   @nrwl/devkit : Not Found
   @nrwl/eslint-plugin-nx : 14.7.5
   @nrwl/express : Not Found
   @nrwl/jest : 14.7.5
   @nrwl/js : Not Found
   @nrwl/linter : 14.7.5
   @nrwl/nest : Not Found
   @nrwl/next : Not Found
   @nrwl/node : Not Found
   @nrwl/nx-cloud : Not Found
   @nrwl/nx-plugin : Not Found
   @nrwl/react : Not Found
   @nrwl/react-native : Not Found
   @nrwl/schematics : Not Found
   @nrwl/storybook : Not Found
   @nrwl/web : Not Found
   @nrwl/workspace : 14.7.5
   typescript : 4.8.3
   ---------------------------------------
   Local workspace plugins:
   ---------------------------------------
   Community plugins:
   	 @ionic/angular: 6.2.7
   	 @nxext/ionic-angular: 14.0.0
@troywweber7 troywweber7 changed the title phantom dependencies in @nrwl/storybook and @nrwl/workspace phantom dependencies in @nrwl/storybook, @nrwl/workspace, and nx Sep 15, 2022
@troywweber7
Copy link
Author

troywweber7 commented Sep 15, 2022

Another one to add (which only came up because I recently did an nx migrate latest which got me to 14.7.5). All the above still seem to be necessary as well. But this one is necessary for a migration script that is published.

  • @nrwl/cypress/src/migrations/update-14-7-0/update-cypress-version-if-10.js directly depends on the following
    • semver

And then once my nx was migrated, another one popped up:

  • nx/src/utils/package-json.js directly depends on the following (it imports the package.json file)
    • @nrwl/linter

@troywweber7 troywweber7 changed the title phantom dependencies in @nrwl/storybook, @nrwl/workspace, and nx phantom dependencies in nx and @nrwl/* packages Sep 15, 2022
@troywweber7 troywweber7 changed the title phantom dependencies in nx and @nrwl/* packages phantom dependencies in nx and @nrwl packages Sep 15, 2022
@troywweber7
Copy link
Author

In addition, another project I'm working on (also on the latest nx) identified the following phantom dependencies:

  • @nrwl/angular
    • src/executors/package/ng-packagr-adjustments/styles/stylesheet-processor.js directly depends on
      • browserslist
    • src/executors/package/ng-packagr-adjustments/ng-package/options.di.js directly depends on
      • find-cache-dir
    • src/executors/package/ng-packagr-adjustments/ng-package/entry-point/entry-point.di.js directly depends on
      • injection-js
    • src/executors/package/ng-packagr-adjustments/ng-package/entry-point/compile-ngc.transform.js directly depends on
      • ora
  • @nrwl/nx-cloud
    • lib/utilities/distributed-task-execution-detection.js directly depends on
      • fs-extra

@FrozenPandaz
Copy link
Collaborator

It will be more productive to address these one at a time.

Could you please open separate issues for each and describe the situation where it is an issue please?

@troywweber7
Copy link
Author

Found another: @nrwl/angular/src/executors/ng-packagr-lite/ng-packagr-adjustments/styles/stylesheet-processor.js directly depends on sass.

@troywweber7
Copy link
Author

@FrozenPandaz I did a migrate recently and I can see that some of these are fixed, but not all of them are. This should not be closed until fixed.

@troywweber7
Copy link
Author

@FrozenPandaz Also, I do not feel I should have to "describe the situation where it is an issue". If you go to the indicated source file relating to the identified .js file, you should see it doing an import or require for the packages. If these are not listed in the package.json, they should be.

@troywweber7
Copy link
Author

@FrozenPandaz for example, review this file: https://github.com/nrwl/nx/blob/master/packages/angular/src/executors/ng-packagr-lite/ng-packagr-adjustments/styles/stylesheet-processor.ts#L252.

Note that in https://github.com/nrwl/nx/blob/master/packages/angular/package.json, "sass" is not listed as a dependency. This is a phantom dependency and should, at the very least, be listed in peer dependencies.

@troywweber7
Copy link
Author

troywweber7 commented Nov 22, 2022

@FrozenPandaz This should not be closed.

@troywweber7
Copy link
Author

@FrozenPandaz it should also not be marked with "more info needed". All the info required is in the original post. When I point out that a file directly depends on a library that is not listed in the package.json, then that is a phantom dependency and therefore an issue.

@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 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants