Address Review Comments

docs/api/view-helpers-api.md

@@ -27,7 +27,7 @@ Uncommonly used options:
 - **general options:**
   - **props:** Ruby Hash which contains the properties to pass to the react object, or a JSON string. If you pass a string, we'll escape it for you.
   - **prerender:** enable server-side rendering of a component. Set to false when debugging!
-  - **auto_load_bundle:** will automatically load bundle for component by calling `append_javascript_pack_tag` and `append_stylesheet_pack_tag` under the hood.
+  - **auto_load_bundle:** will automatically load the bundle for component by calling `append_javascript_pack_tag` and `append_stylesheet_pack_tag` under the hood.
   - **id:** Id for the div, will be used to attach the React component. This will get assigned automatically if you do not provide an id. Must be unique.
   - **html_options:** Any other HTML options get placed on the added div for the component. For example, you can set a class (or inline style) on the outer div so that it behaves like a span, with the styling of `display:inline-block`. You may also use an option of `tag: "span"` to replace the use of the default DIV tag to be a SPAN tag.
   - **trace:** set to true to print additional debugging information in the browser. Defaults to true for development, off otherwise. Only on the **client side** will you will see the `railsContext` and your props.

docs/guides/configuration.md

@@ -155,8 +155,8 @@ ReactOnRails.configure do |config|
   ################################################################################
   # FILE SYSTEM BASED COMPONENT REGISTRY
   ################################################################################
-  # components_subdirectory is the name of the directory that is used to automatically detect and register components 
-  # for the usage on the rails view. default is nil, you can enable the feature by updating it in the next line.
+  # components_subdirectory is the name of the subdirectory matched to detect and register components automatically
+  # The default is nil. You can enable the feature by updating it in the next line.
   config.components_subdirectory = "ror_components"
 
   # For automated component registry, `render_component` view helper method tries to load bundle for component from

docs/guides/file-system-based-automated-bundle-generation.md

