nrwl / nx

Smart Monorepos · Fast CI
https://nx.dev
MIT License
23.21k stars 2.31k forks source link

Non-existent configuration no longer throwing error #17757

Open flippidippi opened 1 year ago

flippidippi commented 1 year ago

Current Behavior

When running a project without an invalid configuration, no error is thrown and it reverts to the default configuration and continues.

This is an issue and especially when doing deployments. We recently moved an app to production but forgot to add the configuration in project.json and ended up deploying with the wrong env vars.

Expected Behavior

When running an nx run command and specifying configuration, if the configuration doesn't exist, the process should throw.

GitHub Repo

https://github.com/nrwl/nx-examples

Steps to Reproduce

  1. npm i
  2. yarn
  3. npx nx run products:build:asd
  4. Notice that it runs nx run products:build and doesn't error even thought asd is not a valid configuration

Nx Report

>  NX   Report complete - copy this into the issue template

   Node   : 18.16.0
   OS     : linux x64
   yarn   : 1.22.19
   Hasher : Native

   nx                 : 16.3.2
   @nx/js             : 16.4.0-beta.3
   @nx/jest           : 16.4.0-beta.3
   @nx/linter         : 16.4.0-beta.3
   @nx/workspace      : 16.4.0-beta.3
   @nx/angular        : 16.4.0-beta.3
   @nx/cypress        : 16.4.0-beta.3
   @nx/devkit         : 16.4.0-beta.3
   @nx/eslint-plugin  : 16.4.0-beta.3
   @nx/react          : 16.4.0-beta.3
   @nrwl/tao          : 16.3.2
   @nx/web            : 16.4.0-beta.3
   @nx/webpack        : 16.4.0-beta.3
   nx-cloud           : 16.0.5
   typescript         : 5.0.4
   ---------------------------------------
   Community plugins:
   @ngrx/component-store : 16.0.0
   @ngrx/effects         : 16.0.0
   @ngrx/entity          : 16.0.0
   @ngrx/router-store    : 16.0.0
   @ngrx/store           : 16.0.0
   @ngrx/store-devtools  : 16.0.0
   ---------------------------------------
   The following packages should match the installed version of nx
     - @nx/js@16.4.0-beta.3
     - @nrwl/js@16.4.0-beta.3
     - @nx/jest@16.4.0-beta.3
     - @nrwl/jest@16.4.0-beta.3
     - @nx/linter@16.4.0-beta.3
     - @nrwl/linter@16.4.0-beta.3
     - @nx/workspace@16.4.0-beta.3
     - @nrwl/workspace@16.4.0-beta.3
     - @nx/angular@16.4.0-beta.3
     - @nrwl/angular@16.4.0-beta.3
     - @nx/cypress@16.4.0-beta.3
     - @nrwl/cypress@16.4.0-beta.3
     - @nx/devkit@16.4.0-beta.3
     - @nrwl/devkit@16.4.0-beta.3
     - @nx/eslint-plugin@16.4.0-beta.3
     - @nrwl/eslint-plugin-nx@16.4.0-beta.3
     - @nx/react@16.4.0-beta.3
     - @nrwl/react@16.4.0-beta.3
     - @nx/web@16.4.0-beta.3
     - @nrwl/web@16.4.0-beta.3
     - @nx/webpack@16.4.0-beta.3
     - @nrwl/webpack@16.4.0-beta.3

   To fix this, run `nx migrate nx@16.4.0-beta.3`

Failure Logs

No response

Operating System

Additional Information

No response

xiongemi commented 10 months ago

@flippidippi I tried to push up a pr to throw an error, but I realized that the current logic will fall to the default configuration if the specified is not found. the reason is that a build sometimes depends on other libraries, whereas the specified configuration might be in its dependent libraries.

flippidippi commented 10 months ago

Then maybe we would need an option in project json for target for something like "don't allow unknown configurations" or something? You are right, we really only want this in our apps/deploys.

Cammisuli commented 10 months ago

hey @flippidippi, adding this change is going to require some design changes on how we build out the task graph. @xiongemi gave it a shot, but we need to have a way to make this opt in for certain targets, like you mentioned.

Your scenario is a valid concern, so we'll think of a way to get this going without breaking a lot of other people who rely on this mechanism.

makirby commented 8 months ago

Came here looking for the same feature. Was very suprised when my nx affected --target:build --configuration:ci ran every single build that was affected.

github-actions[bot] commented 2 months ago

This issue has been automatically marked as stale because it hasn't had any activity for 6 months. Many things may have changed within this time. The issue may have already been fixed or it may not be relevant anymore. If at this point, this is still an issue, please respond with updated information. It will be closed in 21 days if no further activity occurs. Thanks for being a part of the Nx community! 🙏

Ambroos commented 2 months ago

Not stale.