From 6cc88951b0812bbc765c5c339c8ad9eeb67c7326 Mon Sep 17 00:00:00 2001 From: pulkitkkr Date: Thu, 18 Aug 2022 14:46:35 -0400 Subject: [PATCH] Address Review Comments --- docs/api/view-helpers-api.md | 2 +- docs/guides/configuration.md | 4 +- ...ystem-based-automated-bundle-generation.md | 2 +- .../config/initializers/react_on_rails.rb | 4 +- lib/react_on_rails/helper.rb | 6 +- lib/react_on_rails/packs_generator.rb | 63 ++++++++++--------- .../webpack_assets_status_checker.rb | 6 +- 7 files changed, 46 insertions(+), 41 deletions(-) diff --git a/docs/api/view-helpers-api.md b/docs/api/view-helpers-api.md index fe023736f2..c23431e42f 100644 --- a/docs/api/view-helpers-api.md +++ b/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. diff --git a/docs/guides/configuration.md b/docs/guides/configuration.md index c820f7f95c..3b2bf32ac6 100644 --- a/docs/guides/configuration.md +++ b/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 diff --git a/docs/guides/file-system-based-automated-bundle-generation.md b/docs/guides/file-system-based-automated-bundle-generation.md index 52d08c2e6c..d42a5dacad 100644 --- a/docs/guides/file-system-based-automated-bundle-generation.md +++ b/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 diff --git a/lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb b/lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb index 4d95b2f46c..17fca3ce97 100644 --- a/lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb +++ b/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 diff --git a/lib/react_on_rails/helper.rb b/lib/react_on_rails/helper.rb index 4dd23b34e5..6e8bc8f0e3 100644 --- a/lib/react_on_rails/helper.rb +++ b/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? diff --git a/lib/react_on_rails/packs_generator.rb b/lib/react_on_rails/packs_generator.rb index 9f07b4eaf3..ecab95e596 100644 --- a/lib/react_on_rails/packs_generator.rb +++ b/lib/react_on_rails/packs_generator.rb @@ -48,15 +48,15 @@ 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) @@ -64,10 +64,10 @@ def create_pack(file_path) 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,14 +110,17 @@ 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 @@ -125,31 +128,29 @@ def defined_server_bundle_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) diff --git a/lib/react_on_rails/test_helper/webpack_assets_status_checker.rb b/lib/react_on_rails/test_helper/webpack_assets_status_checker.rb index c48a275553..bd41f18cf8 100644 --- a/lib/react_on_rails/test_helper/webpack_assets_status_checker.rb +++ b/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