Nx: [Feature Request] Allow lint to fail if only warnings are found

Created on 30 Mar 2020  路  8Comments  路  Source: nrwl/nx

Hello,

I am using @nrwl/linter:lint and as per eslint specification command will not fail if only warnings are detected. There is however a argument:

--max-warnings Int             Number of warnings to trigger nonzero exit code - default: -1

WIth this argument I could usually make sure that lint process succedes only if no errors or warnings are found. This is useful because while most errors (like console.logs or undefined vars) are harmless during development I do not wish for those to end up in production code. We usually prevent this by setting --max-warning=0 for our production build checkup stage.

I know that there is a workaround and we could manually override all rules to error while doing the production build but this is time consuming and clunky because we have then 2 configurations that need maintaining.

I would much prefer having an option to set --max-warnings through @nrwl/linter:lint command. I think it would also be useful to others.

I am also open in submiting a PR for this feature is this is something we could agree on implementing.

Prerequisites

  • [x] I am running the latest version
  • [x] I checked the documentation (nx.dev) and found no answer
  • [x] I checked to make sure that this issue has not already been filed
  • [x] I'm reporting the issue to the correct repository (not related to React, Angular or any dependency)

Expected Behavior

Be able to run something like:

nx affected:lint --max-warning=0
nx affected:lint --strict // or something like this, more like a boolean

Current Behavior

There is no easy way to fail lint stage if only warnings are found.

Context

Latest nx project with stocks.

linter feature

Most helpful comment

@capJavert sorry I didn't come back to you earlier 馃檹. Other things came up and didn't have a chance to look at it again.

But just gave it a look now and I have a fix locally. Gonna push up a PR that adds a maxWarnings to the builder. I'll discuss it with the team

All 8 comments

@juristr just to check, is this something you guys would implement (and if yes when?) - or it could be easier (more welcome) if I implement this and create a PR?

Thanks!

Hi, I'm experiencing the same issue. In the meantime, how do I pass the option of --max-warnings=0 to eslint when I run an nx linting command like nx affected:lint? Thanks!

Because no response came from @nrwl team we just patched the lint library to always throw if warnings or errors are detected, here is a diff:

diff --git a/node_modules/@nrwl/linter/src/builders/lint/lint.impl.js b/node_modules/@nrwl/linter/src/builders/lint/lint.impl.js
index 7a17cd4..e27ae84 100644
--- a/node_modules/@nrwl/linter/src/builders/lint/lint.impl.js
+++ b/node_modules/@nrwl/linter/src/builders/lint/lint.impl.js
@@ -96,7 +96,7 @@ function run(options, context) {
             fs_1.writeFileSync(pathToFile, formattedResults);
         }
         if (bundledReport.warningCount > 0 && printInfo) {
-            context.logger.warn('Lint warnings found in the listed files.\n');
+            context.logger.error('Lint warnings found in the listed files.\n');
         }
         if (bundledReport.errorCount > 0 && printInfo) {
             context.logger.error('Lint errors found in the listed files.\n');
@@ -107,7 +107,7 @@ function run(options, context) {
             context.logger.info('All files pass linting.\n');
         }
         return {
-            success: options.force || bundledReport.errorCount === 0
+            success: options.force || (bundledReport.warningCount === 0 && bundledReport.errorCount === 0)
         };
     });
 }

You can use it with something like patch-package.

Hope we get some response from @nrwl team because it would be cool to implement this officially.

Ah I see, very cool, thanks!

@capJavert sorry I didn't come back to you earlier 馃檹. Other things came up and didn't have a chance to look at it again.

But just gave it a look now and I have a fix locally. Gonna push up a PR that adds a maxWarnings to the builder. I'll discuss it with the team

@capJavert merged 馃帀. Give it a try and let us know. Again sorry for the delay

Thanks, @juristr looks good, is there any date when this is going to land in next release?

@capJavert not yet but should be soon

Was this page helpful?
0 / 5 - 0 ratings

Related issues

olakara picture olakara  路  3Comments

MichaelWarneke picture MichaelWarneke  路  3Comments

Koslun picture Koslun  路  3Comments

danieldanielecki picture danieldanielecki  路  3Comments

ZempTime picture ZempTime  路  3Comments