nrwl / nx

Smart Monorepos ยท Fast CI
https://nx.dev
MIT License
23.36k stars 2.33k forks source link

NX does not handle multiple configurations: nx run app:build --configuration=production,stage-env #2839

Closed leon closed 5 months ago

leon commented 4 years ago

Expected Behavior

When running commands using nx, they should behave the same as ng

This works ng run app:build --configuration=production,stage-env

this does not nx run app:build --configuration=production,stage-env

Current Behavior

It seems that nx is stripping my extra stage-env configurations from the --configuration

Failure Information (for bugs)

I think it has to do with that in earlier version of the angular cli, multiple configurations where not supported, they where added here in November 2019 https://github.com/angular/angular-cli/pull/15819

The problem nx has, is that it only supports one config. https://github.com/nrwl/nx/blob/115a1abd75e3898beb3f91d93f4edc85b4b9a90b/packages/tao/src/commands/run.ts#L46 https://github.com/nrwl/nx/blob/115a1abd75e3898beb3f91d93f4edc85b4b9a90b/packages/tao/src/commands/run.ts#L120-L138

Solution

Change the implementation so that we are not limited to one config.

leon commented 4 years ago

I've done some more testing and have found that it's --configuration that is causing the problem

if i do

-c production,test-env

it works but

---configuration production,test-env

does not work.

So it must be something to do with how we are passing along args.

FrozenPandaz commented 4 years ago

Interesting, thanks for reporting this! :+1:

rodzappa commented 4 years ago

For me, none of cases works; $ nx run app:build -c production,stage-env

returns:

> nx run financeiro:build --c=production,ninedev-production 
Schema validation failed with the following errors:
  Data path "" should NOT have additional properties(--).

and

$ nx run app:build --configuration=production,stage-env

strips the extra stage-env configuration from the --configuration

I'm using these modules versions:

>  NX  Report complete - copy this into the issue template

  @nrwl/angular : 9.5.0
  @nrwl/cli : 9.5.0
  @nrwl/cypress : 9.5.0
  @nrwl/eslint-plugin-nx : 9.5.0
  @nrwl/express : Not Found
  @nrwl/jest : 9.5.0
  @nrwl/linter : 9.5.0
  @nrwl/nest : Not Found
  @nrwl/next : Not Found
  @nrwl/node : Not Found
  @nrwl/react : Not Found
  @nrwl/schematics : Not Found
  @nrwl/tao : 9.5.0
  @nrwl/web : Not Found
  @nrwl/workspace : 9.5.0
  typescript : 3.8.3
Tiagogv commented 3 years ago

Adding double quotes for me did the trick. nx build app -c "production,stage-env" it translated to: ng run app:build --c=production,stage-env

andriychuma commented 3 years ago

None of the following works in my case:

nx build --configuration=production,staging
nx build -c=production,staging
nx build -c production,staging
nx build -c "production,staging"

Each one reports the following error:

>  NX   ERROR  Cannot find configuration 'production,staging' for project 'my-project'

However, this command works fine:

ng build -c=production,staging

Here is my environment:

  Node : 13.7.0
  OS   : win32 x64
  npm  : 6.14.8

  nx : Not Found
  @nrwl/angular : 11.0.8
  @nrwl/cli : 11.0.8
  @nrwl/cypress : 11.0.8
  @nrwl/devkit : 11.0.8
  @nrwl/eslint-plugin-nx : Not Found
  @nrwl/express : Not Found
  @nrwl/jest : 11.0.8
  @nrwl/linter : 11.0.8
  @nrwl/nest : Not Found
  @nrwl/next : Not Found
  @nrwl/node : Not Found
  @nrwl/react : Not Found
  @nrwl/schematics : Not Found
  @nrwl/tao : 11.0.8
  @nrwl/web : Not Found
  @nrwl/workspace : 11.0.8
  typescript : 4.0.5
Splaktar commented 3 years ago

I would consider this more as a bug than a feature request. When migrating from the Angular CLI and running node ./decorate-angular-cli.js, scripts that used to work for building your projects no longer work. This is certainly a problem when trying to move to Nx and take advantage of caching (the primary use case motivating this client).

fbaba-nibtravel commented 3 years ago

