Create-react-app: Webpack warnings trigger build errors when running on CI

[scholtzm]

(Initial discussion starts here.)

Pull request https://github.com/facebookincubator/create-react-app/pull/944 adds a new behaviour related to CIs - crash the build during CI whenever linter warnings are encountered.

This PR however introduced new side effect - webpack warnings will also crash the build, e.g.:

This seems to be a pre-built javascript file. Though this is possible, it's not recommended. Try to require the original source to get better results.

We could consider this particular warning to be harmless.
Since we have no access to webpack config, we cannot use module.noParse option either.

[scholtzm]

My initial idea was to check warnings to see if there are any related to eslint-loader.

Warnings emitted by eslint-loader contain full loader path, e.g. /long/path/to/node_modules/eslint-loader/index.js.

I couldn't find anything else that would mention eslint or eslint-loader that could be also reliably used to determine that a specific warning comes from ESLint.

[gaearon]

Agree. I’m not sure about this specific fix but please send a PR to get the ball rolling.
Then we can consider alternatives.

[scholtzm]

I dug a bit further and couldn't find anything that would provide information about which loader emitted the warning in a reliable manner.

For example an ESLint warning about eqeqeq, loaders property looks like this:

[
  '/system/path/node_modules/babel-loader/index.js?{"babelrc":false,"presets":["/system/path/node_modules/babel-preset-react-app/index.js"]}',
  '/system/path/node_modules/eslint-loader/index.js'
]

There's also issuer and request which are just long ! webpack strings formed from the info above.

edit. Maybe we could use failOnError and failOnWarning provided by eslint-loader.

[kachkaev]

Same issue here. My temporary workaround was to unset CI variable in the build script (using .gitlab-ci.yml):

#...

build-app:
  stage: build-app
  cache:
    paths:
      - node_modules
    key: "node_modules"
    untracked: true
  script:
    - npm install
    - unset CI # <-------- this line helped
    - npm run build
  artifacts:
    paths:
      - build

#...

[scholtzm]

@kachkaev Are there any side effects? Because of this, I'm still stuck at 0.7.5.

[kachkaev]

@scholtzm I have not noticed any. Please share yours – others (including myself) may face them in the next project.

[gaearon]

Maybe we can ask Webpack to expose a way to turn that warning off? I never found it useful.

[scholtzm]

@kachkaev Thinking about it right now, I think Jest uses CI env var to determine certain settings.

@gaearon As per my initial post here, in my particular case, I'd have to use noParse option. I don't think there is a global setting that would allow us to disable this warning globally. Seems like noParse is the "official" way.

[gaearon]

https://github.com/webpack/webpack/issues/4263

[mikew]

Why not change the logic to process.env.CI && stats.compilation.errors.length?

[gaearon]

Because that's the opposite of the intention. The intention is to fail CI on warnings (but only useful warnings coming from ESLint).

FWIW I did try this with master, and I can’t reproduce this specific warning anymore. Which library should I try with?

[scholtzm]

In my case, it was triggered by localForage.

[gaearon]

Can you provide a code snippet?

[scholtzm]

Just simple import will trigger the warning.

https://github.com/scholtzm/arnold/blob/master/src/util/storage.js#L1

[mikew]

@gaearon

Because that's the opposite of the intention. The intention is to fail CI on warnings (but only useful warnings coming from ESLint).

That's my point. Eslint has errors and warnings, and changing the behaviour to treat warnings as errors is changing the semantics. When really they should just be changed to errors in cra's eslint config.

[gaearon]

Please see past discussions when people repeatedly asked to treat warnings as errors in CI. This is common practice.

[gaearon]

@scholtzm Can you check if the issue reproduces on master?

[scholtzm]

I will report back this weekend.

[holman]

fwiw, I've been getting the initial This seems to be a pre-built javascript file warning for some time, but I just upgraded to 1.0.x and it's being suppressed correctly now. :sparkles:

[gaearon]

The funny thing is we didn’t actually do anything to suppress it. I have no idea why it’s gone 😄

[gaearon]

I’ll close for now but if somebody can reproduce please give me instructions.

[holman]

Haha, MYSTERY FIX

I'll keep an eye out on my end for any weird changes here regardless.