Check ESLint warnings and errors in npm run build

[mjomble]

Fixes #440

Test plan:

echo foo() > template/src/index.js
npm run build

Expected output:

Creating an optimized production build...
Compiled with warnings.

Warning in ./template/src/index.js

E:\dev\create-react-app\template\src\index.js
  1:1  warning  'foo' is not defined  no-undef

✖ 1 problem (0 errors, 1 warning)


You may use special comments to disable some warnings.
Use // eslint-disable-next-line to ignore the next line.
Use /* eslint-disable */ to ignore all warnings in a file.

followed by npm errors.

Should not show "Compiled successfully." message.

Also, when running npm start and there are warnings, but the server is still started, this should appear after the warnings:

The app is running at:

  http://localhost:3000/

Note that the development build is not optimized.
To create a production build, use npm run build.

[gaearon]

Thanks! I’m currently on a little hiatus so I will review this in bulk before cutting 0.3.0.

[gaearon]

(In the meantime please make sure Travis doesn't fail.)

[mjomble]

Travis build fixed. Should I squash the commits or is it ok to keep them separate?

[gaearon]

Keeping separate is OK, GitHub will squash them on merge.

[gaearon]

Here is what I see on a lint warning:

Instead of npm cryptic crash, I would like the regular build output at the end. It should still build the project successfully if only warnings were encountered.

Let’s also change the error output in the error case (I mean actual build error) to make it more like npm start output when build fails. I mean the red “Failed to compile.” message that is missing in npm run build output.

[mjomble]

In this discussion, we agreed that ESLint warnings should fail the build :)

[gaearon]

Haha, sorry :P. I now think it might make more sense to do three things:

1. Do not fail on lint warnings
2. Fail on build or lint errors
3. Promote "no-undef" to be an error (Turn some of the linter messages into errors #498)

Does this make sense? Sorry for the churn.

[mjomble]

Yeah, makes sense, I'll try to update this soon 👍

[gaearon]

Hi, do you think you’ll have time to finish it?

[mjomble]

Yes, but I don't know when exactly :)
If someone else can find time for it sooner, I won't mind if they finish it instead.

[gaearon]

Okay, no worries for now, but please let us know if you can’t pick it up within a couple of weeks.
Cheers!

[gaearon]

I’m closing as this is outdated and the PR has been open for more than a month.
I’m happy to review it again if you revive it addressing the comments!

[bogas04]

Hi !

I'm building a chrome extension using create-react-app, and due to this change, I can't use chrome because it's obviously undefined in the code base. I guess I can alter .eslintrc at root to fix this, but I'm not sure how:

{
  "extends": "react-app",
  "globals": {
    "chrome": true
  }
}

This doesn't seem to be working. Can you guide me a bit on how to extend eslint rules. I learned about "extends" from #808

For now I've just used global comment (which works) as all chrome related calls reside in one file in my case.

/* global chrome */

[gaearon]

For now I've just used global comment (which works) as all chrome related calls reside in one file in my case.

That's the intended solution. Alternatively you can probably read window.chrome.