Skip to content

Commit 75515af

Browse files
justin808claude
justin808
and
claude
authored
[codex] Fix shakapacker config helper binstubs (#1132)
### Summary Fixes #1123. This updates `shakapacker:export_bundler_config` to dispatch existing app binstubs by shebang, so upgraded apps with legacy JavaScript binstubs still run through Node while Ruby binstubs continue through Ruby. It also keeps installed, dummy, and generated helper binstubs in sync so `NODE_ENV` follows `RAILS_ENV`, Node is found without executing it during lookup, and non-`Error` CLI rejections print useful messages. ### Pull Request checklist - [x] Add/update test to cover these changes - [x] ~Update documentation~ - [x] ~Update CHANGELOG file~ ### Other Information Validated with focused RSpec helper/rake specs, Jest config exporter tests, TypeScript type-checking, ESLint, and `git diff --check`. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes how `shakapacker:export_bundler_config` selects and executes binstubs and updates Node discovery / env defaults, which can affect command execution across platforms (notably Windows). Covered by new RSpec/Jest tests, but still impacts developer tooling entrypoints. > > **Overview** > Fixes Shakapacker helper/binstub behavior to better support upgraded apps and Windows/ESM environments. > > The `shakapacker:export_bundler_config` rake task now inspects the `bin/shakapacker-config` shebang and dispatches *legacy JS binstubs via `node`* while continuing to run *Ruby binstubs via the current `RbConfig.ruby`*. > > The installed/generatable Ruby helper binstubs (`shakapacker-config` and `diff-bundler-config`) are updated to (1) locate `node` via explicit PATH/PATHEXT scanning without executing it, (2) default `NODE_ENV` from `RAILS_ENV` (`test`/`development` -> `development`, otherwise `production`), and (3) improve CLI error printing for non-`Error` rejections. New specs ensure binstub sync, correct dispatching, env mapping, and init-generated stub parity. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit c5a536a. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 94392be commit 75515af

12 files changed

Lines changed: 449 additions & 34 deletions

File tree

‎bin/shakapacker-config‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,6 @@ const { run } = require("shakapacker/configExporter")
66
run(process.argv.slice(2))
77
.then((exitCode) => process.exit(exitCode))
88
.catch((error) => {
9-
console.error(error.message)
9+
console.error(error instanceof Error ? error.message : String(error))
1010
process.exit(1)
1111
})

‎lib/install/bin/diff-bundler-config‎

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
#!/usr/bin/env ruby
22
# frozen_string_literal: true
33

4-
# Keep in sync with lib/install/bin/shakapacker-config and the template in
5-
# package/configExporter/cli.ts (createBinStub).
4+
require "rbconfig"
5+
6+
# Keep helper logic in sync across:
7+
# - lib/install/bin/shakapacker-config
8+
# - lib/install/bin/diff-bundler-config
9+
# - spec/dummy/bin/shakapacker-config
10+
# - package/configExporter/cli.ts (createBinStub).
611
def shakapacker_app_root
712
candidate = File.expand_path("..", __dir__)
813
return candidate if File.exist?(File.join(candidate, "Gemfile"))
@@ -12,17 +17,42 @@ def shakapacker_app_root
1217
Dir.pwd
1318
end
1419

20+
def shakapacker_executable_candidates(executable)
21+
extensions = [
22+
RbConfig::CONFIG["EXEEXT"],
23+
*ENV.fetch("PATHEXT", "").split(File::PATH_SEPARATOR)
24+
].compact.reject(&:empty?)
25+
return [executable] if extensions.empty? || File.extname(executable) != ""
26+
27+
([executable] + extensions.map { |extension| "#{executable}#{extension}" }).uniq
28+
end
29+
30+
def shakapacker_find_executable(executable)
31+
ENV.fetch("PATH", "").split(File::PATH_SEPARATOR).each do |path|
32+
shakapacker_executable_candidates(executable).each do |candidate|
33+
executable_path = File.join(path, candidate)
34+
return executable_path if File.file?(executable_path) && File.executable?(executable_path)
35+
end
36+
end
37+
38+
nil
39+
end
40+
1541
def shakapacker_node_binary
16-
node_bin = "node"
17-
return node_bin if system(node_bin, "--version", out: File::NULL, err: File::NULL)
42+
node_bin = shakapacker_find_executable("node")
43+
return node_bin if node_bin
1844

19-
warn "[Shakapacker] Could not find Node.js executable #{node_bin.inspect}. " \
45+
warn '[Shakapacker] Could not find Node.js executable "node". ' \
2046
"Install Node.js and try again."
2147
exit 1
2248
end
2349

50+
def shakapacker_node_env
51+
%w[development test].include?(ENV["RAILS_ENV"]) ? "development" : "production"
52+
end
53+
2454
ENV["RAILS_ENV"] ||= ENV["RACK_ENV"] || "development"
25-
ENV["NODE_ENV"] ||= "development"
55+
ENV["NODE_ENV"] ||= shakapacker_node_env
2656

2757
app_root = shakapacker_app_root
2858
node_bin = shakapacker_node_binary

‎lib/install/bin/shakapacker-config‎

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
#!/usr/bin/env ruby
22
# frozen_string_literal: true
33

4-
# Keep in sync with lib/install/bin/diff-bundler-config and the template in
5-
# package/configExporter/cli.ts (createBinStub).
4+
require "rbconfig"
5+
6+
# Keep helper logic in sync across:
7+
# - lib/install/bin/shakapacker-config
8+
# - lib/install/bin/diff-bundler-config
9+
# - spec/dummy/bin/shakapacker-config
10+
# - package/configExporter/cli.ts (createBinStub).
611
def shakapacker_app_root
712
candidate = File.expand_path("..", __dir__)
813
return candidate if File.exist?(File.join(candidate, "Gemfile"))
@@ -12,17 +17,42 @@ def shakapacker_app_root
1217
Dir.pwd
1318
end
1419

20+
def shakapacker_executable_candidates(executable)
21+
extensions = [
22+
RbConfig::CONFIG["EXEEXT"],
23+
*ENV.fetch("PATHEXT", "").split(File::PATH_SEPARATOR)
24+
].compact.reject(&:empty?)
25+
return [executable] if extensions.empty? || File.extname(executable) != ""
26+
27+
([executable] + extensions.map { |extension| "#{executable}#{extension}" }).uniq
28+
end
29+
30+
def shakapacker_find_executable(executable)
31+
ENV.fetch("PATH", "").split(File::PATH_SEPARATOR).each do |path|
32+
shakapacker_executable_candidates(executable).each do |candidate|
33+
executable_path = File.join(path, candidate)
34+
return executable_path if File.file?(executable_path) && File.executable?(executable_path)
35+
end
36+
end
37+
38+
nil
39+
end
40+
1541
def shakapacker_node_binary
16-
node_bin = "node"
17-
return node_bin if system(node_bin, "--version", out: File::NULL, err: File::NULL)
42+
node_bin = shakapacker_find_executable("node")
43+
return node_bin if node_bin
1844

19-
warn "[Shakapacker] Could not find Node.js executable #{node_bin.inspect}. " \
45+
warn '[Shakapacker] Could not find Node.js executable "node". ' \
2046
"Install Node.js and try again."
2147
exit 1
2248
end
2349

50+
def shakapacker_node_env
51+
%w[development test].include?(ENV["RAILS_ENV"]) ? "development" : "production"
52+
end
53+
2454
ENV["RAILS_ENV"] ||= ENV["RACK_ENV"] || "development"
25-
ENV["NODE_ENV"] ||= "development"
55+
ENV["NODE_ENV"] ||= shakapacker_node_env
2656

2757
app_root = shakapacker_app_root
2858
node_bin = shakapacker_node_binary

‎lib/tasks/shakapacker/export_bundler_config.rake‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,21 @@
11
namespace :shakapacker do
2+
def shakapacker_config_binstub_command(bin_path)
3+
# Read in binary mode so Windows CRLF line endings do not leak \r into the shebang.
4+
shebang = File.open(bin_path, "rb", &:gets).to_s
5+
command = shebang.delete_prefix("#!").strip.split(/\s+/)
6+
executable = File.basename(command.first.to_s)
7+
8+
if executable == "env"
9+
executable = File.basename(command.drop(1).find { |part| !part.start_with?("-") }.to_s)
10+
end
11+
12+
# Legacy JS binstubs are dispatched via PATH lookup; Kernel#exec resolves
13+
# "node" through the shell's PATH. The Ruby binstubs perform their own
14+
# explicit lookup via shakapacker_find_executable, which matters more on
15+
# Windows where PATHEXT is involved.
16+
executable == "node" ? ["node", bin_path.to_s] : [RbConfig.ruby, bin_path.to_s]
17+
end
18+
219
desc <<~DESC
320
Export webpack or rspack configuration for debugging and analysis
421
@@ -54,14 +71,14 @@ namespace :shakapacker do
5471
$stderr.puts ""
5572

5673
Dir.chdir(Rails.root) do
57-
exec(RbConfig.ruby, gem_bin_path, *ARGV[1..])
74+
Kernel.exec(RbConfig.ruby, gem_bin_path, *ARGV[1..])
5875
end
5976
else
6077
# Pass through command-line arguments after the task name.
61-
# Invoke with RbConfig.ruby so the binstub runs under the same Ruby as Rake
62-
# (avoids version-manager/shebang mismatches and works on Windows).
78+
# Ruby binstubs run under the same Ruby as Rake; legacy JavaScript
79+
# binstubs from upgraded apps still need Node until users refresh them.
6380
Dir.chdir(Rails.root) do
64-
exec(RbConfig.ruby, bin_path.to_s, *ARGV[1..])
81+
Kernel.exec(*shakapacker_config_binstub_command(bin_path), *ARGV[1..])
6582
end
6683
end
6784
end

‎package/bin/shakapacker-config.cjs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@ const { run } = require("../configExporter")
55
run(process.argv.slice(2))
66
.then((exitCode) => process.exit(exitCode))
77
.catch((error) => {
8-
console.error(error.message)
8+
console.error(error instanceof Error ? error.message : String(error))
99
process.exit(1)
1010
})

‎package/configExporter/cli.ts‎

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -523,8 +523,13 @@ function createBinStub(binStubPath: string): void {
523523
const stubContent = `#!/usr/bin/env ruby
524524
# frozen_string_literal: true
525525
526-
# Keep in sync with lib/install/bin/shakapacker-config and
527-
# lib/install/bin/diff-bundler-config; update all three when changing helpers.
526+
require "rbconfig"
527+
528+
# Keep helper logic in sync across:
529+
# - lib/install/bin/shakapacker-config
530+
# - lib/install/bin/diff-bundler-config
531+
# - spec/dummy/bin/shakapacker-config
532+
# - package/configExporter/cli.ts (createBinStub).
528533
def shakapacker_app_root
529534
candidate = File.expand_path("..", __dir__)
530535
return candidate if File.exist?(File.join(candidate, "Gemfile"))
@@ -534,17 +539,42 @@ def shakapacker_app_root
534539
Dir.pwd
535540
end
536541
542+
def shakapacker_executable_candidates(executable)
543+
extensions = [
544+
RbConfig::CONFIG["EXEEXT"],
545+
*ENV.fetch("PATHEXT", "").split(File::PATH_SEPARATOR)
546+
].compact.reject(&:empty?)
547+
return [executable] if extensions.empty? || File.extname(executable) != ""
548+
549+
([executable] + extensions.map { |extension| "#{executable}#{extension}" }).uniq
550+
end
551+
552+
def shakapacker_find_executable(executable)
553+
ENV.fetch("PATH", "").split(File::PATH_SEPARATOR).each do |path|
554+
shakapacker_executable_candidates(executable).each do |candidate|
555+
executable_path = File.join(path, candidate)
556+
return executable_path if File.file?(executable_path) && File.executable?(executable_path)
557+
end
558+
end
559+
560+
nil
561+
end
562+
537563
def shakapacker_node_binary
538-
node_bin = "node"
539-
return node_bin if system(node_bin, "--version", out: File::NULL, err: File::NULL)
564+
node_bin = shakapacker_find_executable("node")
565+
return node_bin if node_bin
540566
541-
warn "[Shakapacker] Could not find Node.js executable #{node_bin.inspect}. " \\
567+
warn '[Shakapacker] Could not find Node.js executable "node". ' \\
542568
"Install Node.js and try again."
543569
exit 1
544570
end
545571
572+
def shakapacker_node_env
573+
%w[development test].include?(ENV["RAILS_ENV"]) ? "development" : "production"
574+
end
575+
546576
ENV["RAILS_ENV"] ||= ENV["RACK_ENV"] || "development"
547-
ENV["NODE_ENV"] ||= "development"
577+
ENV["NODE_ENV"] ||= shakapacker_node_env
548578
549579
app_root = shakapacker_app_root
550580
node_bin = shakapacker_node_binary

‎spec/dummy/bin/shakapacker-config‎

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
#!/usr/bin/env ruby
22
# frozen_string_literal: true
33

4-
# Keep in sync with lib/install/bin/shakapacker-config and the template in
5-
# package/configExporter/cli.ts (createBinStub).
4+
require "rbconfig"
5+
6+
# Keep helper logic in sync across:
7+
# - lib/install/bin/shakapacker-config
8+
# - lib/install/bin/diff-bundler-config
9+
# - spec/dummy/bin/shakapacker-config
10+
# - package/configExporter/cli.ts (createBinStub).
611
def shakapacker_app_root
712
candidate = File.expand_path("..", __dir__)
813
return candidate if File.exist?(File.join(candidate, "Gemfile"))
@@ -12,17 +17,42 @@ def shakapacker_app_root
1217
Dir.pwd
1318
end
1419

20+
def shakapacker_executable_candidates(executable)
21+
extensions = [
22+
RbConfig::CONFIG["EXEEXT"],
23+
*ENV.fetch("PATHEXT", "").split(File::PATH_SEPARATOR)
24+
].compact.reject(&:empty?)
25+
return [executable] if extensions.empty? || File.extname(executable) != ""
26+
27+
([executable] + extensions.map { |extension| "#{executable}#{extension}" }).uniq
28+
end
29+
30+
def shakapacker_find_executable(executable)
31+
ENV.fetch("PATH", "").split(File::PATH_SEPARATOR).each do |path|
32+
shakapacker_executable_candidates(executable).each do |candidate|
33+
executable_path = File.join(path, candidate)
34+
return executable_path if File.file?(executable_path) && File.executable?(executable_path)
35+
end
36+
end
37+
38+
nil
39+
end
40+
1541
def shakapacker_node_binary
16-
node_bin = "node"
17-
return node_bin if system(node_bin, "--version", out: File::NULL, err: File::NULL)
42+
node_bin = shakapacker_find_executable("node")
43+
return node_bin if node_bin
1844

19-
warn "[Shakapacker] Could not find Node.js executable #{node_bin.inspect}. " \
45+
warn '[Shakapacker] Could not find Node.js executable "node". ' \
2046
"Install Node.js and try again."
2147
exit 1
2248
end
2349

50+
def shakapacker_node_env
51+
%w[development test].include?(ENV["RAILS_ENV"]) ? "development" : "production"
52+
end
53+
2454
ENV["RAILS_ENV"] ||= ENV["RACK_ENV"] || "development"
25-
ENV["NODE_ENV"] ||= "development"
55+
ENV["NODE_ENV"] ||= shakapacker_node_env
2656

2757
app_root = shakapacker_app_root
2858
node_bin = shakapacker_node_binary

‎spec/shakapacker/binstub_sync_spec.rb‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
end
3838
end
3939

40-
it "documents every divergent binstub and only divergent binstubs" do
40+
it "all documented divergent binstubs still exist in both directories" do
4141
install_basenames = Dir.glob(File.join(gem_root, "lib/install/bin/*")).map { |p| File.basename(p) }
4242
bin_basenames = Dir.glob(File.join(gem_root, "bin/*")).map { |p| File.basename(p) }
4343
actually_shared = install_basenames & bin_basenames
@@ -48,4 +48,27 @@
4848
"but those files are no longer present in both bin/ and lib/install/bin/. " \
4949
"Remove them from the list."
5050
end
51+
52+
it "spec/dummy/bin/shakapacker-config matches lib/install/bin/shakapacker-config" do
53+
install_content = File.read(File.join(gem_root, "lib", "install", "bin", "shakapacker-config"))
54+
dummy_content = File.read(File.join(gem_root, "spec", "dummy", "bin", "shakapacker-config"))
55+
56+
expect(dummy_content).to eq(install_content),
57+
"spec/dummy/bin/shakapacker-config and lib/install/bin/shakapacker-config have diverged. " \
58+
"Update both files to keep them in sync."
59+
end
60+
61+
# lib/install/bin/diff-bundler-config and lib/install/bin/shakapacker-config share
62+
# the same helper functions and only legitimately diverge on the .cjs script name
63+
# they dispatch to. Normalizing that one line catches any other drift between them.
64+
it "lib/install/bin/diff-bundler-config stays in sync with lib/install/bin/shakapacker-config" do
65+
shakapacker_config = File.read(File.join(gem_root, "lib", "install", "bin", "shakapacker-config"))
66+
diff_bundler_config = File.read(File.join(gem_root, "lib", "install", "bin", "diff-bundler-config"))
67+
68+
normalized = diff_bundler_config.sub('"diff-bundler-config.cjs"', '"shakapacker-config.cjs"')
69+
70+
expect(normalized).to eq(shakapacker_config),
71+
"lib/install/bin/diff-bundler-config and lib/install/bin/shakapacker-config have diverged " \
72+
"beyond the intentional .cjs script name difference. Update both files to keep them in sync."
73+
end
5174
end

0 commit comments

Comments
 (0)