Skip to content

Commit 5de8924

Browse files
committed
Delegate watch binstub ownership to Shakapacker
1 parent 82cfd43 commit 5de8924

11 files changed

Lines changed: 151 additions & 103 deletions

File tree

CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,14 @@ After a release, run `/update-changelog` in Claude Code to analyze commits, writ
3737

3838
#### Fixed
3939

40+
- **Stopped replacing Shakapacker-owned watch binstubs during installation**: React on Rails no longer ships
41+
its own `bin/shakapacker-watch` template. Generated Procfiles use Shakapacker's optional watch binstub when
42+
it is present and fall back to `bin/shakapacker --watch` for older supported Shakapacker installations.
43+
Existing Shakapacker-provided or customized watch binstubs remain untouched, including under `--force`. Fixes
44+
[Issue 4617](https://github.com/shakacode/react_on_rails/issues/4617).
45+
[PR 4715](https://github.com/shakacode/react_on_rails/pull/4715) by
46+
[ihabadham](https://github.com/ihabadham).
47+
4048
- **Server rendering no longer crashes on or corrupts lone UTF-16 surrogates**: When the JavaScript
4149
renderer emits a string containing a lone surrogate (commonly from truncating text mid-emoji, e.g. an
4250
excerpt cut with `slice`/`substring`), `JSON.stringify` serializes it as a `\uXXXX` escape that Ruby's

react_on_rails/lib/generators/react_on_rails/base_generator.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -251,9 +251,7 @@ def copy_base_files
251251
ensure_new_app_root_route_initialized
252252

253253
base_path = "base/base/"
254-
base_files = %w[Procfile.dev
255-
Procfile.dev-static-assets
256-
Procfile.dev-prod-assets
254+
base_files = %w[Procfile.dev-prod-assets
257255
.dev-services.yml.example
258256
.env.example
259257
bin/shakapacker-precompile-hook]
@@ -262,7 +260,9 @@ def copy_base_files
262260
# Exception: Redux still needs the HelloWorld controller even with RSC
263261
base_files << "app/controllers/hello_world_controller.rb" unless use_rsc? && !options.redux?
264262
base_files << "app/controllers/home_controller.rb" if generate_new_app_home_page?
265-
base_templates = %w[config/initializers/react_on_rails.rb]
263+
base_templates = %w[Procfile.dev
264+
Procfile.dev-static-assets
265+
config/initializers/react_on_rails.rb]
266266
base_files.each { |file| copy_file("#{base_path}#{file}", file) }
267267
copy_react_on_rails_default_layout(base_path)
268268
warn_existing_hello_world_tailwind_layout

react_on_rails/lib/generators/react_on_rails/generator_helper.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,16 @@ def detect_react_version
492492
nil
493493
end
494494

495+
# Prefer Shakapacker's optional watcher binstub when the application has it. Older
496+
# supported Shakapacker installations can still run watch mode through the required
497+
# bin/shakapacker binstub, so React on Rails does not need to vendor a fallback watcher.
498+
def shakapacker_watch_command
499+
watch_binstub = File.join(destination_root, "bin/shakapacker-watch")
500+
executable = File.exist?(watch_binstub) ? "bin/shakapacker-watch" : "bin/shakapacker"
501+
502+
"#{executable} --watch"
503+
end
504+
495505
# Check if Shakapacker 9.0 or higher is available
496506
# Returns true if Shakapacker >= 9.0, false otherwise
497507
#

react_on_rails/lib/generators/react_on_rails/install_generator.rb

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -675,15 +675,10 @@ def add_bin_scripts
675675

676676
# Copy bin scripts from templates
677677
template_bin_path = "#{__dir__}/templates/base/base/bin"
678-
shakapacker_watch_path = File.join(destination_root, "bin/shakapacker-watch")
679-
preserve_existing_shakapacker_watch = File.exist?(shakapacker_watch_path)
680-
# Always exclude these scripts from the bulk copy. `dev` needs app-specific patches,
681-
# while Shakapacker owns its watch binstub when it provides one.
682-
directory_options = { exclude_pattern: %r{/(?:dev|shakapacker-watch)(?:\.tt)?\z} }
678+
# Always exclude `dev` from the bulk copy; it is handled explicitly below
679+
# so we can patch DEFAULT_ROUTE and AUTO_OPEN_BROWSER_ONCE after copying.
680+
directory_options = { exclude_pattern: %r{/dev(?:\.tt)?\z} }
683681
directory template_bin_path, "bin", directory_options
684-
unless preserve_existing_shakapacker_watch
685-
copy_file("#{template_bin_path}/shakapacker-watch", "bin/shakapacker-watch")
686-
end
687682

688683
if preserve_existing_bin_dev?
689684
if use_rsc? && !options.redux? && !options.new_app?
@@ -706,10 +701,7 @@ def add_bin_scripts
706701

707702
# Make these and only these files executable. Use destination_root so
708703
# chmod remains correct even if an earlier generator step changed Dir.pwd.
709-
files_to_become_executable = bin_scripts_to_chmod(
710-
template_bin_path,
711-
preserve_existing_shakapacker_watch:
712-
)
704+
files_to_become_executable = bin_scripts_to_chmod(template_bin_path)
713705
File.chmod(0o755, *files_to_become_executable)
714706
end
715707

@@ -774,10 +766,8 @@ def preserve_existing_bin_dev?
774766
!!@preserve_existing_bin_dev
775767
end
776768

777-
def bin_scripts_to_chmod(template_bin_path, preserve_existing_shakapacker_watch:)
778-
files = Dir.children(template_bin_path).reject do |filename|
779-
filename == "dev" || (filename == "shakapacker-watch" && preserve_existing_shakapacker_watch)
780-
end
769+
def bin_scripts_to_chmod(template_bin_path)
770+
files = Dir.children(template_bin_path).reject { |filename| filename == "dev" }
781771
files << "dev" unless preserve_existing_bin_dev?
782772
files.map { |filename| File.join(destination_root, "bin/#{filename}") }
783773
end

react_on_rails/lib/generators/react_on_rails/rsc_setup.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ def add_rsc_to_procfile
183183
⚠️ Procfile.dev not found. Skipping RSC bundle watcher addition.
184184
185185
You'll need to add the RSC bundle watcher to your process manager manually:
186-
rsc-bundle: RSC_BUNDLE_ONLY=true bin/shakapacker-watch --watch
186+
rsc-bundle: RSC_BUNDLE_ONLY=true #{shakapacker_watch_command}
187187
MSG
188188
return
189189
end
@@ -198,7 +198,7 @@ def add_rsc_to_procfile
198198
rsc_watcher_line = <<~PROCFILE
199199
200200
# React on Rails Pro - RSC bundle watcher
201-
rsc-bundle: RSC_BUNDLE_ONLY=true bin/shakapacker-watch --watch
201+
rsc-bundle: RSC_BUNDLE_ONLY=true #{shakapacker_watch_command}
202202
PROCFILE
203203

204204
append_to_file("Procfile.dev", rsc_watcher_line)

react_on_rails/lib/generators/react_on_rails/templates/base/base/Procfile.dev-static-assets renamed to react_on_rails/lib/generators/react_on_rails/templates/base/base/Procfile.dev-static-assets.tt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
web: bin/rails server -p ${PORT:-3000}
2-
js: bin/shakapacker-watch --watch
2+
js: <%= shakapacker_watch_command %>

react_on_rails/lib/generators/react_on_rails/templates/base/base/Procfile.dev renamed to react_on_rails/lib/generators/react_on_rails/templates/base/base/Procfile.dev.tt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,4 @@
77
# For manual worktree setup (no base-port tool), see .env.example.
88
rails: bundle exec rails s -p ${PORT:-3000}
99
dev-server: bin/shakapacker-dev-server
10-
server-bundle: SERVER_BUNDLE_ONLY=true bin/shakapacker-watch --watch
10+
server-bundle: SERVER_BUNDLE_ONLY=true <%= shakapacker_watch_command %>

react_on_rails/lib/generators/react_on_rails/templates/base/base/bin/shakapacker-watch

Lines changed: 0 additions & 26 deletions
This file was deleted.

react_on_rails/spec/dummy/bin/shakapacker-watch

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,71 @@
11
#!/usr/bin/env sh
22
set -eu
33

4+
show_help() {
5+
cat <<'HELP'
6+
Usage: bin/shakapacker-watch [OPTIONS]
7+
8+
Signal-safe wrapper around bin/shakapacker for Procfile-based workflows
9+
(foreman, overmind, bin/dev). Traps INT/TERM signals and forwards TERM
10+
to the child process, preventing Ruby interrupt backtraces on Ctrl-C.
11+
12+
All options are passed through to bin/shakapacker. Common options:
13+
14+
--watch, -w Watch for file changes and rebuild
15+
--progress Show build progress
16+
--help, -h Show this help
17+
--version, -v Show version (from bin/shakapacker)
18+
19+
Examples:
20+
bin/shakapacker-watch --watch # Procfile watcher
21+
bin/shakapacker-watch --watch --progress # Procfile watcher with progress
22+
23+
Procfile example:
24+
web: bin/rails s
25+
js: bin/shakapacker-watch --watch
26+
HELP
27+
exit 0
28+
}
29+
30+
case "${1:-}" in
31+
-h|--help) show_help ;;
32+
esac
33+
434
child_pid=""
35+
shutdown_requested=""
36+
script_dir=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)
537

638
cleanup() {
7-
if [ -n "${child_pid}" ] && kill -0 "$child_pid" 2>/dev/null; then
39+
shutdown_requested=1
40+
if [ -z "${child_pid}" ]; then
41+
return
42+
fi
43+
44+
if kill -0 "$child_pid" 2>/dev/null; then
845
kill -TERM "$child_pid" 2>/dev/null || true
46+
i=0
47+
while [ "$i" -lt 50 ] && kill -0 "$child_pid" 2>/dev/null; do
48+
sleep 0.1
49+
i=$((i + 1))
50+
done
51+
52+
if kill -0 "$child_pid" 2>/dev/null; then
53+
kill -KILL "$child_pid" 2>/dev/null || true
54+
fi
55+
956
wait "$child_pid" 2>/dev/null || true
1057
fi
1158

1259
exit 0
1360
}
1461

15-
bin/shakapacker "$@" &
16-
child_pid=$!
1762
trap cleanup INT TERM
63+
"$script_dir/shakapacker" "$@" &
64+
child_pid=$!
65+
66+
if [ -n "$shutdown_requested" ]; then
67+
cleanup
68+
fi
1869

1970
if wait "$child_pid"; then
2071
status=0

react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb

Lines changed: 37 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -461,18 +461,15 @@ def run_add_agent_files(options = {})
461461
end
462462
end
463463

464-
it "creates the shakapacker watch wrapper and uses it in Procfiles" do
465-
assert_file "bin/shakapacker-watch" do |content|
466-
expect(content).to include('bin/shakapacker "$@" &')
467-
expect(content).to include("trap cleanup INT TERM")
468-
end
464+
it "uses the standard Shakapacker command when the optional watch binstub is absent" do
465+
assert_no_file "bin/shakapacker-watch"
469466

470467
assert_file "Procfile.dev" do |content|
471-
expect(content).to include("server-bundle: SERVER_BUNDLE_ONLY=true bin/shakapacker-watch --watch")
468+
expect(content).to include("server-bundle: SERVER_BUNDLE_ONLY=true bin/shakapacker --watch")
472469
end
473470

474471
assert_file "Procfile.dev-static-assets" do |content|
475-
expect(content).to include("js: bin/shakapacker-watch --watch")
472+
expect(content).to include("js: bin/shakapacker --watch")
476473
end
477474
end
478475

@@ -513,6 +510,38 @@ def run_add_agent_files(options = {})
513510
end
514511
end
515512

513+
context "when the Shakapacker watch binstub is already installed" do
514+
shakapacker_watch = <<~RUBY
515+
#!/usr/bin/env ruby
516+
puts "Shakapacker-owned watcher"
517+
RUBY
518+
519+
before(:all) do
520+
run_generator_test_with_args(%w[], package_json: true) do
521+
simulate_preinstalled_shakapacker(source_path: "app/javascript", source_entry_path: "packs")
522+
simulate_existing_file("bin/shakapacker-watch", shakapacker_watch)
523+
File.chmod(0o640, File.join(destination_root, "bin/shakapacker-watch"))
524+
end
525+
end
526+
527+
it "preserves Shakapacker's binstub under --force" do
528+
assert_file "bin/shakapacker-watch" do |content|
529+
expect(content).to eq(shakapacker_watch)
530+
end
531+
expect(File.stat(File.join(destination_root, "bin/shakapacker-watch")).mode & 0o777).to eq(0o640)
532+
end
533+
534+
it "uses Shakapacker's binstub in generated Procfiles" do
535+
assert_file "Procfile.dev" do |content|
536+
expect(content).to include("server-bundle: SERVER_BUNDLE_ONLY=true bin/shakapacker-watch --watch")
537+
end
538+
539+
assert_file "Procfile.dev-static-assets" do |content|
540+
expect(content).to include("js: bin/shakapacker-watch --watch")
541+
end
542+
end
543+
end
544+
516545
context "with a pre-installed custom Shakapacker source root" do
517546
before(:all) do
518547
run_generator_test_with_args(%w[], package_json: true, force: false) do
@@ -3580,7 +3609,7 @@ class ActiveSupport::TestCase
35803609
assert_file "Procfile.dev" do |content|
35813610
expect(content).to include("RSC_BUNDLE_ONLY=true")
35823611
expect(content).to include("rsc-bundle:")
3583-
expect(content).to include("bin/shakapacker-watch --watch")
3612+
expect(content).to include("bin/shakapacker --watch")
35843613
end
35853614
end
35863615

@@ -4760,12 +4789,7 @@ class ActiveSupport::TestCase
47604789

47614790
it "does not chmod copied bin scripts in pretend mode" do
47624791
allow(install_generator).to receive(:directory)
4763-
allow(install_generator).to receive(:copy_file).and_call_original
47644792
allow(install_generator).to receive(:use_rsc?).and_return(false)
4765-
shakapacker_watch_template = File.expand_path(
4766-
"../../../lib/generators/react_on_rails/templates/base/base/bin/shakapacker-watch",
4767-
__dir__
4768-
)
47694793

47704794
expect(install_generator).to receive(:say_status)
47714795
.with(:gsub, "bin/dev", true)
@@ -4776,9 +4800,6 @@ class ActiveSupport::TestCase
47764800
expect(File).not_to receive(:chmod)
47774801

47784802
install_generator.send(:add_bin_scripts)
4779-
4780-
expect(install_generator).to have_received(:copy_file)
4781-
.with(shakapacker_watch_template, "bin/shakapacker-watch")
47824803
end
47834804

47844805
it "does not install typescript dependencies in pretend mode" do
@@ -6327,39 +6348,6 @@ class ActiveSupport::TestCase
63276348
expect(File.stat(shakapacker_watch_path).mode & 0o777).to eq(0o640)
63286349
end
63296350

6330-
it "preserves an existing Shakapacker watch binstub when run with --force" do
6331-
shakapacker_watch = <<~RUBY
6332-
#!/usr/bin/env ruby
6333-
puts "Customized watcher"
6334-
RUBY
6335-
shakapacker_watch_path = File.join(destination_root, "bin/shakapacker-watch")
6336-
force_generator = described_class.new([], { force: true }, destination_root:)
6337-
simulate_existing_file("bin/shakapacker-watch", shakapacker_watch)
6338-
File.chmod(0o640, shakapacker_watch_path)
6339-
6340-
Dir.chdir(destination_root) do
6341-
force_generator.send(:add_bin_scripts)
6342-
end
6343-
6344-
expect(File.read(shakapacker_watch_path)).to eq(shakapacker_watch)
6345-
expect(File.stat(shakapacker_watch_path).mode & 0o777).to eq(0o640)
6346-
end
6347-
6348-
it "installs the React on Rails Shakapacker watch fallback when the binstub is absent" do
6349-
template_path = File.expand_path(
6350-
"../../../lib/generators/react_on_rails/templates/base/base/bin/shakapacker-watch",
6351-
__dir__
6352-
)
6353-
shakapacker_watch_path = File.join(destination_root, "bin/shakapacker-watch")
6354-
6355-
Dir.chdir(destination_root) do
6356-
install_generator.send(:add_bin_scripts)
6357-
end
6358-
6359-
expect(File.read(shakapacker_watch_path)).to eq(File.read(template_path))
6360-
expect(File.stat(shakapacker_watch_path).mode & 0o777).to eq(0o755)
6361-
end
6362-
63636351
it "detects custom bin/dev files" do
63646352
simulate_existing_file("bin/dev", "#!/usr/bin/env ruby\nputs 'custom'\n")
63656353

0 commit comments

Comments
 (0)