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

Build executor: class name minification should be optional when using optimization: true #8537

Closed
alexd6631 opened this issue Jan 14, 2022 · 5 comments · Fixed by #8567
Closed
Labels
outdated scope: react Issues related to React support for Nx type: bug

Comments

@alexd6631
Copy link

Current Behavior

Since 13.4, when building for production with optimization on, it also minify the class names causing a variety of issues with library depending on reflection.

There are several issues already reported :

Expected Behavior

When building for production, class name minification should be easily configurable and opt-out in build settings, so we can have the same behavior as 13.2.
Disabling optimization could be a workaround, but it also disable the process.env.NODE_ENV substitution, which is problematic in my case

Steps to Reproduce

Create a simple node js app, it should generate a production configuration.

"configurations": {
        "production": {
          "optimization": true,
          "extractLicenses": true,
          "inspect": false,
          "fileReplacements": [
            {
              "replace": "apps/server/src/environments/environment.ts",
              "with": "apps/server/src/environments/environment.prod.ts"
            }
          ]
        }
      },

Build as usual :
npx nx build --configuration=production

Inspect the output main.js in dist/ : class names are minified

Environment

Node : 16.13.1
 OS   : darwin arm64
 npm  : 8.1.2
 
 nx : 13.4.4
 @nrwl/angular : undefined
 @nrwl/cli : 13.4.4
 @nrwl/cypress : 13.4.4
 @nrwl/devkit : 13.4.4
 @nrwl/eslint-plugin-nx : 13.4.4
 @nrwl/express : undefined
 @nrwl/jest : 13.4.4
 @nrwl/linter : 13.4.4
 @nrwl/nest : 13.4.4
 @nrwl/next : undefined
 @nrwl/node : 13.4.4
 @nrwl/nx-cloud : undefined
 @nrwl/react : undefined
 @nrwl/react-native : undefined
 @nrwl/schematics : undefined
 @nrwl/tao : 13.4.4
 @nrwl/web : undefined
 @nrwl/workspace : 13.4.4
 @nrwl/storybook : undefined
 @nrwl/gatsby : undefined
 typescript : 4.5.4
 rxjs : 7.5.2
 ---------------------------------------
 Community plugins:
        @angular/animations: 13.1.2
        @angular/cdk: 13.1.2
        @angular/common: 13.1.2
        @angular/compiler: 13.1.2
        @angular/core: 13.1.2
        @angular/forms: 13.1.2
        @angular/material: 13.1.2
        @angular/platform-browser: 13.1.2
        @angular/platform-browser-dynamic: 13.1.2
        @angular/router: 13.1.2
        @ngrx/effects: 13.0.2
        @ngrx/entity: 13.0.2
        @ngrx/router-store: 13.0.2
        @ngrx/store: 13.0.2
        @ngrx/store-devtools: 13.0.2
        @angular-devkit/build-angular: 13.1.3
        @angular/compiler-cli: 13.1.2
        @angular/language-service: 13.1.2
@FrozenPandaz FrozenPandaz added the scope: react Issues related to React support for Nx label Jan 15, 2022
@bilalshaikh42
Copy link

We are also running into this issue. I am not sure what optimization does other than minify class names/variables. But if there are other features, it would be useful to be able to have optimized builds while not breaking the reflection due to changes in the class names.
This is needed for a whole bunch of things in NestJs including OpenApi generation, class validation, database validation, input parsing, etc.

@alexd6631
Copy link
Author

I investigated a bit, it seems a possible quick fix would be to configure TerserPlugin to not mangle names in node.config.ts

const TerserPlugin = require('terser-webpack-plugin');
...
 if (options.optimization) {
        webpackConfig.optimization = {
            minimize: true,
+            minimizer: [new TerserPlugin({
+                terserOptions: {
+                    mangle: false
+                },
+              })],
            concatenateModules: true,
        };
    }

I can make a PR of the quick fix if necessary, but Nx core dev team might want to expose the terserOptions in the build settings (so some users can still mangle the names and customize minification for production) so it would be more involved than this.

alexd6631 pushed a commit to alexd6631/nx that referenced this issue Jan 17, 2022
disable name mangling when building for production with optimization on,
in order not to break
features relying on runtime reflection

ISSUES CLOSED: nrwl#8537
@alexd6631
Copy link
Author

Hey,
I just wrapped the previous code in a PR, let's see how it get reviewed :)

alexd6631 pushed a commit to alexd6631/nx that referenced this issue Jan 17, 2022
disable name mangling when building for production with optimization on,
in order not to break
features relying on runtime reflection

ISSUES CLOSED: nrwl#8537
vsavkin pushed a commit that referenced this issue Jan 20, 2022
disable name mangling when building for production with optimization on,
in order not to break
features relying on runtime reflection

ISSUES CLOSED: #8537
@Tadjaur
Copy link

Tadjaur commented May 23, 2022

It seems that this solution does not work on my side.
After creating a custom webpack configuration that sets the environments variable (.env file) in a nestjs project, typeorm requests fail again.
After some research, I finally set optimization.minimize to false on webpackconfig.
Since this is a server project, I don't really need minification.

@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.

4 participants