Default Production Tasks is not Stopping When Webpack returns non-zero!

[justin808]

See discussion in #730

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?

[robwise]

Commented on PR, should we just do both?

[justin808]

100% we need to check the return status...

I'm not sure on the --bail because it might be preferable to see all the failures in the CI report or deployment. OTOH, it might be preferable for CI or deployment to fail more quickly. Default?

[robwise]

I think we default to bail early and then they can change it if they want CI to keep running

[justin808]

This issue is fixed by Webpack V2. Once that is merged...then it's up to the users to configure their package.json appropriately.

[justin808]

See #730 for a demonstration of how V2 fixes the issue.

[justin808]

I'll close this issue once I merge the Webpack v2 changes.

[justin808]

React on Rails is updated for Webpack v2! If you're on webpack v1, use the --bail option!