Better message if server asset compile fails

[justin808]

If somebody is doing client side only rendering, maybe we shouldn't have a default for the server side rendering setup?

Maybe we can make this a bit smarter?

@ernie Any more details? do you have the samples of your config when it didn't work and what error message you saw?

[ernie]

When it didn't work:

RSpec.configure do |config|
  ReactOnRails::TestHelper.configure_rspec_to_compile_assets(config)
  # ...
end

ReactOnRails.configure do |config|
  config.server_bundle_js_file = "app/assets/javascripts/generated/server-bundle.js"
  # ...
end

Did some digging in the source, found the code looking at whether server_bundle_js_file was empty, and then found https://github.com/shakacode/react_on_rails/blob/51f77e0c512444c0eb65d2ac54669eb587b1f90b/docs/additional_reading/optional_configuration.md and noticed a comment that wasn't in my copy of the initializer:

# Set the server_bundle_js_file to "" if you know that you will not be server rendering.

That did the trick.

[justin808]

We need an else condition here if the file is not found and we should check that the file is not blank.

https://github.com/shakacode/react_on_rails/blob/master/lib%2Freact_on_rails%2Fconfiguration.rb#L38

 def initialize(server_bundle_js_file: nil, prerender: nil, replay_console: nil,
                   trace: nil, development_mode: nil,
                   logging_on_server: nil, server_renderer_pool_size: nil,
                   server_renderer_timeout: nil, raise_on_prerender_error: nil,
                   skip_display_none: nil, generated_assets_dirs: DEFAULT_GENERATED_ASSETS_DIRS)
      self.server_bundle_js_file = if File.exist?(server_bundle_js_file)
                                     server_bundle_js_file
                                   end

We can add a check here:

https://github.com/shakacode/react_on_rails/blob/master/lib%2Freact_on_rails%2Ftest_helper%2Fwebpack_assets_compiler.rb#L27

Instead of just:

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

We need something like:

unless Utils.last_process_completed_successfully?
 msg = "Error in building assets!\n#{build_output}"
 msg << "Check your react_on_rails.rb initializer to set `config.server_bundle_js_file` to blank if you are not using server rendering" if Utils.server_rendering_is_enabled? && build_output =~ /server/
fail msg

Later for the next major version bump

We should also change the default for the config.server_bundle_js_file:

https://github.com/shakacode/react_on_rails/blob/master/lib%2Freact_on_rails%2Fconfiguration.rb#L14

  def self.configuration
    @configuration ||= Configuration.new(
      generated_assets_dirs: DEFAULT_GENERATED_ASSETS_DIRS,
      server_bundle_js_file: File.join(*%w(app assets javascripts generated server.js)),

We need to do this once we have some other "breaking changes".

[ernie]

Also, when you run the install, wouldn't it be possible to default the server js to empty when someone uses --no-server-rendering? That would have prevented my issue altogether.

[justin808]

Bingo. Here's the main bug:

https://github.com/shakacode/react_on_rails/blob/master/lib%2Fgenerators%2Freact_on_rails%2Ftemplates%2Fbase%2Fbase%2Fconfig%2Finitializers%2Freact_on_rails.rb#L7

# Shown below are the defaults for configuration
ReactOnRails.configure do |config|
  # Client bundles are configured in application.js

  # Server rendering:
  # Server bundle is a single file for all server rendering of components.
  config.server_bundle_js_file = "app/assets/javascripts/generated/server-bundle.js"

[aaronvb]

Also, when you run the install, wouldn't it be possible to default the server js to empty when someone uses --no-server-rendering? That would have prevented my issue altogether.

@ernie Just added this in #295, thanks for the feedback!

[justin808]

First part fixed in Release 3.0.4.

[justin808]

We can re-open a new issue if there's more to do here.