Better documentation for test helpers, check config messages

.travis.yml

@@ -37,7 +37,7 @@ before_install:
 
 install:
  - travis_retry gem install bundler
- - travis_retry nvm install node 8.0.0
+ - travis_retry nvm install 9.11.1
  - node -v
  - travis_retry npm i -g yarn@0.27.5
  - travis_retry yarn global add babel-cli

CHANGELOG.md

@@ -10,6 +10,12 @@ Changes since last non-beta release.
 
 *Please add entries here for your pull requests that are not yet released.*
 
+#### Changed
+- Throw if configuration.generated_assets_dir specified, and using webpacker, and if that doesn't match the public_output_path. Otherwise, warn if generated_assets_dir is specified
+- Fix the setup for tests for spec/dummy so they automatically rebuild by correctly setting the source_path in the webpacker.yml
+- Updated documentation for the testing setup.
+- Above in [PR 1072](https://github.com/shakacode/react_on_rails/pull/1072) by [justin808](https://github.com/justin808).
+
 ### [11.0.3] - 2018-04-24
 
 #### Fixed

docs/additional-reading/rspec-configuration.md

@@ -8,25 +8,25 @@ RSpec.configure do |config|
  ReactOnRails::TestHelper.configure_rspec_to_compile_assets(config)
 ```
 
-You can pass one or more RSpec metatags as an optional second parameter to this helper method if you want this helper to run on examples other than where `:js` or `:server_rendering` (those are the defaults). The helper will compile webpack files at most once per test run. The helper will not compile the webpack files unless they are out of date (stale). The helper is configurable in terms of what command is used to prepare the files.
+You can pass one or more RSpec metatags as an optional second parameter to this helper method if you want this helper to run on examples other than where `:js`, `:server_rendering`, or `:controller` (those are the defaults). The helper will compile webpack files at most once per test run. The helper will not compile the webpack files unless they are out of date (stale). The helper is configurable in terms of what command is used to prepare the files. If you don't specify these metatags for your relevant JavaScript tests, then you'll need to do the following.
 
 If you are using Webpack to build CSS assets, you should do something like this to ensure that you assets are built for any specs under `specs/requests` or `specs/features`:
 
 ```ruby
  ReactOnRails::TestHelper.configure_rspec_to_compile_assets(config, :requires_webpack_assets)
-
- # Because we're using some CSS Webpack files, we need to ensure the webpack files are generated
- # for all feature specs. https://github.com/shakacode/react_on_rails/issues/792
  config.define_derived_metadata(file_path: %r{spec/(features|requests)}) do |metadata|
  metadata[:requires_webpack_assets] = true
  end
 ```
 
 Please take note of the following:
+- If you are using Webpacker, be **SURE** to configure the `source_path` in your `config/webpacker.yml` unless you are using the defaults for webpacker. If you are not using webpacker, all files in the node_modules_location are used for your test sources.
+
 - This utility uses your `build_test_command` to build the static generated files. This command **must not** include the `--watch` option. If you have different server and client bundle files, this command **must** create all the bundles. If you are using webpacker, the default value will come from the `config/webpacker.yml` value for the `public_output_path` and the `source_path`
+
 - If you add an older file to your source files, that is already older than the produced output files, no new recompilation is done. The solution to this issue is to clear out your directory of webpack generated files when adding new source files that may have older dates. This is actually a common occurrence when you've built your test generated files and then you sync up your repository files.
 
-- By default, the webpack processes look for the `app/assets/webpack` folders (configured as setting `webpack_generated_files` in the `config/react_on_rails.rb`. If this folder is missing, is empty, or contains files in the `config.webpack_generated_files` list with `mtime`s older than any of the files in your `client` folder, the helper will recompile your assets. You can override the location of these files inside of `config/initializers/react_on_rails.rb` by passing a filepath (relative to the root of the app) to the `generated_assets_dir` configuration option.
+- By default, the webpack processes look for the `config.generated_assets_dir` folder for generated files, configured via setting `webpack_generated_files`, in the `config/react_on_rails.rb`. If the `config.generated_assets_dir` folder is missing, is empty, or contains files in the `config.webpack_generated_files` list with `mtime`s older than any of the files in your `client` folder, the helper will recompile your assets. You can override the location of these files inside of `config/initializers/react_on_rails.rb` by passing a filepath (relative to the root of the app) to the `generated_assets_dir` configuration option.
 
 The following `config/react_on_rails.rb` settings **must** match your setup:
 ```ruby

docs/basics/configuration.md

@@ -47,7 +47,8 @@ ReactOnRails.configure do |config|
  # Alternately, you may configure this. It is relative to your Rails root directory.
  # A custom, non-webpacker, config might use something like:
  #
- config.generated_assets_dir = File.join(%w[public webpack], Rails.env)
+ # config.generated_assets_dir = File.join(%w[public webpack], Rails.env)
+ # This setting should not be used if using webpacker. 
 
  # The test helper needs to know where your JavaScript files exist. The default is configured
  # by your config/webpacker.yml soure_path:

lib/react_on_rails/configuration.rb

@@ -1,7 +1,5 @@
 # frozen_string_literal: true
 
-# NOTE: ReactOnRails::Utils.using_webpacker? always will return false when called here.
-
 module ReactOnRails
  def self.configure
  yield(configuration)
@@ -21,14 +19,37 @@ def self.setup_config_values
  check_i18n_directory_exists
  check_i18n_yml_directory_exists
  check_server_render_method_is_only_execjs
+ error_if_using_webpacker_and_generated_assets_dir_not_match_public_output_path
+ end
+
+ def self.error_if_using_webpacker_and_generated_assets_dir_not_match_public_output_path
+ return unless ReactOnRails::WebpackerUtils.using_webpacker?
+
+ return if @configuration.generated_assets_dir.blank?
+
+ webpacker_public_output_path = ReactOnRails::WebpackerUtils.webpacker_public_output_path
+
+ if File.expand_path(@configuration.generated_assets_dir) == webpacker_public_output_path.to_s
+ Rails.logger.warn("You specified /config/initializers/react_on_rails.rb generated_assets_dir "\
+ "with Webpacker. Remove this line from your configuration file.")
+ else
+ msg = <<-MSG.strip_heredoc
+ Error configuring /config/initializers/react_on_rails.rb: You are using webpacker
+ and your specified value for generated_assets_dir = #{@configuration.generated_assets_dir}
+ that does not match the value for public_output_path specified in
+ webpacker.yml = #{webpacker_public_output_path}. You should remove the configuration
+ value for "generated_assets_dir" from your config/initializers/react_on_rails.rb file.
+ MSG
+ raise ReactOnRails::Error, msg
+ end
  end
 
  def self.check_server_render_method_is_only_execjs
  return if @configuration.server_render_method.blank? ||
  @configuration.server_render_method == "ExecJS"
 
  msg = <<-MSG.strip_heredoc
- Error configuring /config/react_on_rails.rb: invalid value for `config.server_render_method`.
+ Error configuring /config/initializers/react_on_rails.rb: invalid value for `config.server_render_method`.
  If you wish to use a server render method other than ExecJS, contact justin@shakacode.com
  for details.
  MSG
@@ -40,7 +61,7 @@ def self.check_i18n_directory_exists
  return if Dir.exist?(@configuration.i18n_dir)
 
  msg = <<-MSG.strip_heredoc
- Error configuring /config/react_on_rails.rb: invalid value for `config.i18n_dir`.
+ Error configuring /config/initializers/react_on_rails.rb: invalid value for `config.i18n_dir`.
  Directory does not exist: #{@configuration.i18n_dir}. Set to value to nil or comment it
  out if not using the React on Rails i18n feature.
  MSG
@@ -52,30 +73,39 @@ def self.check_i18n_yml_directory_exists
  return if Dir.exist?(@configuration.i18n_yml_dir)
 
  msg = <<-MSG.strip_heredoc
- Error configuring /config/react_on_rails.rb: invalid value for `config.i18n_yml_dir`.
+ Error configuring /config/initializers/react_on_rails.rb: invalid value for `config.i18n_yml_dir`.
  Directory does not exist: #{@configuration.i18n_yml_dir}. Set to value to nil or comment it
  out if not using this i18n with React on Rails, or if you want to use all translation files.
  MSG
  raise ReactOnRails::Error, msg
  end
 
  def self.ensure_generated_assets_dir_present
- return if @configuration.generated_assets_dir.present?
+ return if @configuration.generated_assets_dir.present? || ReactOnRails::WebpackerUtils.using_webpacker?
 
  @configuration.generated_assets_dir = DEFAULT_GENERATED_ASSETS_DIR
- puts "ReactOnRails: Set generated_assets_dir to default: #{DEFAULT_GENERATED_ASSETS_DIR}"
+ Rails.logger.warn "ReactOnRails: Set generated_assets_dir to default: #{DEFAULT_GENERATED_ASSETS_DIR}"
  end
 
  def self.configure_generated_assets_dirs_deprecation
  return if @configuration.generated_assets_dirs.blank?
 
- puts "[DEPRECATION] ReactOnRails: Use config.generated_assets_dir rather than "\
+ if ReactOnRails::WebpackerUtils.using_webpacker?
+ webpacker_public_output_path = ReactOnRails::WebpackerUtils.webpacker_public_output_path
+ Rails.logger.warn "Error configuring config/initializers/react_on_rails. Define neither the "\
+ "generated_assets_dirs no the generated_assets_dir when using Webpacker. This is defined by "\
+ "public_output_path specified in webpacker.yml = #{webpacker_public_output_path}."
+ return
+ end
+
+ Rails.logger.warn "[DEPRECATION] ReactOnRails: Use config.generated_assets_dir rather than "\
  "generated_assets_dirs"
  if @configuration.generated_assets_dir.blank?
  @configuration.generated_assets_dir = @configuration.generated_assets_dirs
  else
- puts "[DEPRECATION] ReactOnRails. You have both generated_assets_dirs and "\
- "generated_assets_dir defined. Define ONLY generated_assets_dir"
+ Rails.logger.warn "[DEPRECATION] ReactOnRails. You have both generated_assets_dirs and "\
+ "generated_assets_dir defined. Define ONLY generated_assets_dir if NOT using Webpacker"\
+ " and define neither if using Webpacker"
  end
  end
 
@@ -91,14 +121,18 @@ def self.ensure_webpack_generated_files_exists
  def self.ensure_server_bundle_js_file_has_no_path
  return unless @configuration.server_bundle_js_file.include?(File::SEPARATOR)
 
- puts "[DEPRECATION] ReactOnRails: remove path from server_bundle_js_file in configuration. "\
- "All generated files must go in #{@configuration.generated_assets_dir}"
+ assets_dir = ReactOnRails::Utils.generated_assets_path
  @configuration.server_bundle_js_file = File.basename(@configuration.server_bundle_js_file)
+
+ Rails.logger_warn do
+ "[DEPRECATION] ReactOnRails: remove path from server_bundle_js_file in configuration. "\
+ "All generated files must go in #{assets_dir}. Using file basename #{@configuration.server_bundle_js_file}"
+ end
  end
 
  def self.configure_skip_display_none_deprecation
  return if @configuration.skip_display_none.nil?
- puts "[DEPRECATION] ReactOnRails: remove skip_display_none from configuration."
+ Rails.logger.warn "[DEPRECATION] ReactOnRails: remove skip_display_none from configuration."
  end
 
  def self.configuration

lib/react_on_rails/server_rendering_pool.rb

@@ -10,7 +10,7 @@ module ReactOnRails
  module ServerRenderingPool
  class << self
  def react_on_rails_pro?
- @react_on_rails_pro ||= gem_available?("react_on_rails_pro")
+ @react_on_rails_pro ||= ReactOnRails::Utils.gem_available?("react_on_rails_pro")
  end
 
  def pool
@@ -26,16 +26,6 @@ def pool
  def server_render_js_with_console_logging(js_code, render_options)
  pool.exec_server_render_js(js_code, render_options)
  end
-
- private
-
- def gem_available?(name)
- Gem::Specification.find_by_name(name)
- rescue Gem::LoadError
- false
- rescue StandardError
- Gem.available?(name)
- end
  end
  end
 end

lib/react_on_rails/test_helper.rb

@@ -31,7 +31,7 @@ module TestHelper
  # Params:
  # config - config for rspec
  # metatags - metatags to add the ensure_assets_compiled check.
- # Default is :js, :server_rendering
+ # Default is :js, :server_rendering, :controller
  def self.configure_rspec_to_compile_assets(config, *metatags)
  metatags = %i[js server_rendering controller] if metatags.empty?
 
@@ -48,30 +48,30 @@ def self.configure_rspec_to_compile_assets(config, *metatags)
  # defaults to ReactOnRails::TestHelper::WebpackAssetsStatusChecker
  # webpack_assets_compiler: provide one method: `def compile`
  # defaults to ReactOnRails::TestHelper::WebpackAssetsCompiler
- # source_path and generated_assets_dir are passed into the default webpack_assets_status_checker if you
+ # source_path and generated_assets_path are passed into the default webpack_assets_status_checker if you
  # don't provide one.
  # webpack_generated_files List of files to check for up-to-date-status, defaulting to
  # webpack_generated_files in your configuration
  def self.ensure_assets_compiled(webpack_assets_status_checker: nil,
  webpack_assets_compiler: nil,
  source_path: nil,
- generated_assets_dir: nil,
+ generated_assets_path: nil,
  webpack_generated_files: nil)
  ReactOnRails::WebpackerUtils.check_manifest_not_cached
  if webpack_assets_status_checker.nil?
  source_path ||= ReactOnRails::Utils.source_path
- generated_assets_dir ||= ReactOnRails::Utils.generated_assets_dir
+ generated_assets_path ||= ReactOnRails::Utils.generated_assets_path
  webpack_generated_files ||= ReactOnRails.configuration.webpack_generated_files
 
  webpack_assets_status_checker ||=
  WebpackAssetsStatusChecker.new(source_path: source_path,
- generated_assets_dir: generated_assets_dir,
+ generated_assets_path: generated_assets_path,
  webpack_generated_files: webpack_generated_files)
 
  unless @printed_once
  puts
- puts "====> React On Rails: Checking #{webpack_assets_status_checker.generated_assets_dir} for "\
- "outdated/missing bundles"
+ puts "====> React On Rails: Checking files in #{webpack_assets_status_checker.generated_assets_path} for "\
+  "outdated/missing bundles based on source_path #{source_path}"
  puts
  @printed_once = true
  end

lib/react_on_rails/test_helper/webpack_assets_status_checker.rb

@@ -13,14 +13,14 @@ class WebpackAssetsStatusChecker
  # source_path is typically configured in the webpacker.yml file
  # for `source_path`
  # or for legacy React on Rails, it's /client, where all client files go
- attr_reader :source_path, :generated_assets_dir
+ attr_reader :source_path, :generated_assets_path
 
  def initialize(
- generated_assets_dir: required("generated_assets_dir"),
+ generated_assets_path: required("generated_assets_path"),
  source_path: required("source_path"),
  webpack_generated_files: required("webpack_generated_files")
  )
- @generated_assets_dir = generated_assets_dir
+ @generated_assets_path = generated_assets_path
  @source_path = source_path
  @webpack_generated_files = webpack_generated_files
  end
@@ -59,13 +59,13 @@ def all_compiled_assets
  if webpack_generated_files.present?
  webpack_generated_files
  else
- file_list = make_file_list(make_globs(generated_assets_dir)).to_ary
+ file_list = make_file_list(make_globs(generated_assets_path)).to_ary
  puts "V" * 80
  puts "Please define config.webpack_generated_files (array) so the test helper knows "\
  "which files are required. If you are using webpacker, you typically need to only "\
  "include 'manifest.json'."
  puts "Detected the possible following files to check for webpack compilation in "\
- "#{generated_assets_dir}"
+ "#{generated_assets_path}"
  puts file_list.join("\n")
  puts "^" * 80
  file_list

lib/react_on_rails/utils.rb

@@ -6,6 +6,7 @@
 require "active_support"
 require "active_support/core_ext/string"
 
+# Naming: goal is that path is a full_path and dir means relative to the rails root.
 module ReactOnRails
  module Utils
  # https://forum.shakacode.com/t/yak-of-the-week-ruby-2-4-pathname-empty-changed-to-look-at-file-size/901
@@ -76,8 +77,7 @@ def self.server_bundle_js_file_path
  begin
  bundle_js_file_path(bundle_name)
  rescue Webpacker::Manifest::MissingEntryError
- Rails.root.join(File.join(Webpacker.config.public_output_path,
- bundle_name)).to_s
+ File.join(ReactOnRails::WebpackerUtils.public_output_path, bundle_name)
  end
  else
  bundle_js_file_path(bundle_name)
@@ -90,7 +90,8 @@ def self.bundle_js_file_path(bundle_name)
  else
  # Default to the non-hashed name in the specified output directory, which, for legacy
  # React on Rails, this is the output directory picked up by the asset pipeline.
- File.join(ReactOnRails.configuration.generated_assets_dir, bundle_name)
+ # For Webpacker, this is the public output path defined in the webpacker.yml file.
+ File.join(generated_assets_path, bundle_name)
  end
  end
 
@@ -130,12 +131,20 @@ def self.source_path
  end
  end
 
- def self.generated_assets_dir
+ def self.generated_assets_path
  if ReactOnRails::WebpackerUtils.using_webpacker?
  ReactOnRails::WebpackerUtils.webpacker_public_output_path
  else
- ReactOnRails.configuration.generated_assets_dir
+ File.expand_path(ReactOnRails.configuration.generated_assets_dir)
  end
  end
+
+ def self.gem_available?(name)
+ Gem::Specification.find_by_name(name)
+ rescue Gem::LoadError
+ false
+ rescue StandardError
+ Gem.available?(name)
+ end
  end
 end