search

TrySound / rollup-plugin-eslint

This plugin in migrated https://github.com/rollup/plugins/tree/master/packages/eslint
MIT License
61 stars 17 forks source link

throwError functionality improvement. #3

Closed git-jiby-me closed 7 years ago

git-jiby-me commented 8 years ago

My use case with throwError is that if there are eslint warnings to stop the build, and i think that is the intended use case for this option. However with how it is done, it ends up that you do stop the rollup process but with an error trace, which doesn't make much sense. For example 

Jibys-MacBook-Pro:web-tracker jibyjose$ npm run build

> web-tracker@0.0.1 build /Users/jibyjose/Workspace/Qumram/web-tracker
> rm -rf build; flow check && rollup -c .rolluprc.js;true

Found 0 errors

/Users/jibyjose/Workspace/Qumram/web-tracker/lib/logger.js
  5:3  warning  Unexpected console statement  no-console

✖ 1 problem (0 errors, 1 warning)

Warnings or errors were found
ESLintError: Warnings or errors were found
    at Error (native)
    at transform (/Users/jibyjose/Workspace/Qumram/web-tracker/node_modules/rollup-plugin-eslint/dist/index.js:43:27)
    at /Users/jibyjose/Workspace/Qumram/web-tracker/node_modules/rollup/www/ROLLUP/rollup/src/utils/transform.js:18:28
    at tryCatch (/Users/jibyjose/Workspace/Qumram/web-tracker/node_modules/rollup/www/ROLLUP/rollup/node_modules/es6-promise/lib/es6-promise/-internal.js:185:12)
    at invokeCallback (/Users/jibyjose/Workspace/Qumram/web-tracker/node_modules/rollup/www/ROLLUP/rollup/node_modules/es6-promise/lib/es6-promise/-internal.js:197:13)
    at publish (/Users/jibyjose/Workspace/Qumram/web-tracker/node_modules/rollup/www/ROLLUP/rollup/node_modules/es6-promise/lib/es6-promise/-internal.js:168:7)
    at flush (/Users/jibyjose/Workspace/Qumram/web-tracker/node_modules/rollup/www/ROLLUP/rollup/node_modules/es6-promise/lib/es6-promise/asap.js:87:5)
    at doNTCallback0 (node.js:428:9)
    at process._tickCallback (node.js:357:13)
    at Function.Module.runMain (module.js:459:11)

In the above the error trace doesn't give any added information and its just fairly ugly.

I am proposing to change the flag and the functionality, by replacing throwError by breakOnWarning and instead of throwing an error, doing an process.exit(1), which for you the previous case would result in an output like

Jibys-MacBook-Pro:web-tracker jibyjose$ npm run build

> web-tracker@0.0.1 build /Users/jibyjose/Workspace/Qumram/web-tracker
> rm -rf build; flow check && rollup -c .rolluprc.js;true

Found 0 errors

/Users/jibyjose/Workspace/Qumram/web-tracker/lib/logger.js
  5:3  warning  Unexpected console statement  no-console

✖ 1 problem (0 errors, 1 warning)

which is much nicer and to the point.

I will create a PR for this, which you can consider if this makes sense to you.

git-jiby-me commented 8 years ago

@TrySound PR https://github.com/TrySound/rollup-plugin-eslint/pull/4 tries to solve this by introducing an additional configuration option breakOnWarning

jsg2021 commented 7 years ago

Would be nice to have a way to not throw on warnings.