diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bc1f2cd0a..024cc2df58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,15 @@ After a release, please make sure to run `bundle exec rake update_changelog`. Th Changes since the last non-beta release. +#### Improved + +- **Automatic Precompile Hook Coordination in bin/dev**: The `bin/dev` command now automatically runs Shakapacker's `precompile_hook` once before starting development processes and sets `SHAKAPACKER_SKIP_PRECOMPILE_HOOK=true` to prevent duplicate execution in spawned webpack processes. + - Eliminates the need for manual coordination, sleep hacks, and duplicate task calls in Procfile.dev + - Users can configure expensive build tasks (like locale generation or ReScript compilation) once in `config/shakapacker.yml` and `bin/dev` handles coordination automatically + - Includes warning for Shakapacker versions below 9.4.0 (the `SHAKAPACKER_SKIP_PRECOMPILE_HOOK` environment variable is only supported in 9.4.0+) + - The `SHAKAPACKER_SKIP_PRECOMPILE_HOOK` environment variable is set for all spawned processes, making it available for custom scripts that need to detect when `bin/dev` is managing the precompile hook + - Addresses [2091](https://github.com/shakacode/react_on_rails/issues/2091) by [justin808](https://github.com/justin808) + ### [v16.2.0.beta.12] - 2025-11-20 #### Added diff --git a/docs/building-features/i18n.md b/docs/building-features/i18n.md index 5e5d06610e..fdfd9da30d 100644 --- a/docs/building-features/i18n.md +++ b/docs/building-features/i18n.md @@ -21,10 +21,31 @@ You can use [Rails internationalization (i18n)](https://guides.rubyonrails.org/i 3. The locale files must be generated before `yarn build` using `rake react_on_rails:locale`. - For development, you should adjust your startup scripts (`Procfile`s) so that they run `bundle exec rake react_on_rails:locale` before running any Webpack watch process (`yarn run build:development`). + **Recommended: Use Shakapacker's precompile_hook with bin/dev** (React on Rails 16.2+, Shakapacker 9.3+) - If you are not using the React on Rails test helper, - you may need to configure your CI to run `bundle exec rake react_on_rails:locale` before any Webpack process as well. + The locale generation task is idempotent and can be safely called multiple times. Configure it in Shakapacker's `precompile_hook` and `bin/dev` will handle coordination automatically: + + ```yaml + # config/shakapacker.yml + default: &default + # Run locale generation before webpack compilation + # Safe to run multiple times - will skip if already built + precompile_hook: 'bundle exec rake react_on_rails:locale' + ``` + + With this configuration, `bin/dev` will: + + - Run the precompile hook **once** before starting development processes + - Set `SHAKAPACKER_SKIP_PRECOMPILE_HOOK=true` to prevent duplicate execution + - Pass the environment variable to all spawned processes (Rails, webpack, etc.) + + This eliminates the need for sleep hacks and manual coordination in `Procfile.dev`. See the [Process Managers documentation](./process-managers.md#precompile-hook-integration) for details. + + **Alternative: Manual coordination** + + For development, you can adjust your startup scripts (`Procfile`s) so that they run `bundle exec rake react_on_rails:locale` before running any Webpack watch process (`yarn run build:development`). + + If you are not using the React on Rails test helper, you may need to configure your CI to run `bundle exec rake react_on_rails:locale` before any Webpack process as well. > [!NOTE] > If you try to lint before running tests, and you depend on the test helper to build your locales, linting will fail because the translations won't be built yet. diff --git a/docs/building-features/process-managers.md b/docs/building-features/process-managers.md index 6a32161fe7..84abf8e956 100644 --- a/docs/building-features/process-managers.md +++ b/docs/building-features/process-managers.md @@ -16,9 +16,46 @@ React on Rails includes `bin/dev` which automatically uses Overmind or Foreman: This script will: -1. Try to use Overmind (if installed) -2. Fall back to Foreman (if installed) -3. Show installation instructions if neither is found +1. Run Shakapacker's `precompile_hook` once (if configured in `config/shakapacker.yml`) +2. Set `SHAKAPACKER_SKIP_PRECOMPILE_HOOK=true` to prevent duplicate execution +3. Try to use Overmind (if installed) +4. Fall back to Foreman (if installed) +5. Show installation instructions if neither is found + +### Precompile Hook Integration + +If you have configured a `precompile_hook` in `config/shakapacker.yml`, `bin/dev` will automatically: + +- Execute the hook **once** before starting development processes +- Set the `SHAKAPACKER_SKIP_PRECOMPILE_HOOK` environment variable +- Pass this environment variable to all spawned processes (Rails, webpack, etc.) +- Prevent webpack processes from re-running the hook independently + +**Note:** The `SHAKAPACKER_SKIP_PRECOMPILE_HOOK` environment variable is supported in Shakapacker 9.4.0 and later. If you're using an earlier version, `bin/dev` will display a warning recommending you upgrade to avoid duplicate hook execution. + +This eliminates the need for manual coordination in your `Procfile.dev`. For example: + +**Before (manual coordination with sleep hacks):** + +```procfile +# Procfile.dev +wp-server: sleep 15 && bundle exec rake react_on_rails:locale && bin/shakapacker --watch +``` + +**After (automatic coordination via bin/dev):** + +```procfile +# Procfile.dev +wp-server: bin/shakapacker --watch +``` + +```yaml +# config/shakapacker.yml +default: &default + precompile_hook: 'bundle exec rake react_on_rails:locale' +``` + +See the [i18n documentation](./i18n.md#internationalization) for more details on configuring the precompile hook. ## Installing a Process Manager diff --git a/lib/react_on_rails/dev/server_manager.rb b/lib/react_on_rails/dev/server_manager.rb index 63e8962634..ce4c7c87be 100644 --- a/lib/react_on_rails/dev/server_manager.rb +++ b/lib/react_on_rails/dev/server_manager.rb @@ -8,6 +8,8 @@ module ReactOnRails module Dev class ServerManager + HELP_FLAGS = ["-h", "--help"].freeze + class << self def start(mode = :development, procfile = nil, verbose: false, route: nil, rails_env: nil) case mode @@ -145,10 +147,17 @@ def show_help puts help_troubleshooting end - # rubocop:disable Metrics/AbcSize + # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity def run_from_command_line(args = ARGV) require "optparse" + # Get the command early to check for help/kill before running hooks + # We need to do this before OptionParser processes flags like -h/--help + command = args.find { |arg| !arg.start_with?("--") && !arg.start_with?("-") } + + # Check if help flags are present in args (before OptionParser processes them) + help_requested = args.any? { |arg| HELP_FLAGS.include?(arg) } + options = { route: nil, rails_env: nil, verbose: false } OptionParser.new do |opts| @@ -172,8 +181,15 @@ def run_from_command_line(args = ARGV) end end.parse!(args) - # Get the command (anything that's not parsed as an option) - command = args[0] + # Run precompile hook once before starting any mode (except kill/help) + # Then set environment variable to prevent duplicate execution in spawned processes. + # Note: We always set SHAKAPACKER_SKIP_PRECOMPILE_HOOK=true (even when no hook is configured) + # to provide a consistent signal that bin/dev is managing the precompile lifecycle. + # This allows custom scripts to detect bin/dev's presence and adjust behavior accordingly. + unless %w[kill help].include?(command) || help_requested + run_precompile_hook_if_present + ENV["SHAKAPACKER_SKIP_PRECOMPILE_HOOK"] = "true" + end # Main execution case command @@ -184,7 +200,7 @@ def run_from_command_line(args = ARGV) start(:static, "Procfile.dev-static-assets", verbose: options[:verbose], route: options[:route]) when "kill" kill_processes - when "help", "--help", "-h" + when "help" show_help when "hmr", nil start(:development, "Procfile.dev", verbose: options[:verbose], route: options[:route]) @@ -194,10 +210,86 @@ def run_from_command_line(args = ARGV) exit 1 end end - # rubocop:enable Metrics/AbcSize + # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity private + def run_precompile_hook_if_present + require "open3" + require "shellwords" + + hook_value = PackerUtils.shakapacker_precompile_hook_value + return unless hook_value + + # Warn if Shakapacker version doesn't support SHAKAPACKER_SKIP_PRECOMPILE_HOOK + warn_if_shakapacker_version_too_old + + puts Rainbow("🔧 Running Shakapacker precompile hook...").cyan + puts Rainbow(" Command: #{hook_value}").cyan + puts "" + + # Capture stdout and stderr for better error reporting + # Use Shellwords.split for safer command execution (prevents shell metacharacter interpretation) + command_args = Shellwords.split(hook_value.to_s) + stdout, stderr, status = Open3.capture3(*command_args) + + if status.success? + puts Rainbow("✅ Precompile hook completed successfully").green + puts "" + else + handle_precompile_hook_failure(hook_value, stdout, stderr) + end + end + + # rubocop:disable Metrics/AbcSize + def handle_precompile_hook_failure(hook_value, stdout, stderr) + puts "" + puts Rainbow("❌ Precompile hook failed!").red.bold + puts Rainbow(" Command: #{hook_value}").red + puts "" + + if stdout && !stdout.strip.empty? + puts Rainbow(" Output:").yellow + stdout.strip.split("\n").each { |line| puts Rainbow(" #{line}").yellow } + puts "" + end + + if stderr && !stderr.strip.empty? + puts Rainbow(" Error:").red + stderr.strip.split("\n").each { |line| puts Rainbow(" #{line}").red } + puts "" + end + + puts Rainbow("💡 Fix the hook command in config/shakapacker.yml or remove it to continue").yellow + exit 1 + end + # rubocop:enable Metrics/AbcSize + + # rubocop:disable Metrics/AbcSize + def warn_if_shakapacker_version_too_old + # Only warn for Shakapacker versions in the range 9.0.0 to 9.3.x + # Versions below 9.0.0 don't use the precompile_hook feature + # Versions 9.4.0+ support SHAKAPACKER_SKIP_PRECOMPILE_HOOK environment variable + has_precompile_hook_support = PackerUtils.shakapacker_version_requirement_met?("9.0.0") + has_skip_env_var_support = PackerUtils.shakapacker_version_requirement_met?("9.4.0") + + return unless has_precompile_hook_support + return if has_skip_env_var_support + + puts "" + puts Rainbow("⚠️ Warning: Shakapacker #{PackerUtils.shakapacker_version} detected").yellow.bold + puts "" + puts Rainbow(" The SHAKAPACKER_SKIP_PRECOMPILE_HOOK environment variable is not").yellow + puts Rainbow(" supported in Shakapacker versions below 9.4.0. This may cause the").yellow + puts Rainbow(" precompile_hook to run multiple times (once by bin/dev, and again").yellow + puts Rainbow(" by each webpack process).").yellow + puts "" + puts Rainbow(" Recommendation: Upgrade to Shakapacker 9.4.0 or later:").cyan + puts Rainbow(" bundle update shakapacker").cyan.bold + puts "" + end + # rubocop:enable Metrics/AbcSize + def help_usage Rainbow("📋 Usage: bin/dev [command] [options]").bold end diff --git a/spec/react_on_rails/dev/server_manager_spec.rb b/spec/react_on_rails/dev/server_manager_spec.rb index fd1c6d1845..e88c0ce9eb 100644 --- a/spec/react_on_rails/dev/server_manager_spec.rb +++ b/spec/react_on_rails/dev/server_manager_spec.rb @@ -268,4 +268,176 @@ def mock_system_calls expect { described_class.show_help }.to output(%r{Usage: bin/dev \[command\]}).to_stdout_from_any_process end end + + describe ".run_from_command_line with precompile hook" do + before do + mock_system_calls + # Clear environment variable before each test + ENV.delete("SHAKAPACKER_SKIP_PRECOMPILE_HOOK") + end + + after do + # Clean up environment variable after each test to ensure test isolation + # This ensures cleanup even if tests fail + ENV.delete("SHAKAPACKER_SKIP_PRECOMPILE_HOOK") + end + + context "when precompile hook is configured" do + before do + # Default to a version that supports the skip flag (no warning) + allow(ReactOnRails::PackerUtils).to receive_messages( + shakapacker_precompile_hook_value: "bundle exec rake react_on_rails:locale", shakapacker_version: "9.4.0" + ) + allow(ReactOnRails::PackerUtils).to receive(:shakapacker_version_requirement_met?) + .with("9.0.0").and_return(true) + allow(ReactOnRails::PackerUtils).to receive(:shakapacker_version_requirement_met?) + .with("9.4.0").and_return(true) + end + + it "runs the hook and sets environment variable for development mode" do + status_double = instance_double(Process::Status, success?: true) + expect(Open3).to receive(:capture3) + .with("bundle", "exec", "rake", "react_on_rails:locale") + .and_return(["", "", status_double]) + + described_class.run_from_command_line([]) + + expect(ENV.fetch("SHAKAPACKER_SKIP_PRECOMPILE_HOOK", nil)).to eq("true") + end + + it "runs the hook and sets environment variable for static mode" do + status_double = instance_double(Process::Status, success?: true) + expect(Open3).to receive(:capture3) + .with("bundle", "exec", "rake", "react_on_rails:locale") + .and_return(["", "", status_double]) + + described_class.run_from_command_line(["static"]) + + expect(ENV.fetch("SHAKAPACKER_SKIP_PRECOMPILE_HOOK", nil)).to eq("true") + end + + it "runs the hook and sets environment variable for prod mode" do + env = { "NODE_ENV" => "production" } + argv = ["bundle", "exec", "rails", "assets:precompile"] + assets_status_double = instance_double(Process::Status, success?: true) + hook_status_double = instance_double(Process::Status, success?: true) + + # Expect both Open3.capture3 calls: one for the hook, one for assets:precompile + expect(Open3).to receive(:capture3) + .with("bundle", "exec", "rake", "react_on_rails:locale") + .and_return(["", "", hook_status_double]) + expect(Open3).to receive(:capture3) + .with(env, *argv) + .and_return(["output", "", assets_status_double]) + + described_class.run_from_command_line(["prod"]) + + expect(ENV.fetch("SHAKAPACKER_SKIP_PRECOMPILE_HOOK", nil)).to eq("true") + end + + it "exits when hook fails" do + status_double = instance_double(Process::Status, success?: false) + expect(Open3).to receive(:capture3) + .with("bundle", "exec", "rake", "react_on_rails:locale") + .and_return(["", "", status_double]) + expect_any_instance_of(Kernel).to receive(:exit).with(1) + + described_class.run_from_command_line([]) + end + + it "does not run hook or set environment variable for kill command" do + expect(Open3).not_to receive(:capture3) + + described_class.run_from_command_line(["kill"]) + + expect(ENV.fetch("SHAKAPACKER_SKIP_PRECOMPILE_HOOK", nil)).to be_nil + end + + it "does not run hook or set environment variable for help command" do + expect(Open3).not_to receive(:capture3) + + described_class.run_from_command_line(["help"]) + + expect(ENV.fetch("SHAKAPACKER_SKIP_PRECOMPILE_HOOK", nil)).to be_nil + end + + it "does not run hook or set environment variable for -h flag" do + expect(Open3).not_to receive(:capture3) + + # The -h flag is handled by OptionParser and calls exit during option parsing + # We need to mock exit to prevent the test from actually exiting + allow_any_instance_of(Kernel).to receive(:exit) + + described_class.run_from_command_line(["-h"]) + + expect(ENV.fetch("SHAKAPACKER_SKIP_PRECOMPILE_HOOK", nil)).to be_nil + end + + context "with Shakapacker version below 9.4.0" do + before do + allow(ReactOnRails::PackerUtils).to receive(:shakapacker_version).and_return("9.3.0") + allow(ReactOnRails::PackerUtils).to receive(:shakapacker_version_requirement_met?) + .with("9.0.0").and_return(true) + allow(ReactOnRails::PackerUtils).to receive(:shakapacker_version_requirement_met?) + .with("9.4.0").and_return(false) + end + + it "displays warning about unsupported SHAKAPACKER_SKIP_PRECOMPILE_HOOK" do + status_double = instance_double(Process::Status, success?: true) + expect(Open3).to receive(:capture3) + .with("bundle", "exec", "rake", "react_on_rails:locale") + .and_return(["", "", status_double]) + + expect do + described_class.run_from_command_line([]) + end.to output(/Warning: Shakapacker 9\.3\.0 detected/).to_stdout_from_any_process + + expect(ENV.fetch("SHAKAPACKER_SKIP_PRECOMPILE_HOOK", nil)).to eq("true") + end + end + + context "with Shakapacker version 9.4.0 or later" do + before do + allow(ReactOnRails::PackerUtils).to receive(:shakapacker_version).and_return("9.4.0") + allow(ReactOnRails::PackerUtils).to receive(:shakapacker_version_requirement_met?) + .with("9.0.0").and_return(true) + allow(ReactOnRails::PackerUtils).to receive(:shakapacker_version_requirement_met?) + .with("9.4.0").and_return(true) + end + + it "does not display warning" do + status_double = instance_double(Process::Status, success?: true) + expect(Open3).to receive(:capture3) + .with("bundle", "exec", "rake", "react_on_rails:locale") + .and_return(["", "", status_double]) + + expect do + described_class.run_from_command_line([]) + end.not_to output(/Warning: Shakapacker/).to_stdout_from_any_process + end + end + end + + context "when no precompile hook is configured" do + before do + allow(ReactOnRails::PackerUtils).to receive(:shakapacker_precompile_hook_value).and_return(nil) + end + + it "sets environment variable even when no hook is configured (provides consistent signal)" do + # The environment variable is intentionally set even when no hook exists + # to provide a consistent signal that bin/dev is managing the precompile lifecycle + expect_any_instance_of(Kernel).not_to receive(:system) + + described_class.run_from_command_line([]) + + expect(ENV.fetch("SHAKAPACKER_SKIP_PRECOMPILE_HOOK", nil)).to eq("true") + end + + it "does not set environment variable for kill command" do + described_class.run_from_command_line(["kill"]) + + expect(ENV.fetch("SHAKAPACKER_SKIP_PRECOMPILE_HOOK", nil)).to be_nil + end + end + end end