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/angular installs as a production dependency #6885

Closed
Ionaru opened this issue Aug 28, 2021 · 11 comments
Closed

@nrwl/angular installs as a production dependency #6885

Ionaru opened this issue Aug 28, 2021 · 11 comments
Labels

Comments

@Ionaru
Copy link

Ionaru commented Aug 28, 2021

Current Behavior

Using create-nx-workspace and selecting any template that includes Angular installs @nrwl/angular under dependencies instead of devDependencies.

image

In the documentation it is shown that @nrwl/angular should be installed as a devdependency.
image

This behaviour causes all kinds of (normally) devDependencies to be installed as regular dependencies. Like everything @nrwl/angular depends on, their dependencies, their dependencies and so on.
image

Expected Behavior

@nrwl/angular is installed under devDependencies.

Steps to Reproduce

create-nx-workspace --preset=angular --appName=myApp --style=css --nx-cloud=false -name=ddapp

Failure Logs

None.

Environment

>  NX  Report complete - copy this into the issue template

  Node : 16.5.0
  OS   : win32 x64
  npm  : 7.21.1

  nx : Not Found
  @nrwl/angular : 12.8.0
  @nrwl/cli : 12.8.0
  @nrwl/cypress : 12.8.0
  @nrwl/devkit : 12.8.0
  @nrwl/eslint-plugin-nx : 12.8.0
  @nrwl/express : Not Found
  @nrwl/jest : 12.8.0
  @nrwl/linter : 12.8.0
  @nrwl/nest : Not Found
  @nrwl/next : Not Found
  @nrwl/node : Not Found
  @nrwl/nx-cloud : Not Found
  @nrwl/react : Not Found
  @nrwl/schematics : Not Found
  @nrwl/tao : 12.8.0
  @nrwl/web : Not Found
  @nrwl/workspace : 12.8.0
  @nrwl/storybook : 12.8.0
  @nrwl/gatsby : Not Found
  typescript : 4.3.5

Additional information

This issue was noticed earlier by @eternalmatt and reported in #3618.

@leosvelperez
Copy link
Member

Thanks for reporting this!

This is intended. The @nrwl/angular package contains utilities that can be used at runtime (NgRx utilities for data persistence). That's why by default, our generators add the package as a dependency.

If you're using any of those utilities, you need it as a dependency, otherwise, it's safe to move it as a devDependency.

I'll keep this open to be clearer in the docs about this fact.

@leosvelperez leosvelperez added the scope: angular Issues related to Angular support in Nx label Sep 3, 2021
@Ionaru
Copy link
Author

Ionaru commented Sep 3, 2021

The NgRx utilities are no doubt useful and I appreciate that they are provided by Nx, but perhaps it's better to split those utilities off from @nrwl/angular into their own package, to not require people to install packages that have no place in a production environment.

For example @nrwl/angular depends on @nrwl/cypress which depends on @cypress/webpack-preprocessor which installs the entirety of webpack into the production environment.

@Coly010
Copy link
Contributor

Coly010 commented Sep 29, 2021

I may be wrong, but an npm install will install both dependencies and devDependencies correct?
So if this package was in either location, it would still be installed.

If you do not make any references to @nrwl/angular within your code, this should be tree-shaken out and not included in the final production bundle.

@eternalmatt
Copy link

eternalmatt commented Sep 29, 2021

This is the behavior of npm install on a local machine, however in CI/CD environment in a large enterprise, you'd typically run npm install once and run the necessary build scripts to create an artifact, then run npm install --prod and store the artifact + node_modules in some artifact repository, copy into a container, and deploy to test and prod environments.

As @nrwl/angular is listed in prod dependencies, its own dependencies such as @nrwl/cypress and @nrwl/jest are being copied into my production artifact and increasing the file size.

Yes the tree-shaking occurs and my web browser user is unaffected, but my artifacts and containers are bloated.

To be clear, it makes sense that @nrwl/angular is a production dependency, as I want to use its utilities. The problem is that there are production-code-worthy aspects in this library (NgRx) as well as generators and build-time tooling (@nrwl/angular:webpack-browser)

I would suggest this team extracts the prod code from this into something like @nrwl/ngrx so that users can use this in dependencies and the rest of @nrwl/angular can be recommended as devDependencies

@Coly010
Copy link
Contributor

Coly010 commented Oct 4, 2021

@eternalmatt Out of curiosity, why do you need the node_modules copied into a container? The built artefact should be the static files you need to host your Angular application right?

We will take the suggestion of creating a new package on board certainly.

@Ionaru
Copy link
Author

Ionaru commented Oct 4, 2021

The way I noticed it was through a npm audit --production test. It failed on a webpack dependency that I did not expect to exist in the production environment.

@eternalmatt
Copy link

@Coly010 it's a good question. Since we are using an Nx monorepo, we have two projects together (Nestjs "api" and Angular "frontend"). The api project contains nest-express API endpoints which serve the website files (index, js, css), so there is a build-time dependency between the two.

Consider we have a project directory that look as follows:

dist/
  apps/
    api/
      main.js
    frontend/
      browser/
        index.html, main.js, favicon.ico (etc)
node_modules/
angular.json
nx.json
package.json

Since these are sharing a package.json, when I go to build/deploy the NestJS api, it is including all the dependencies of the frontend project. The "api" project needs node_modules at runtime. main.js is a @nrwl/node:build output of api. Of course there may be different ways to accomplish the same, but this is briefly how my CI/CD works.

@Coly010
Copy link
Contributor

Coly010 commented Oct 6, 2021

That makes more sense. I thought it was simply an Angular site being deployed. We'll put splitting the NgRx production code onto our internal roadmap.

Thanks for highlighting the issue! :)

@github-actions
Copy link

This issue has been automatically marked as stale because it hasn't had any recent activity. It will be closed in 14 days if no further activity occurs.
If we missed this issue please reply to keep it active.
Thanks for being a part of the Nx community! 🙏

@Waterstraal
Copy link

Can this issue be reopened? Like @Ionaru said running commands like npm audit or npx @cyclonedx/bom now incorrectly includes many devDependencies.

Like others said, if it is needed for ngrx utilities, these should be moved to their own package imho.

@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
Projects
None yet
Development

No branches or pull requests

5 participants