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

Rakefile

@@ -2,6 +2,9 @@
 
 # Rake will automatically load any *.rake files inside of the "rakelib" folder
 # See rakelib/
+
+require_relative "./spec/react_on_rails/support/rails32_helper"
+
 tasks = %w[run_rspec lint]
 prepare_for_ci = %w[node_package dummy_apps]
 
@@ -11,7 +14,7 @@ if ENV["USE_COVERALLS"] == "TRUE"
   tasks << "coveralls:push"
 end
 
-if File.basename(ENV["BUNDLE_GEMFILE"] || "") == "Gemfile.rails32"
+if using_rails32?
   tasks = %w[run_rspec:gem_rails32 run_rspec:dummy_no_webpacker]
   prepare_for_ci = %w[node_package dummy_apps:dummy_no_webpacker]
 end

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_full_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/react_on_rails_helper.rb

@@ -485,7 +485,9 @@ def rails_context(server_side: required("server_side"))
           inMailer: in_mailer?,
           # Locale settings
           i18nLocale: I18n.locale,
-          i18nDefaultLocale: I18n.default_locale
+          i18nDefaultLocale: I18n.default_locale,
+          rorVersion: ReactOnRails::VERSION,
+          rorPro: ReactOnRails::Utils.react_on_rails_pro?
         }
         if defined?(request) && request.present?
           # Check for encoding of the request's original_url and try to force-encoding the

lib/react_on_rails/server_rendering_pool.rb

@@ -9,12 +9,8 @@
 module ReactOnRails
   module ServerRenderingPool
     class << self
-      def react_on_rails_pro?
-        @react_on_rails_pro ||= gem_available?("react_on_rails_pro")
-      end
-
       def pool
-        @pool ||= if react_on_rails_pro?
+        @pool ||= if ReactOnRails::Utils.react_on_rails_pro?
                     ReactOnRailsPro::ServerRenderingPool::ProRendering
                   else
                     ReactOnRails::ServerRenderingPool::RubyEmbeddedJavaScript
@@ -26,16 +22,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,31 @@ 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_full_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_full_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_full_path ||= ReactOnRails::Utils.generated_assets_full_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_full_path: generated_assets_full_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_full_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_full_path
 
       def initialize(
-        generated_assets_dir: required("generated_assets_dir"),
+        generated_assets_full_path: required("generated_assets_full_path"),
         source_path: required("source_path"),
         webpack_generated_files: required("webpack_generated_files")
       )
-        @generated_assets_dir = generated_assets_dir
+        @generated_assets_full_path = generated_assets_full_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_full_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_full_path}"
             puts file_list.join("\n")
             puts "^" * 80
             file_list

lib/react_on_rails/utils.rb

@@ -76,8 +76,8 @@ 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.webpacker_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_full_path, bundle_name)
       end
     end
 
@@ -130,12 +131,24 @@ def self.source_path
       end
     end
 
-    def self.generated_assets_dir
+    def self.generated_assets_full_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
+
+    def self.react_on_rails_pro?
+      @react_on_rails_pro ||= gem_available?("react_on_rails_pro")
+    end
   end
 end