Skip to content

fix: fails when failOnError or failOnWarning enabled - #72

Merged
ricardogobbosouza merged 3 commits into
masterfrom
fix-fails
Feb 4, 2021
Merged

fix: fails when failOnError or failOnWarning enabled#72
ricardogobbosouza merged 3 commits into
masterfrom
fix-fails

Conversation

@ricardogobbosouza

@ricardogobbosouza ricardogobbosouza commented Feb 1, 2021

Copy link
Copy Markdown
Contributor

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

Fix when failOnError or failOnWarning is enabled and there is an error or warning from eslint locking the compilation and not displaying anything.

Resolve #51, #21, #71

Breaking Changes

Additional Info

@codecov

codecov Bot commented Feb 1, 2021

Copy link
Copy Markdown

Codecov Report

Merging #72 (7818502) into master (4daddf5) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #72   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         8           
  Lines          245       243    -2     
  Branches        66        65    -1     
=========================================
- Hits           245       243    -2     
Impacted Files Coverage Δ
src/ESLintError.js 100.00% <ø> (ø)
src/linter.js 100.00% <ø> (ø)
src/options.js 100.00% <ø> (ø)
src/index.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4daddf5...3ce4e43. Read the comment docs.

Comment thread src/index.js Outdated

@alexander-akait alexander-akait left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do not set warnings and errors on empty array, when failOnError is disabled?

@ricardogobbosouza

Copy link
Copy Markdown
Contributor Author

You're right!
Do you think failOnError should be enabled by default?

@alexander-akait

Copy link
Copy Markdown
Member

Hard to say, don't think there is a correct answer, everyone decides how best to set up, but for me default true value is good to 0CJS, I want to fail if I have problems with linting by default

@ricardogobbosouza

Copy link
Copy Markdown
Contributor Author

For me it makes more sense too, not to generate a breaking change

Comment thread src/options.js
@ricardogobbosouza
ricardogobbosouza merged commit 8a72a8a into master Feb 4, 2021
@ricardogobbosouza
ricardogobbosouza deleted the fix-fails branch February 4, 2021 21:40
@decademoon

Copy link
Copy Markdown

It seems that failOnError and failOnWarning not only determines whether the build fails, but also whether errors/warnings will be reported. Is this intentional? I.e. if failOnError is false then lint errors are never reported.

Perhaps this code should be changed from:

https://github.com/webpack-contrib/eslint-webpack-plugin/blob/f6d26b75d6dd15ce5f1f9078492893b4bd54ae3c/src/index.js#L133-L144

To

 if (warnings && !options.failOnWarning) { 
   // @ts-ignore 
   compilation.warnings.push(warnings); 
 } else if (warnings && options.failOnWarning) { 
   // @ts-ignore 
   compilation.errors.push(warnings); 
 } 
  
 if (errors && options.failOnError) { 
   // @ts-ignore 
   compilation.errors.push(errors); 
 } else if (errors && !options.failOnError) {  // <-- added
   // @ts-ignore 
   compilation.warnings.push(errors); 
 }

@ricardogobbosouza

ricardogobbosouza commented Feb 5, 2021

Copy link
Copy Markdown
Contributor Author

Its intentional #72 (review)

Please open a PR with this change if possible 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

eslint-webpack-plugin failing the build despite having failOnError set to false

3 participants