Hi, just FYI, I recently upgraded NX 10 where -c config1,config2 was working and now it's broken on NX 11. Agree with @Splaktar, this is a bug, not a feature request as it was working before.

Btw, this is the doco from the Angular CLI: https://angular.io/guide/workspace-config#alternate-build-configurations

shaharkazaz commented 3 years ago

@fbaba-nibtravel same here, just upgraded from NX 10 to NX 11 where -c=config1,config2 was working and now I'm getting:

NX ERROR Cannot find configuration 'config1,config2' for project 'platform'

@FrozenPandaz Any chance you can rethink this and mark it as a bug? it's blocking me from upgrading.

FrozenPandaz commented 3 years ago

Hi everyone,

Sorry, I think there was some confusion with how the Angular CLI handles this behavior.

It should be possible to support this behavior but as @vsavkin mentioned here, there are definitely some complexities here. I think we should be able to take this one step at a time though starting with the ability to execute nx build app1 --configuration config1,config2.

Thank you for being patient and calmly explaining the confusion.

FrozenPandaz commented 3 years ago

Hi all,

So I attempted to implement the behavior and thought about the problem some more. We also have plans to improve the way workspaces are configured which is being considered here as well.

While it was possible, I think it complicates the mental model because the defined configurations are not really intended to be used in isolation yet appear in the configuration.

If I understand correctly, at a high level, people want to keep workspace.json/angular.json DRY by reusing options in existing configurations.

In my opinion, I do not think that combining multiple configurations is a good way to achieve this. Most use cases and examples that I see involve combining multiple configurations for a different use case from the combined configurations. For example, --configuration production Is meant to go to production while --configuration production,stage is not meant to go to production. Furthermore, --configuration stage is never meant to be used on its own as it is only a partial configuration. In other words, having the ability to combine configurations, while technically cool, allows for a broader set of valid options than intended.

Rather, my initial thought is that extending configurations is a better way to achieve this. For example, to create a configuration for staging that is the same as production but just a little different, one would define staging as an extension of production. You could also define production as an extension of staging. This would allow for the same intended behavior without allowing for unintentional possibilities. How I imagine this looking in practice is in workspace.json, you can indicate that a configuration extends another:

{
  ...,
  "my-app": {
    "options": { ... },
    "configurations": {
      "production": {
        ...
      },
      "staging": {
        "__extends__": "production",
        "baseUrl": "https://stage.example.com"
      }
    }
  }
}

I understand that if the behavior from Angular CLI isn't supported, workspace.json can get unwieldy especially when building multiple apps for multiple environments with multiple languages etc. Tangentially, as mentioned in our Nx 12 Roadmap, we are planning on adding the ability to break up the project's configuration into their own configuration file.

With the ability to break up workspace.json as well as extending configurations, I think we can get the intended behavior without complicating the mental model.

As a result, I don't think it's a good idea to merge https://github.com/nrwl/nx/pull/4889. For the time being, my recommendation would be to duplicate the configuration which I understand is not ideal.

What do you all think about this? If this all sounds good, we will address this soon and get to a much better place for configuring workspaces.

leon commented 3 years ago

Being able to extend sounds like a good plan.

The drawback is that we no longer will be 100% compatible with angular cli.

As long as it's well documented it shouldn't be a problem to migrate to it.

georgiee commented 3 years ago

Hello @FrozenPandaz , thanks for looking into this topic.

While I think your response fits 100% to what @leon intended to achieve it misses entirely the problem described in a linked "duplicate". Please see my comment https://github.com/nrwl/nx/issues/4296#issuecomment-768894491 in #4296.

There are two type of requirements around this features:

Those are two entirely different problems. To be honest, to me A) makes little sense. I'm a fan of being explicit as this has the lowest level of cognitive load to my brain and makes reading code easier but it has its fans I'm sure.

But B) is almost an entirely different domain of problem. I absolutely need to mix my language configuration with different environments (development, staging, production) to serve or build the desired configuration. That feature of composing configurations always seemed really off to me. It's like the Angular CLI team decided for the wrong location to place the language configuration but when you look into the details of what you can do with "locale" configuration it makes a little bit more sense again. For example your are allowed to change any configuration per locale (because it is a configuration) and one example is to change the main.ts per locale, outlined in one of the examples, do initialize your app differently per locale.

