Fix exit code of webpack build by adding --bail

[ypresto]

Without --bail webpack error returns exit code of 0 which is ignored by CI services.

webpack/webpack#708 (comment)

This change is 

[coveralls]

Coverage remained the same at 99.329% when pulling f17fb5d on ypresto:bail-error-on-webpack-fail into 08756b2 on shakacode:master.

[coveralls]

Coverage remained the same at 99.329% when pulling f17fb5d on ypresto:bail-error-on-webpack-fail into 08756b2 on shakacode:master.

[coveralls]

Coverage remained the same at 99.329% when pulling f17fb5d on ypresto:bail-error-on-webpack-fail into 08756b2 on shakacode:master.

[coveralls]

Coverage remained the same at 99.329% when pulling f17fb5d on ypresto:bail-error-on-webpack-fail into 08756b2 on shakacode:master.

[ypresto]

Travis fails, but it seems random failure not related to this PR..?

[justin808]

@ypresto I'm not quite sure on what exactly is this fixing. Can you provide some example?

Should this change get merged over to: https://github.com/shakacode/react-webpack-rails-tutorial

CC: @squadette, since you might make some changes for webpack v2.

[ypresto]

I'm using wercker (CI service) and calling npm run build:production on it to ensure webpack bundle is buildable. So if there is syntax error in JS code CI will (should) fail.

But webpack does not return exit code other than zero when without --bail option so CI cannot fail even there is a error raised by webpack.

Then our CI (and perhaps assets:precompile on deploy) missed compile error and we unexpectedly deployed app which does not work...

[justin808]

@ypresto Please see: #732

Are you using the built-in deployment via automatic rake precompile task?

@ypresto can you please try on a very simple case of webpack failing, with and with out the --bail option and see what this line does:

https://github.com/shakacode/react_on_rails/blob/master/lib/tasks/assets.rake#L55

# Sprockets independent tasks
namespace :react_on_rails do
  namespace :assets do
    desc <<-DESC
Compile assets with webpack
Uses command defined with ReactOnRails.configuration.npm_build_production_command
sh "cd client && `ReactOnRails.configuration.npm_build_production_command`"
    DESC
    task webpack: :environment do
      if ReactOnRails.configuration.npm_build_production_command.present?
        sh "cd client && #{ReactOnRails.configuration.npm_build_production_command}"
      end
    end
  end
end

@robwise @alexfedoseev I think the bug is that we're not forcing the rake task to fail on the line that's running: ReactOnRails.configuration.npm_build_production_command.

By contrast, I think we're doing the correct thing here:

https://github.com/shakacode/react_on_rails/blob/master/lib/react_on_rails/test_helper/webpack_assets_compiler.rb#L11

# You can replace this implementation with your own for use by the
# ReactOnRails::TestHelper.ensure_assets_compiled helper
module ReactOnRails
  module TestHelper
    class WebpackAssetsCompiler
      def compile_assets
        puts "\nBuilding Webpack assets..."

        build_output = `cd client && #{ReactOnRails.configuration.npm_build_test_command}`

        raise "Error in building assets!\n#{build_output}" unless Utils.last_process_completed_successfully?

        puts "Completed building Webpack assets."
      end
    end
  end
end

@ypresto Given all this, would it make more sense to just change the code around assets.rake rather than recommending --bail?

@robwise @alexfedoseev any opnions?

I opened up issue #730.

[justin808]

@ypresto Any comments?

[robwise]

Is this one of those differences between backticks and sh? If what you're saying is true, then we need to do both (change to use bail and also fix it so we fail if the process exits 1)?

[justin808]

@robwise is correct. @ypresto Great start. Let us know if you want me to finish this one up, or if you can do it?

[ypresto]

Are you using the built-in deployment via automatic rake precompile task?

Yes, we call yarn insatll then rails assets:precompile in deployment script.

---

Putting syntax error in *.tsx (TypeScript with JSX) file in my project produced below output:

with --bail

...
[at-loader] Using typescript@2.2.1 from typescript and "tsconfig.json" from /app/client/tsconfig.json.

Error
    at NormalModule.<anonymous> (/app/client/node_modules/webpack/lib/NormalModule.js:113:20)
    at NormalModule.onModuleBuild (/app/client/node_modules/webpack-core/lib/NormalModuleMixin.js:310:10)
    at nextLoader (/app/client/node_modules/webpack-core/lib/NormalModuleMixin.js:275:25)
    at /app/client/node_modules/webpack-core/lib/NormalModuleMixin.js:292:15
    at context.callback (/app/client/node_modules/webpack-core/lib/NormalModuleMixin.js:148:14)
    at /app/client/node_modules/awesome-typescript-loader/src/index.ts:104:13
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
rails aborted!
Command failed with status (1): [cd client && yarn run build:production...]
...

without --bail

...
ERROR in [at-loader] app/bundles/path/to/component.tsx:137:22
    TS2304: Cannot find name 'FoobarProps'.
Done in 5.00s.
I, [2017-02-27T18:10:45.546250 #1]  INFO -- : Writing /app/public/assets/application_static-7120673678374b7f075befc5065cd17c167caee2af07e195251a48fccd452aa5.js
I, [2017-02-27T18:10:45.548660 #1]  INFO -- : Writing /app/public/assets/application_static-7120673678374b7f075befc5065cd17c167caee2af07e195251a48fccd452aa5.js.g
...

[justin808]

Thanks @ypresto!

@robwise Seems that --bail AND checking the result status is needed!

[justin808]

WTF -- webpack v1.x does not return an exist status of non-zero without --bail @robwise.

[ypresto]

WTF -- webpack v1.x does not return an exist status of non-zero without --bail

Yes, it is absolutely confusing feature which should only be enabled with --watch. >_<

[justin808]

@ypresto This was fixed in Webpack v2. So I won't change the defaults installer for the --bail, but rather for webpack v2.