Better documentation for test helpers, check config messages

* 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.

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

@@ -21,14 +21,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 you 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 +63,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,15 +75,15 @@ 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}"

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?
 
@@ -70,8 +70,8 @@ def self.ensure_assets_compiled(webpack_assets_status_checker: nil,
 
  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_dir} for "\
+  "outdated/missing bundles based on source_path #{source_path}"
  puts
  @printed_once = true
  end

lib/react_on_rails/utils.rb

@@ -90,7 +90,7 @@ 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)
+ File.join(generated_assets_dir, bundle_name)
  end
  end
 

spec/dummy/Gemfile.lock

@@ -7,7 +7,7 @@ GIT
 PATH
  remote: ../..
  specs:
- react_on_rails (11.0.0)
+ react_on_rails (11.0.3)
  addressable
  connection_pool
  execjs (~> 2.5)

spec/dummy/app/views/pages/react_helmet.erb

@@ -1,5 +1,10 @@
 
-<% react_helmet_app = react_component_hash("ReactHelmetApp", prerender: true, props: { hello: "world" }, trace: true) %>
+<% react_helmet_app = react_component_hash("ReactHelmetApp",
+ prerender: true,
+ props: { helloWorldData: { name: "Mr. Server Side Rendering"}},
+ id: "react-helmet-0",
+ trace: true) %>
+
 
 <% content_for :title do %>
  <%= react_helmet_app['title'] %>

spec/dummy/app/views/shared/_header.erb

@@ -81,7 +81,7 @@
  <%= link_to "Generator function returns object with renderedHtml", rendered_html_path %>
  </li>
  <li>
- <%= link_to "Generator function returns object with renderedHtml as another object", react_helmet_path %>
+ <%= link_to "Generator function returns object with renderedHtml as another object (react-helmet)", react_helmet_path %>
  </li>
  <li>
  <%= link_to "Image Example", image_example_path %>

spec/dummy/client/app/components/ReactHelmet.jsx

@@ -1,12 +1,14 @@
 import React from 'react';
 import { Helmet } from 'react-helmet';
+import HelloWorld from './HelloWorld';
 
 const EchoProps = (props) => (
  <div>
  <Helmet>
  <title>Custom page title</title>
  </Helmet>
  Props: {JSON.stringify(props)}
+ <HelloWorld {...props}/>
  </div>
 );
 

spec/dummy/config/initializers/react_on_rails.rb

@@ -19,7 +19,9 @@ def self.custom_context(view_context)
  config.node_modules_location = "client" # Pre 9.0.0 always used "client"
  config.build_production_command = "yarn run build:production"
  config.build_test_command = "yarn run build:test"
- config.generated_assets_dir = File.join(%w[public webpack], Rails.env)
+
+ # See webpacker.yml public_output_path for generated_assets_dir
+ # config.generated_assets_dir = File.join(%w[public webpack], Rails.env)
  config.webpack_generated_files = %w[manifest.json]
  config.server_bundle_js_file = "server-bundle.js"
  config.rendering_extension = RenderingExtension

spec/dummy/config/webpacker.yml

@@ -5,6 +5,7 @@ default: &default
  custom_compile: true
  # Cache manifest.json for performance
  cache_manifest: false
+ source_path: client/app
 
 development:
  <<: *default

spec/dummy/package.json

@@ -7,9 +7,10 @@
  "test": "yarn run build:test && yarn run lint && rspec",
  "lint": "cd client && yarn run lint",
  "build:clean": "rm -rf public/webpack || true",
+ "build:test": "(cd client && yarn run build:test)",
  "build:production:client": "(cd client && yarn run build:production:client)",
  "build:production:server": "(cd client && yarn run build:production:server)",
- "build:client": "(cd client && yarn run build:client",
+ "build:client": "(cd client && yarn run build:client)",
  "build:server": "(cd client && yarn run build:server)",
  "build:dev:client": "(cd client && yarn run build:dev:client)",
  "build:dev:server": "(cd client && yarn run build:dev:server)",

spec/dummy/spec/features/integration_spec.rb

@@ -207,9 +207,10 @@ def change_text_expect_dom_selector(dom_selector)
  subject { page }
  background { visit react_helmet_path }
  scenario "renderedHtmls should not have any errors" do
- expect(subject).to have_text 'Props: {"hello":"world"}'
+ expect(subject).to have_text 'Props: {"helloWorldData":{"name":"Mr. Server Side Rendering"}}'
  expect(subject).to have_css "title", text: /\ACustom page title\z/, visible: false
  expect(subject.html).to include("[SERVER] RENDERED ReactHelmetApp to dom node with id")
+ change_text_expect_dom_selector("div#react-helmet-0")
  end
  end
 

spec/dummy/spec/rails_helper.rb

@@ -43,8 +43,10 @@
 
 RSpec.configure do |config|
  # Ensure that if we are running js tests, we are using latest webpack assets
- # This will use the defaults of :js and :server_rendering meta tags
- ReactOnRails::TestHelper.configure_rspec_to_compile_assets(config)
+ ReactOnRails::TestHelper.configure_rspec_to_compile_assets(config, :requires_webpack_assets)
+ config.define_derived_metadata(file_path: %r{spec/(features|requests)}) do |metadata|
+ metadata[:requires_webpack_assets] = true
+ end
 
  # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures
  config.fixture_path = "#{::Rails.root}/spec/fixtures"