@@ -16,7 +16,7 @@ default:
 For more details, see [Configuration and Code](https://github.com/shakacode/shakapacker#configuration-and-code) section in [shakapacker](https://github.com/shakacode/shakapacker/).
 
 ### Configure Components Subdirectory
-`components_subdirectory`  is the name of the directories that contain components that will be automatically registered and used in Rails views.
+`components_subdirectory`  is the name of the matched directories containing components that will be automatically registered for use by the view helpers.
 For example, configure `config/initializers/react_on_rails` to set the name for `components_subdirectory`.·
 
 ```rb

lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb

@@ -47,8 +47,8 @@
   ################################################################################
   # FILE SYSTEM BASED COMPONENT REGISTRY
   ################################################################################
-  # components_subdirectory is the directory that is used to automatically detect and register components for the usage
-  # on the rails view. default is nil, you can enable the feature by updating it in the next line.
+  # `components_subdirectory`  is the name of the matching directories that contain automatically registered components
+  # for use in the Rails views. The default is nil, you can enable the feature by updating it in the next line.
   # config.components_subdirectory = "ror_components"
   #
   # For automated component registry, `render_component` view helper method tries to load bundle for component from

lib/react_on_rails/helper.rb

@@ -92,13 +92,13 @@ def react_component(component_name, options = {})
     end
 
     def load_pack_for_component(component_name)
-      component_pack_file = generated_components_pack component_name
-      is_component_pack_present = File.exist? "#{component_pack_file}.jsx"
+      component_pack_file = generated_components_pack(component_name)
+      is_component_pack_present = File.exist?("#{component_pack_file}.jsx")
       is_development = ENV["RAILS_ENV"] == "development"
 
       if is_development && !is_component_pack_present
         ReactOnRails::PacksGenerator.generate
-        raise_generated_missing_pack_warning component_name
+        raise_generated_missing_pack_warning(component_name)
       end
 
       ReactOnRails::PacksGenerator.raise_nested_enteries_disabled unless ReactOnRails::WebpackerUtils.nested_entries?

lib/react_on_rails/packs_generator.rb

@@ -48,26 +48,26 @@ def raise_nested_enteries_disabled
     private
 
     def generate_packs
-      common_component_to_path.each_value { |component_path| create_pack component_path }
-      client_component_to_path.each_value { |component_path| create_pack component_path }
+      common_component_to_path.each_value { |component_path| create_pack(component_path) }
+      client_component_to_path.each_value { |component_path| create_pack(component_path) }
 
       create_server_pack if ReactOnRails.configuration.server_bundle_js_file.present?
     end
 
     def create_pack(file_path)
-      output_path = generated_pack_path file_path
-      content = pack_file_contents file_path
+      output_path = generated_pack_path(file_path)
+      content = pack_file_contents(file_path)
 
       File.write(output_path, content)
 
       puts(Rainbow("Generated Packs: #{output_path}").yellow)
     end
 
     def pack_file_contents(file_path)
-      registered_component_name = component_name file_path
+      registered_component_name = component_name(file_path)
       <<~FILE_CONTENT
         import ReactOnRails from 'react-on-rails';
-        import #{registered_component_name} from '#{relative_component_path_from_generated_pack file_path}';
+        import #{registered_component_name} from '#{relative_component_path_from_generated_pack(file_path)}';
 
         ReactOnRails.register({#{registered_component_name}});
       FILE_CONTENT
@@ -81,8 +81,8 @@ def create_server_pack
     end
 
     def generated_server_pack_file_content
-      common_components_for_server_bundle = common_component_to_path.delete_if { |k| server_component_to_path.key? k }
-      component_for_server_registration_to_path = common_components_for_server_bundle.merge server_component_to_path
+      common_components_for_server_bundle = common_component_to_path.delete_if { |k| server_component_to_path.key?(k) }
+      component_for_server_registration_to_path = common_components_for_server_bundle.merge(server_component_to_path)
 
       server_component_imports = component_for_server_registration_to_path.map do |name, component_path|
         "import #{name} from '#{relative_path(generated_server_bundle_file_path, component_path)}';"
@@ -110,46 +110,47 @@ def add_generated_pack_to_server_bundle
     end
 
     def generated_server_bundle_file_path
-      generated_server_bundle_file_name = component_name defined_server_bundle_file_path.sub(".js", "-generated.js")
+      file_ext = File.extname(defined_server_bundle_file_path)
+      generated_server_bundle_file_path = defined_server_bundle_file_path.sub("#{file_ext}", "-generated#{file_ext}")
+      generated_server_bundle_file_name = component_name(generated_server_bundle_file_path)
+      source_entry_path = ReactOnRails::WebpackerUtils.webpacker_source_entry_path
 
-      "#{source_entry_path}/#{generated_server_bundle_file_name}.js"
+      "#{source_entry_path}/#{generated_server_bundle_file_name}#{file_ext}"
     end
 
     def clean_generated_packs_directory
-      FileUtils.rm_rf generated_packs_directory_path
-      FileUtils.mkdir_p generated_packs_directory_path
+      FileUtils.rm_rf(generated_packs_directory_path)
+      FileUtils.mkdir_p(generated_packs_directory_path)
     end
 
     def defined_server_bundle_file_path
       ReactOnRails::Utils.server_bundle_js_file_path
     end
 
     def generated_packs_directory_path
-      "#{source_entry_path}/generated"
-    end
+      source_entry_path = ReactOnRails::WebpackerUtils.webpacker_source_entry_path
 
-    def source_entry_path
-      ReactOnRails::WebpackerUtils.webpacker_source_entry_path
+      "#{source_entry_path}/generated"
     end
 
     def relative_component_path_from_generated_pack(ror_component_path)
-      component_file_pathname = Pathname.new ror_component_path
+      component_file_pathname = Pathname.new(ror_component_path)
       component_generated_pack_path = generated_pack_path(ror_component_path)
-      generated_pack_pathname = Pathname.new component_generated_pack_path
+      generated_pack_pathname = Pathname.new(component_generated_pack_path)
 
       relative_path(generated_pack_pathname, component_file_pathname)
     end
 
     def relative_path(from, to)
-      from_path = Pathname.new from
-      to_path = Pathname.new to
+      from_path = Pathname.new(from)
+      to_path = Pathname.new(to)
 
-      relative_path = to_path.relative_path_from from_path
+      relative_path = to_path.relative_path_from(from_path)
       relative_path.sub("../", "")
     end
 
     def generated_pack_path(file_path)
-      "#{generated_packs_directory_path}/#{component_name file_path}.jsx"
+      "#{generated_packs_directory_path}/#{component_name(file_path)}.jsx"
     end
 
     def component_name(file_path)
@@ -174,7 +175,7 @@ def client_component_to_path
       client_specific_components = component_name_to_path(client_render_components_paths)
 
       duplicate_components = common_component_to_path.slice(*client_specific_components.keys)
-      duplicate_components.each_key { |component| raise_client_component_overrides_common component }
+      duplicate_components.each_key { |component| raise_client_component_overrides_common(component) }
 
       client_specific_components
     end
@@ -184,10 +185,10 @@ def server_component_to_path
       server_specific_components = component_name_to_path(server_render_components_paths)
 
       duplicate_components = common_component_to_path.slice(*server_specific_components.keys)
-      duplicate_components.each_key { |component| raise_server_component_overrides_common component }
+      duplicate_components.each_key { |component| raise_server_component_overrides_common(component) }
 
       server_specific_components.each_key do |k|
-        raise_missing_client_component(k) unless client_component_to_path.key? k
+        raise_missing_client_component(k) unless client_component_to_path.key?(k)
       end
 
       server_specific_components
@@ -204,10 +205,14 @@ def components_subdirectory
     end
 
     def webpack_assets_status_checker
+      source_path = ReactOnRails::Utils.source_path
+      generated_assets_full_path = ReactOnRails::Utils.generated_assets_full_path
+      webpack_generated_files = ReactOnRails.configuration.webpack_generated_files
+
       @webpack_assets_status_checker ||= ReactOnRails::TestHelper::WebpackAssetsStatusChecker.new(
-        source_path: ReactOnRails::Utils.source_path,
-        generated_assets_full_path: ReactOnRails::Utils.generated_assets_full_path,
-        webpack_generated_files: ReactOnRails.configuration.webpack_generated_files
+        source_path: source_path,
+        generated_assets_full_path: generated_assets_full_path,
+        webpack_generated_files: webpack_generated_files
       )
     end
 
@@ -283,7 +288,7 @@ def minimum_required_shakapacker_version
     def prepend_to_file_if_not_present(file, text_to_prepend)
       file_content = File.read(file)
 
-      return if file_content.include? text_to_prepend
+      return if file_content.include?(text_to_prepend)
 
       content_with_prepended_text = text_to_prepend + file_content
       File.write(file, content_with_prepended_text)

lib/react_on_rails/test_helper/webpack_assets_status_checker.rb

@@ -26,11 +26,11 @@ def initialize(
       end
 
       def stale_generated_webpack_files
-        stale_generated_files client_files
+        stale_generated_files(client_files)
       end
 
       def stale_generated_component_packs
-        stale_generated_files component_pack_files
+        stale_generated_files(component_pack_files)
       end
 
       def stale_generated_files(files)
@@ -39,7 +39,7 @@ def stale_generated_files(files)
 
         return ["manifest.json"] if manifest_needed
 
-        most_recent_mtime = find_most_recent_mtime files
+        most_recent_mtime = find_most_recent_mtime(files)
         all_compiled_assets.each_with_object([]) do |webpack_generated_file, stale_gen_list|
           if !File.exist?(webpack_generated_file) ||
              File.mtime(webpack_generated_file) < most_recent_mtime