See this example: https://angular.io/guide/i18n#localize-build-command

"build": {
  ...
  "configurations": {
    ...
    "fr": {
      "localize": ["fr"],
      "main": "src/main.fr.ts",
      ...
    }
  }
},
"serve": {
  ...
  "configurations": {
    ...
    "fr": {
      "browserTarget": "*project-name*:build:fr"
    }
  }
}

A usual configuration rather looks like this:

   "i18n": {
  "sourceLocale": "en-US",
  "locales": {
    "fr": "src/locale/messages.fr.xlf",
    "de": "src/locale/messages.fr.xlf",
    "es": "src/locale/messages.fr.xlf"
  }
},
"architect": {
  "build": {
    "builder": "@angular-devkit/build-angular:browser",
    "options": {
    },
    "configurations": {
      "fr": {
        "localize": ["fr"]
      },
      "de": {
        "localize": ["de"]
      },
      "es": {
        "localize": ["es"]
      },

To build all of your locales you can do this:

yarn ng build --localize

but if you want to use only one locale you can also use the configuration composition:

yarn ng build --production,fr

this also works for serve if yo u set the appropriate locales:

"configurations": {
  "production": {
    "browserTarget": "i18example:build:production"
  },
   "fr": {
    "localize": ["fr"]
  },
  "de": {
    "localize": ["de"]
  },
  "es": {
    "localize": ["es"]
  },
}
yarn ng serve --fr

Here it falls apart.

yarn ng build --production,fr,de

If you chose to pass two locale-wise configurations to get two folders in dist: dist/de, dist/frin the production environment you might want to use this but that's wrong as the composition doesn't work in that extend. You need to create explicit options with "localize": ["es", "de"] in the angular.json.

You have already seen the property --localize. It is also exposed on the CLI level. You can use ng build --localize to use all locales available. It also seems to support to pass in an array --localize de,fr (see https://github.com/angular/angular-cli/blob/41a6fb82afa424c7bbbc9b9d40d8fde198d7ab55/packages/angular/cli/lib/config/schema.json#L2109-L2124) but the linked schema is only for the angular.json itself an describes the localize attribute and not the CLI option. So this fails yarn ng build --localize fr,de with unknown option fr,de.


Long story short, the configuration composition (that's how Angular calls this) has a real impact on teams relying on the native i18n handling. I'm very much aware that the Angular CLI team has here a lot of potential to improve the API surface but it works. It just works and that's why I would like to see Nx to support configuration composition. As said, yes, I can always pick ng to do the builds but I expected are more congruent API from Nx in this aspect.

Maybe you can evaluate your draft PR around providing this feature (https://github.com/nrwl/nx/pull/4889/files) again with those new information?

Thank you for considering this change in Nx ๐Ÿ™

tmtron commented 3 years ago

Absolutely agree with @georgiee

Our use case is to have different brandings per customer - basically the same as for locales: we need these in all stages: dev, production

We definitely want to stay angular compatible: i.e. use angular.json

Splaktar commented 3 years ago

Agree with the above here as well. My client isn't that concerned with (A) (mixing staging, production, test configs), they are very concerned with (B) (mixing locales and brands/customer builds).

They are also not happy with the divergence from the Angular CLI functionality. They still consider it a bug for Nx CLI to not have a feature that the Angular CLI has, especially for migrations of projects that heavily use and depend upon this feature.

The extending feature sounds good, but has it been discussed with the Angular CLI team? would they consider implementing something compatible with this?

aiprogs commented 3 years ago

We definitely want to stay angular compatible: i.e. use angular.json, too.

tmtron commented 3 years ago

We use nx V 11.5.1 and even calling ng directly does not work. e.g..

./node_modules/@angular/cli/bin/ng build project --configuration=production,branding

I guess it's because nx has "decorated" the ngcommand.
How can we call the real ng?

Update 26.03.2021 Quote from dogmatico's comment

We removed the call to decorate-angula-cli script from the package.json postinstall hook.

johanchouquet commented 3 years ago

I also agree with @georgiee here. We can compose configurations not only for the stage (dev / prod) with locales, but all other things, such as pwa and more. In my company, I use this feature extensively. I also would like to Nx to be 100% compatible with Angular Cli. The extension feature is quite interesting, but I see it fits for a specific use case. Composing configuration allows an extreme flexibility, and offers me a solution where I know I will find myself in good hands.

JamieWritesCode commented 3 years ago

Our use case is to have different brandings per customer - basically the same as for locales: we need these in all stages: dev, production

We also share a similar use case and would like to maintain compatibility with the angular cli. I agree that the extension feature is interesting and a good solution to potential unintended configuration compositions as described by @FrozenPandaz however the ability to compose configs on the fly, while less declarative and explicit, does have some value in its flexibility.

I would be happy to see the extension feature implemented and even documented as the suggested way to compose configs for workspace.json but the default angular cli behaviour should also be supported.

khanyuriy commented 3 years ago

Same for us. We have to copy paste configuration common to production for all environments.

raphael22 commented 2 years ago

Any news on this ? Serve and build should accept multi-configurations as Angular CLI does. Copy/Paste of big configurations is so frustating.

stefanmo commented 2 years ago

I just spent 2 hours rewriting my configs only to end up on this thread with no solutions because I also changed my angular.json file to version 2 at some point in the past so I cannot remove the decorate-angular-cli script ๐Ÿ˜ž

Are there any plans for this to be fixed in the near future?

Coly010 commented 2 years ago

Hey! ๐Ÿ™‚

We have looked at this issue deeply and discussed it at length.

Some context

This isn't a feature that we can support natively within Nx as it will have major conflicts with other aspects of Nx itself. The configuration property is used for tasks such as:


To combine multiple configurations

There are a few things that you can do

nx build app --configuration=staging --assets=apps/app/src/assets/qa --localize fr
{
"targets:" {
    "build:dev-qa": {
      "executor": "@angular-devkit/architect:allOf",
      "options": {
        "targets": [
          {
            "target": "app1:build:development,qa"
          }
        ]
      }
    }
  }
}
{
  "targets": {
    "build": {
      ...
      "configurations": {
        "staging-qa" : {
           ...
           // options you need from Staging config
           "assets": [
             "apps/app/src/assets/qa"
           ],
           "localize": ["fr"]
        }
      }
    }
  }
}
nx build app --configuration=staging-qa

For i18n use cases

It is possible to pass multiple --localize flags.

If you really need this command to be executed as a single command, then you have multiple further options available to you:

nx build app --configuration=qa-fr-de
{
  "scripts": {
    ...
    "app:build:qa-fr-de": "nx build app --configuration=qa --localize fr --localize de"
  }
}
npm run app:build:qa-fr-de

Thank you all for raising this issue and communicating the use cases you have for this feature. We apologise for not being able to support it natively, however, hopefully the workarounds listed above should aid you.

I'm more than welcome to help people implement the workarounds if you are having difficulties.

raphael22 commented 2 years ago

@Coly010 Hi ! Thanks to the teams for have a look at this.

I think the main issue is that we still can't merge configuration despite your solutions. For large project the issue will be to duplicate every part of assets for a new config that change only one entry of those assets.

allOf can be a solution for build (despite having to create a new target entry for each combination), but what about "serve dev-qa" ?

If configuration.assets could have a mergeStrategy its would be perfect.

webberwang commented 2 years ago

I found a bug & it seems related to this, so I created a separate issue

g3tr1ght commented 2 years ago

Threads like this get me wondering as to why "caching feature" is so involved and tangled in the core design. My personal take is one shouldn't design any solutions around "caching" concept. One should be able to come up with a basic design and add optional features like caching atop which consumers can switch on/off based on the specific usage requirements. Caching has always been an optional thing in software. Imagine we had built networks specifically based on the caching concept, I doubt we would have been able to use internet at all. Every non-simplistic project has some requirements and custom scenario-based setup which require the tooling to work providing basic functionality and being able to switch on/off additional features. I would expect to be able to either apply multiple configurations to a particular app build task via CLI args or split configurations into multiple JS chunks and merge in a custom script (e.g. webpack, eslint). This feature is more fundamental than caching from my point of view.

In any case, awesome work nrwl team! The project is coming along very well and I'm stoked about your progress.

DzmVasileusky commented 2 years ago

@raphael22 Regarding serve we are doing it like this

        "serve": {
            "executor": "@angular-builders/custom-webpack:dev-server",
            "configurations": {
               ...
                "debug-staging": {
                    "browserTarget": "app:build:debug,debug-staging" // here two configs are mixed
                }
            }
            ...
        }
paulstelzer commented 1 year ago

It is still not working to make multipe configurations work in nx while this is working in angular. Any new information about this? PS: Workaround mentioned in https://github.com/nrwl/nx/issues/2839#issuecomment-1059075550 is working so it seems the only solution

binarybro commented 1 year ago

It's very counter-intuitive to apply all of these workarounds when migrating to Nx. Thank you for providing some examples, but I had to apply https://github.com/nrwl/nx/issues/2839#issuecomment-1191945895 to make it work, and now I'm not using the @nrwl executor. Who knows when I will run into issues with this again.

jihoun commented 1 year ago

As a more flexible workaround would it be realistic to have configuration files in javascript rather than json? Then we could easily write reusable blocks with infinite flexibility:

const production = {
      optimization: true,
      extractLicenses: true,
      inspect: false,
};
const alphaEnv = {
  fileReplacements:[/*...*/];
};
const betaEnv = {
  fileReplacements:[/*...*/];
};
module.exports = {
  targets: {
    build: {
      /* ... */
      configurations: {
        "alpha-prod": {
            ...production,
            ...alphaEnv,
            /* environment specific options */
        },
       "beta-prod":  {
          ...production,
          ...betaEnv,
          /* environment specific options */
       }
    }
  }
};
andresbm05 commented 1 year ago

Hi, to add some more use cases. Currently I'm facing this issue and in our case we have configuration set per partner, device and environment. Currently we have 17 partner options, 4 device options and 2 environments.

With the configuration composition we only need to define these 23 configuration and mix and match as we please, but with the workaround you suggest we would need 136 different configurations (a bit less as not all combinations are used but still a lot), that's insane!

Plus another thing to mention is that imo decorating something, like the angular CLI, should add features but never remove existing features. As a workaround, if having multiple configuration is not possible with the nx cli it could just fall back and use the default ng cli for that.

IslombekHasan commented 1 year ago

Thank you all for raising this issue and communicating the use cases you have for this feature. We apologise for not being able to support it natively, however, hopefully the workarounds listed above should aid you.

I don't understand. How did it work before then? It was clearly there in <Nx 11. Something changed in nx 11 that broke this behavior...

Rafarel commented 1 year ago

When you set the defaultConfiguration to production,fr in your build target for example it will work ! So why can't we do the same with the --configuration argument? This is crazy as I have to declare mixed configuration for every case. Please we have to fix/add this feature. Plus I am pretty sure it was working this way not so long ago? Thanks!

I opened an issue that was considered as a duplicate but I'm explaining my case here : https://github.com/nrwl/nx/issues/17275

Have a great day!

webberwang commented 1 year ago

There should be a central place to set project configuration for all projects, in nx.json preferably. This way we don't need to add them as cli flags

vigie commented 1 year ago

In my case I have a module-federation configuration that just updates the value for customWebpackConfig. I'm not prepared to add n more configurations across all other existing ones to get this to work. None of the workarounds look right. Looking at how to call the original ng as the only solution at this point.

shlajin commented 1 year ago

I want to distinguish dev/prod builds and dev/prod envs. For example, if a bug occurs in production, I find it easier to run the app in production env (i.e. connect to production servers) to debug the problem from my local computer, BUT use a dev build, so I don't have to fight with code minification and source maps.

What I do now is a custom executor that merges a bunch of .env files and then calls an underlying command. It works, but it feels clunky.

ntrp commented 1 year ago

I am confused, it's a simple thing while at the same time pretty common and requested, why did is not getting any priority? It's been like 3 years now..

If you don't want to implement config merging then simply add support for js base configs (like every single node based tools does) so, at least, people can compose their config without duplicating huge blocks to change one value.

anakreon commented 8 months ago

If this is still not officially supported, I think the least of the efforts for the dev team could be to add some kind of notification or an error message that this is the case. Especially since people may expect the command to mirror Angular CLI where this is supported. I just went through the automated migration from Angular which also migrated the comma-separated configurations with no issues. And the build command also seemed to work with no issues. There is literally zero warning that your configurations are not applied.