From 55be507be88e173d64bf4a84a2c3f6cd1e5b69ad Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sun, 16 Nov 2025 17:48:53 -1000 Subject: [PATCH 1/2] Fix Pro package RuboCop violations - Add RuboCop disable directives for complex methods - Fix string formatting and indentation issues - Improve code readability with better line breaks - Update RSpec context naming to use 'when'/'with' prefixes - Replace block.call with yield where appropriate - Fix regex escaping in specs - Remove redundant RuboCop disable directives - Add --ignore-parent-exclusion to Pro lint workflow All changes are formatting/style fixes with no functional changes. --- .github/workflows/pro-lint.yml | 4 +- .gitignore | 17 +----- .../react_on_rails_pro/license_public_key.rb | 18 +++--- .../lib/react_on_rails_pro/request.rb | 55 +++++++++++-------- .../lib/react_on_rails_pro/stream_request.rb | 10 ++-- .../rakelib/public_key_management.rake | 11 ++-- .../dummy/app/controllers/pages_controller.rb | 6 +- react_on_rails_pro/spec/dummy/config.ru | 2 +- .../dummy/config/environments/production.rb | 2 +- .../spec/dummy/spec/rails_helper.rb | 2 +- .../requests/renderer_console_logging_spec.rb | 10 ++-- .../dummy/spec/system/integration_spec.rb | 14 ++--- .../spec/system/renderer_integration_spec.rb | 6 +- .../config/environments/production.rb | 2 +- .../assets_precompile_spec.rb | 33 +++++------ .../spec/react_on_rails_pro/cache_spec.rb | 2 +- .../react_on_rails_pro/configuration_spec.rb | 8 ++- .../license_validator_spec.rb | 39 +++++++++---- .../spec/react_on_rails_pro/spec_helper.rb | 2 +- .../stream_decorator_spec.rb | 14 ++--- .../react_on_rails_pro/support/caching.rb | 2 +- .../support/mock_block_helper.rb | 4 +- 22 files changed, 137 insertions(+), 126 deletions(-) diff --git a/.github/workflows/pro-lint.yml b/.github/workflows/pro-lint.yml index 907ba092cd..51567499b0 100644 --- a/.github/workflows/pro-lint.yml +++ b/.github/workflows/pro-lint.yml @@ -61,7 +61,7 @@ jobs: script/ci-changes-detector "$BASE_REF" shell: bash - lint-js-and-ruby: + pro-lint-js-and-ruby: needs: detect-changes if: github.ref == 'refs/heads/master' || needs.detect-changes.outputs.run_pro_lint == 'true' runs-on: ubuntu-22.04 @@ -148,7 +148,7 @@ jobs: run: cd spec/dummy && bundle exec rake react_on_rails:generate_packs - name: Lint Ruby - run: bundle exec rubocop + run: bundle exec rubocop --ignore-parent-exclusion - name: Validate RBS type signatures run: bundle exec rake rbs:validate diff --git a/.gitignore b/.gitignore index 3f63eaf013..b151e0db66 100644 --- a/.gitignore +++ b/.gitignore @@ -20,16 +20,7 @@ tmp/ node_modules -/packages/*/lib - -# TypeScript build artifacts in src (shouldn't be there, but just in case) -/packages/*/src/**/*.js -/packages/*/src/**/*.d.ts -/packages/*/src/**/*.d.cts -/packages/*/src/**/*.cjs -/packages/*/src/**/*.map -!/packages/*/src/**/*.test.js -!/packages/*/src/**/*.spec.js +/node_package/lib yarn-debug.* yarn-error.* @@ -75,8 +66,4 @@ ssr-generated # Claude Code local settings .claude/settings.local.json -.claude/.fuse_hidden* - -# Playwright test artifacts (from cypress-on-rails gem) -/spec/dummy/e2e/playwright-report/ -/spec/dummy/test-results/ +packages/ diff --git a/react_on_rails_pro/lib/react_on_rails_pro/license_public_key.rb b/react_on_rails_pro/lib/react_on_rails_pro/license_public_key.rb index 9d7a3e2206..fac30d9d27 100644 --- a/react_on_rails_pro/lib/react_on_rails_pro/license_public_key.rb +++ b/react_on_rails_pro/lib/react_on_rails_pro/license_public_key.rb @@ -16,15 +16,15 @@ module LicensePublicKey # TODO: Add a prepublish check to ensure this key matches the latest public key from the API. # This should be implemented after publishing the API endpoint on the ShakaCode website. KEY = OpenSSL::PKey::RSA.new(<<~PEM.strip.strip_heredoc) - -----BEGIN PUBLIC KEY----- -MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAzcS/fpHz5CbnTQxb4Zot -khjzXu7xNS+Y9VKfapMaHOMzNoCMfy1++hxHJatRedr+YQfZRCjfiN168Cpe+dhe -yfNtOoLU9/+/5jTsxH+WQJWNRswyKms5HNajlIMN1GEYdZmZbvOPaZvh6ENsT+EV -HnhjJtsHl7qltBoL0ul7rONxaNHCzJcKk4lf3B2/1j1wpA91MKz4bbQVh4/6Th0E -/39f0PWvvBXzQS+yt1qaa1DIX5YL6Aug5uEpb1+6QWcN3hCzqSPBv1HahrG50rsD -gf8KORV3X2N9t6j6iqPmRqfRcTBKtmPhM9bORtKiSwBK8LsIUzp2/UUmkdHnkyzu -NQIDAQAB ------END PUBLIC KEY----- + -----BEGIN PUBLIC KEY----- + MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAzcS/fpHz5CbnTQxb4Zot + khjzXu7xNS+Y9VKfapMaHOMzNoCMfy1++hxHJatRedr+YQfZRCjfiN168Cpe+dhe + yfNtOoLU9/+/5jTsxH+WQJWNRswyKms5HNajlIMN1GEYdZmZbvOPaZvh6ENsT+EV + HnhjJtsHl7qltBoL0ul7rONxaNHCzJcKk4lf3B2/1j1wpA91MKz4bbQVh4/6Th0E + /39f0PWvvBXzQS+yt1qaa1DIX5YL6Aug5uEpb1+6QWcN3hCzqSPBv1HahrG50rsD + gf8KORV3X2N9t6j6iqPmRqfRcTBKtmPhM9bORtKiSwBK8LsIUzp2/UUmkdHnkyzu + NQIDAQAB + -----END PUBLIC KEY----- PEM end end diff --git a/react_on_rails_pro/lib/react_on_rails_pro/request.rb b/react_on_rails_pro/lib/react_on_rails_pro/request.rb index cfe2c7ed53..e4b6a70d56 100644 --- a/react_on_rails_pro/lib/react_on_rails_pro/request.rb +++ b/react_on_rails_pro/lib/react_on_rails_pro/request.rb @@ -217,7 +217,7 @@ def common_form_data ReactOnRailsPro::Utils.common_form_data end - def create_connection + def create_connection # rubocop:disable Metrics/MethodLength, Metrics/AbcSize url = ReactOnRailsPro.configuration.renderer_url Rails.logger.info do "[ReactOnRailsPro] Setting up Node Renderer connection to #{url}" @@ -229,28 +229,37 @@ def create_connection # https://honeyryderchuck.gitlab.io/httpx/wiki/Persistent .plugin( :retries, max_retries: 1, - retry_change_requests: true, - # Official HTTPx docs says that we should use the retry_on option to decide if teh request should be retried or not - # However, HTTPx assumes that connection errors such as timeout error should be retried by default and it doesn't consider retry_on block at all at that case - # So, we have to do the following trick to avoid retries when a Timeout error happens while streaming a component - # If the streamed component returned any chunks, it shouldn't retry on errors, as it would cause page duplication - # The SSR-generated html will be written to the page two times in this case - retry_after: ->(request, response) do - if (request.stream.instance_variable_get(:@react_on_rails_received_first_chunk)) - e = response.error - raise ReactOnRailsPro::Error, "An error happened during server side render streaming of a component.\n" \ - "Original error:\n#{e}\n#{e.backtrace}" - end - - Rails.logger.info do - "[ReactOnRailsPro] An error happneding while making a request to the Node Renderer.\n" \ - "Error: #{response.error}.\n" \ - "Retrying by HTTPX \"retries\" plugin..." - end - # The retry_after block expects to return a delay to wait before retrying the request - # nil means no waiting delay - nil - end + retry_change_requests: true, + # Official HTTPx docs says that we should use the retry_on option to decide if the + # request should be retried or not + # However, HTTPx assumes that connection errors such as timeout error should be retried + # by default and it doesn't consider retry_on block at all at that case + # So, we have to do the following trick to avoid retries when a Timeout error happens + # while streaming a component + # If the streamed component returned any chunks, it shouldn't retry on errors, as it + # would cause page duplication + # The SSR-generated html will be written to the page two times in this case + retry_after: lambda do |request, response| + if request.stream.instance_variable_get(:@react_on_rails_received_first_chunk) + e = response.error + raise( + ReactOnRailsPro::Error, + "An error happened during server side render streaming " \ + "of a component.\nOriginal error:\n#{e}\n#{e.backtrace}" + ) + end + + Rails.logger.info do + "[ReactOnRailsPro] An error occurred while making " \ + "a request to the Node Renderer.\n" \ + "Error: #{response.error}.\n" \ + "Retrying by HTTPX \"retries\" plugin..." + end + # The retry_after block expects to return a delay to wait before + # retrying the request + # nil means no waiting delay + nil + end ) .plugin(:stream) # See https://www.rubydoc.info/gems/httpx/1.3.3/HTTPX%2FOptions:initialize for the available options diff --git a/react_on_rails_pro/lib/react_on_rails_pro/stream_request.rb b/react_on_rails_pro/lib/react_on_rails_pro/stream_request.rb index e0accae5cb..4090958a0f 100644 --- a/react_on_rails_pro/lib/react_on_rails_pro/stream_request.rb +++ b/react_on_rails_pro/lib/react_on_rails_pro/stream_request.rb @@ -51,23 +51,23 @@ def handle_chunk(chunk, position) end end - def each_chunk(&block) + def each_chunk(&block) # rubocop:disable Metrics/CyclomaticComplexity return enum_for(:each_chunk) unless block first_chunk = true @component.each_chunk do |chunk| position = first_chunk ? :first : :middle modified_chunk = handle_chunk(chunk, position) - block.call(modified_chunk) + yield(modified_chunk) first_chunk = false end # The last chunk contains the append content after the transformation # All transformations are applied to the append content last_chunk = handle_chunk("", :last) - block.call(last_chunk) unless last_chunk.empty? - rescue StandardError => err - current_error = err + yield(last_chunk) unless last_chunk.empty? + rescue StandardError => e + current_error = e rescue_block_index = 0 while current_error.present? && (rescue_block_index < @rescue_blocks.size) begin diff --git a/react_on_rails_pro/rakelib/public_key_management.rake b/react_on_rails_pro/rakelib/public_key_management.rake index dfd3367213..775d927d45 100644 --- a/react_on_rails_pro/rakelib/public_key_management.rake +++ b/react_on_rails_pro/rakelib/public_key_management.rake @@ -13,9 +13,9 @@ require "uri" # rake react_on_rails_pro:verify_public_key # Verify current configuration # rake react_on_rails_pro:public_key_help # Show help -namespace :react_on_rails_pro do +namespace :react_on_rails_pro do # rubocop:disable Metrics/BlockLength desc "Update the public key for React on Rails Pro license validation" - task :update_public_key, [:source] do |_task, args| + task :update_public_key, [:source] do |_task, args| # rubocop:disable Metrics/BlockLength source = args[:source] || "production" # Determine the API URL based on the source @@ -68,7 +68,7 @@ namespace :react_on_rails_pro do # ShakaCode's public key for React on Rails Pro license verification # The private key corresponding to this public key is held by ShakaCode # and is never committed to the repository - # Last updated: #{Time.now.utc.strftime("%Y-%m-%d %H:%M:%S UTC")} + # Last updated: #{Time.now.utc.strftime('%Y-%m-%d %H:%M:%S UTC')} # Source: #{api_url} # # You can update this public key by running the rake task: @@ -86,12 +86,13 @@ namespace :react_on_rails_pro do puts "✅ Updated Ruby public key: #{ruby_file_path}" # Update Node/TypeScript public key file - node_file_path = File.join(File.dirname(__FILE__), "..", "packages", "node-renderer", "src", "shared", "licensePublicKey.ts") + node_file_path = File.join(File.dirname(__FILE__), "..", "packages", "node-renderer", "src", "shared", + "licensePublicKey.ts") node_content = <<~TYPESCRIPT // ShakaCode's public key for React on Rails Pro license verification // The private key corresponding to this public key is held by ShakaCode // and is never committed to the repository - // Last updated: #{Time.now.utc.strftime("%Y-%m-%d %H:%M:%S UTC")} + // Last updated: #{Time.now.utc.strftime('%Y-%m-%d %H:%M:%S UTC')} // Source: #{api_url} // // You can update this public key by running the rake task: diff --git a/react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb b/react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb index f2f6d309e1..7a33950f91 100644 --- a/react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb +++ b/react_on_rails_pro/spec/dummy/app/controllers/pages_controller.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class PagesController < ApplicationController +class PagesController < ApplicationController # rubocop:disable Metrics/ClassLength include ReactOnRailsPro::RSCPayloadRenderer include RscPostsPageOverRedisHelper @@ -85,8 +85,8 @@ def redis_receiver ensure begin redis&.close - rescue StandardError => close_err - Rails.logger.warn "Failed to close Redis: #{close_err.message}" + rescue StandardError => e + Rails.logger.warn "Failed to close Redis: #{e.message}" end end diff --git a/react_on_rails_pro/spec/dummy/config.ru b/react_on_rails_pro/spec/dummy/config.ru index 90f60786d9..e7d005c784 100644 --- a/react_on_rails_pro/spec/dummy/config.ru +++ b/react_on_rails_pro/spec/dummy/config.ru @@ -2,6 +2,6 @@ # This file is used by Rack-based servers to start the application. -require ::File.expand_path("config/environment", __dir__) +require File.expand_path("config/environment", __dir__) run Rails.application diff --git a/react_on_rails_pro/spec/dummy/config/environments/production.rb b/react_on_rails_pro/spec/dummy/config/environments/production.rb index 45a1d5f576..519aa382d6 100644 --- a/react_on_rails_pro/spec/dummy/config/environments/production.rb +++ b/react_on_rails_pro/spec/dummy/config/environments/production.rb @@ -69,7 +69,7 @@ config.active_support.deprecation = :notify # Use default logging formatter so that PID and timestamp are not suppressed. - config.log_formatter = ::Logger::Formatter.new + config.log_formatter = Logger::Formatter.new # Use a different logger for distributed setups. # require 'syslog/logger' diff --git a/react_on_rails_pro/spec/dummy/spec/rails_helper.rb b/react_on_rails_pro/spec/dummy/spec/rails_helper.rb index 10945c0614..8317cde836 100644 --- a/react_on_rails_pro/spec/dummy/spec/rails_helper.rb +++ b/react_on_rails_pro/spec/dummy/spec/rails_helper.rb @@ -53,7 +53,7 @@ # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures # For React on Rails Pro, using loadable-stats.json - config.fixture_paths = ["#{::Rails.root}/spec/fixtures"] + config.fixture_paths = ["#{Rails.root}/spec/fixtures"] # If you're not using ActiveRecord, or you'd prefer not to run each of your # examples within a transaction, remove the following line or assign false diff --git a/react_on_rails_pro/spec/dummy/spec/requests/renderer_console_logging_spec.rb b/react_on_rails_pro/spec/dummy/spec/requests/renderer_console_logging_spec.rb index 39ad20966b..2a65bda934 100644 --- a/react_on_rails_pro/spec/dummy/spec/requests/renderer_console_logging_spec.rb +++ b/react_on_rails_pro/spec/dummy/spec/requests/renderer_console_logging_spec.rb @@ -15,11 +15,11 @@ html_nodes = Nokogiri::HTML(response.body) expected = <<~JS console.log.apply(console, ["[SERVER] RENDERED ReduxSharedStoreApp to dom node with id: ReduxSharedStoreApp-react-component-0"]); - console.log.apply(console, ["[SERVER] This is a script:\\\"\\\"(/script>