Skip to content

Commit bafac33

Browse files
justin808claude
andcommitted
Replace fragile regex validation with Shakapacker 9.0+ integration
Addresses code review feedback about fragile regex matching by implementing a proper integration with Shakapacker 9.0+ private_output_path. **Key Changes:** **1. Auto-Detection from Shakapacker (configuration.rb)** - New `auto_detect_server_bundle_path_from_shakapacker` method - Automatically reads `private_output_path` from Shakapacker 9.0+ config - Only applies if user hasn't explicitly set `server_bundle_output_path` - Gracefully falls back to default if detection fails - Logs info message when auto-detection succeeds **2. Removed Fragile Regex Validation (doctor.rb)** - Removed `validate_server_bundle_path_sync` method (regex matching) - Removed `extract_webpack_output_path` method (pattern detection) - Removed `normalize_path` method (no longer needed) - Replaced with `check_shakapacker_private_output_path` method **3. Recommendation-Based Doctor Checks (doctor.rb)** - Detects Shakapacker version and capabilities - Pre-9.0: Recommends upgrading for better DX - 9.0+ without config: Shows how to configure private_output_path - 9.0+ with config matching: Success message - 9.0+ with config mismatch: Warning with fix instructions - No Shakapacker: Informs about manual configuration **4. Updated Generator Templates** - **React on Rails initializer**: Documents Shakapacker 9.0+ approach first - **Webpack config**: Shows config.privateOutputPath pattern - Both templates emphasize single source of truth in shakapacker.yml - Clear migration path for older Shakapacker versions **5. Comprehensive Test Coverage (8 new tests)** - Shakapacker not defined scenario - Pre-9.0 Shakapacker (no private_output_path support) - 9.0+ with matching config - 9.0+ with mismatched config - 9.0+ without config - Error handling - All tests passing **Benefits:** - No fragile regex parsing of webpack configs - Single source of truth in shakapacker.yml - Automatic configuration for Shakapacker 9.0+ users - Backward compatible with older Shakapacker versions - Clear upgrade path and recommendations - Robust error handling **Breaking Changes:** None - Existing configurations continue to work - Auto-detection only applies to default values - Explicit user configuration always takes precedence Addresses: @justin's feedback on PR #1967 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent dd0d5ba commit bafac33

File tree

5 files changed

+165
-234
lines changed

5 files changed

+165
-234
lines changed

lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.tt

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,19 @@ ReactOnRails.configure do |config|
4343
#
4444
config.server_bundle_js_file = "server-bundle.js"
4545

46-
# ⚠️ IMPORTANT: This must match output.path in config/webpack/serverWebpackConfig.js
46+
# ⚠️ RECOMMENDED: Use Shakapacker 9.0+ private_output_path instead
4747
#
48-
# Both are currently set to 'ssr-generated' (relative to Rails.root)
49-
# Keeping these in sync ensures React on Rails can find the server bundle at runtime.
48+
# If using Shakapacker 9.0+, add to config/shakapacker.yml:
49+
# private_output_path: ssr-generated
5050
#
51-
# Run 'rails react_on_rails:doctor' to verify these configs are in sync.
51+
# React on Rails will auto-detect this value, eliminating the need to set it here.
52+
# This keeps your webpack and Rails configs in sync automatically.
5253
#
53-
# Configure where server bundles are output. Defaults to "ssr-generated".
54-
# This path is relative to Rails.root and should point to a private directory
55-
# (outside of public/) for security.
56-
config.server_bundle_output_path = "ssr-generated"
54+
# For older Shakapacker versions or custom setups, manually configure:
55+
# config.server_bundle_output_path = "ssr-generated"
56+
#
57+
# The path is relative to Rails.root and should point to a private directory
58+
# (outside of public/) for security. Run 'rails react_on_rails:doctor' to verify.
5759

5860
# Enforce that server bundles are only loaded from private (non-public) directories.
5961
# When true, server bundles will only be loaded from the configured server_bundle_output_path.

lib/generators/react_on_rails/templates/base/base/config/webpack/serverWebpackConfig.js.tt

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,20 @@ const configureServer = () => {
4545
serverWebpackConfig.plugins.unshift(new bundler.optimize.LimitChunkCountPlugin({ maxChunks: 1 }));
4646

4747
// Custom output for the server-bundle
48-
// ⚠️ IMPORTANT: This output.path must match server_bundle_output_path in
49-
// config/initializers/react_on_rails.rb
5048
//
51-
// Both are currently set to 'ssr-generated' (relative to Rails.root)
52-
// Keeping these in sync ensures React on Rails can find the server bundle at runtime.
49+
// ⚠️ RECOMMENDED: Use Shakapacker 9.0+ for automatic configuration
5350
//
54-
// Server bundles are output to a private directory (not public) for security.
55-
// This prevents server-side code from being exposed via the web server.
51+
// With Shakapacker 9.0+, use config.privateOutputPath to automatically sync
52+
// with shakapacker.yml private_output_path. This eliminates manual path configuration.
53+
//
54+
// For Shakapacker 9.0+:
55+
// serverWebpackConfig.output = {
56+
// filename: 'server-bundle.js',
57+
// globalObject: 'this',
58+
// path: config.privateOutputPath,
59+
// };
60+
//
61+
// For older Shakapacker or custom setups, use hardcoded path:
5662
serverWebpackConfig.output = {
5763
filename: 'server-bundle.js',
5864
globalObject: 'this',

lib/react_on_rails/configuration.rb

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ def setup_config_values
142142
check_component_registry_timeout
143143
validate_generated_component_packs_loading_strategy
144144
validate_enforce_private_server_bundles
145+
auto_detect_server_bundle_path_from_shakapacker
145146
end
146147

147148
private
@@ -214,6 +215,35 @@ def validate_enforce_private_server_bundles
214215
"the public directory. Please set it to a directory outside of public."
215216
end
216217

218+
# Auto-detect server_bundle_output_path from Shakapacker 9.0+ private_output_path
219+
# Only sets if user hasn't explicitly configured server_bundle_output_path
220+
def auto_detect_server_bundle_path_from_shakapacker
221+
# Skip if user explicitly set server_bundle_output_path to something other than default
222+
return if server_bundle_output_path != "ssr-generated"
223+
224+
# Skip if Shakapacker is not available
225+
return unless defined?(::Shakapacker)
226+
227+
# Check if Shakapacker config has private_output_path method (9.0+)
228+
return unless ::Shakapacker.config.respond_to?(:private_output_path)
229+
230+
begin
231+
private_path = ::Shakapacker.config.private_output_path
232+
return unless private_path
233+
234+
# Convert from Pathname to relative string path
235+
relative_path = private_path.to_s.sub("#{Rails.root}/", "")
236+
self.server_bundle_output_path = relative_path
237+
238+
Rails.logger.info("ReactOnRails: Auto-detected server_bundle_output_path from " \
239+
"shakapacker.yml private_output_path: '#{relative_path}'")
240+
rescue StandardError => e
241+
# Fail gracefully - if auto-detection fails, keep the default
242+
Rails.logger.debug("ReactOnRails: Could not auto-detect server bundle path from " \
243+
"Shakapacker: #{e.message}")
244+
end
245+
end
246+
217247
def check_minimum_shakapacker_version
218248
ReactOnRails::PackerUtils.raise_shakapacker_version_incompatible_for_basic_pack_generation unless
219249
ReactOnRails::PackerUtils.supports_basic_pack_generation?

lib/react_on_rails/doctor.rb

Lines changed: 48 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -685,8 +685,8 @@ def analyze_server_rendering_config(content)
685685
enforce_private_match = content.match(/config\.enforce_private_server_bundles\s*=\s*([^\s\n,]+)/)
686686
checker.add_info(" enforce_private_server_bundles: #{enforce_private_match[1]}") if enforce_private_match
687687

688-
# Validate webpack config matches Rails config
689-
validate_server_bundle_path_sync(rails_bundle_path)
688+
# Check Shakapacker integration and provide recommendations
689+
check_shakapacker_private_output_path(rails_bundle_path)
690690

691691
# RSC bundle file (Pro feature)
692692
rsc_bundle_match = content.match(/config\.rsc_bundle_js_file\s*=\s*["']([^"']+)["']/)
@@ -1158,91 +1158,69 @@ def safe_display_config_value(label, config, method_name)
11581158
end
11591159
end
11601160

1161-
# Validates that webpack serverWebpackConfig.js output.path matches
1162-
# React on Rails config.server_bundle_output_path
1163-
def validate_server_bundle_path_sync(rails_bundle_path)
1164-
webpack_config_path = "config/webpack/serverWebpackConfig.js"
1165-
1166-
unless File.exist?(webpack_config_path)
1167-
checker.add_info("\n ℹ️ Webpack server config not found - skipping path validation")
1161+
# Check Shakapacker private_output_path integration and provide recommendations
1162+
# rubocop:disable Metrics/MethodLength
1163+
def check_shakapacker_private_output_path(rails_bundle_path)
1164+
unless defined?(::Shakapacker)
1165+
checker.add_info("\n ℹ️ Shakapacker not detected - using manual configuration")
11681166
return
11691167
end
11701168

1171-
begin
1172-
webpack_content = File.read(webpack_config_path)
1169+
# Check if Shakapacker 9.0+ with private_output_path support
1170+
unless ::Shakapacker.config.respond_to?(:private_output_path)
1171+
checker.add_info(<<~MSG.strip)
1172+
\n 💡 Recommendation: Upgrade to Shakapacker 9.0+
11731173
1174-
# Try to extract the path from webpack config
1175-
webpack_bundle_path = extract_webpack_output_path(webpack_content, webpack_config_path)
1174+
Shakapacker 9.0+ adds 'private_output_path' in shakapacker.yml for server bundles.
1175+
This eliminates the need to configure server_bundle_output_path separately.
11761176
1177-
return unless webpack_bundle_path
1177+
Benefits:
1178+
- Single source of truth in shakapacker.yml
1179+
- Automatic detection by React on Rails
1180+
- No configuration duplication
1181+
MSG
1182+
return
1183+
end
11781184

1179-
# Normalize and compare paths
1180-
normalized_webpack_path = normalize_path(webpack_bundle_path)
1181-
normalized_rails_path = normalize_path(rails_bundle_path)
1185+
# Shakapacker 9.0+ is available
1186+
begin
1187+
private_path = ::Shakapacker.config.private_output_path
11821188

1183-
if normalized_webpack_path == normalized_rails_path
1184-
checker.add_success("\n ✅ Webpack and Rails configs are in sync (both use '#{rails_bundle_path}')")
1185-
else
1186-
checker.add_warning(<<~MSG.strip)
1187-
\n ⚠️ Configuration mismatch detected!
1189+
if private_path
1190+
relative_path = private_path.to_s.sub("#{Rails.root}/", "")
11881191

1189-
React on Rails config (config/initializers/react_on_rails.rb):
1190-
server_bundle_output_path = "#{rails_bundle_path}"
1192+
if relative_path == rails_bundle_path
1193+
checker.add_success("\n ✅ Using Shakapacker 9.0+ private_output_path: '#{relative_path}'")
1194+
checker.add_info(" Auto-detected from shakapacker.yml - no manual config needed")
1195+
else
1196+
checker.add_warning(<<~MSG.strip)
1197+
\n ⚠️ Configuration mismatch detected!
11911198
1192-
Webpack config (#{webpack_config_path}):
1193-
output.path = "#{webpack_bundle_path}" (relative to Rails.root)
1199+
Shakapacker private_output_path: '#{relative_path}'
1200+
React on Rails server_bundle_output_path: '#{rails_bundle_path}'
11941201
1195-
These must match for server rendering to work correctly.
1202+
Recommendation: Remove server_bundle_output_path from your React on Rails
1203+
initializer and let it auto-detect from shakapacker.yml private_output_path.
1204+
MSG
1205+
end
1206+
else
1207+
checker.add_info(<<~MSG.strip)
1208+
\n 💡 Recommendation: Configure private_output_path in shakapacker.yml
11961209
1197-
To fix:
1198-
1. Update server_bundle_output_path in config/initializers/react_on_rails.rb, OR
1199-
2. Update output.path in #{webpack_config_path}
1210+
Add to config/shakapacker.yml:
1211+
private_output_path: #{rails_bundle_path}
12001212
1201-
Make sure both point to the same directory relative to Rails.root.
1213+
This will:
1214+
- Keep webpack and Rails configs in sync automatically
1215+
- Enable auto-detection by React on Rails
1216+
- Serve as single source of truth for server bundle location
12021217
MSG
12031218
end
12041219
rescue StandardError => e
1205-
checker.add_info("\n ℹ️ Could not validate webpack config: #{e.message}")
1206-
end
1207-
end
1208-
1209-
# Extract output.path from webpack config, supporting multiple patterns
1210-
def extract_webpack_output_path(webpack_content, _webpack_config_path)
1211-
# Pattern 1: path: require('path').resolve(__dirname, '../../ssr-generated')
1212-
# Explicitly match ../../path pattern for clarity
1213-
hardcoded_pattern = %r{path:\s*require\(['"]path['"]\)\.resolve\(__dirname,\s*['"]\.\./\.\./([^'"]+)['"]\)}
1214-
if (match = webpack_content.match(hardcoded_pattern))
1215-
return match[1]
1216-
end
1217-
1218-
# Pattern 2: path: config.outputPath (can't validate - runtime value)
1219-
if webpack_content.match?(/path:\s*config\.outputPath/)
1220-
checker.add_info(<<~MSG.strip)
1221-
\n ℹ️ Webpack config uses config.outputPath (from shakapacker.yml)
1222-
Cannot validate sync with Rails config as this is resolved at build time.
1223-
Ensure your shakapacker.yml public_output_path matches server_bundle_output_path.
1224-
MSG
1225-
return nil
1220+
checker.add_info("\n ℹ️ Could not check Shakapacker config: #{e.message}")
12261221
end
1227-
1228-
# Pattern 3: path: some_variable (can't validate)
1229-
if webpack_content.match?(/path:\s*[a-zA-Z_]\w*/)
1230-
checker.add_info("\n ℹ️ Webpack config uses a variable for output.path - cannot validate")
1231-
return nil
1232-
end
1233-
1234-
checker.add_info("\n ℹ️ Could not parse webpack server bundle path - skipping validation")
1235-
nil
1236-
end
1237-
1238-
# Normalize path for comparison (remove leading ./, trailing /)
1239-
def normalize_path(path)
1240-
return path unless path.is_a?(String)
1241-
1242-
normalized = path.strip
1243-
normalized = normalized.sub(%r{^\.?/}, "") # Remove leading ./ or /
1244-
normalized.sub(%r{/$}, "") # Remove trailing /
12451222
end
1223+
# rubocop:enable Metrics/MethodLength
12461224
end
12471225
# rubocop:enable Metrics/ClassLength
12481226
end

0 commit comments

Comments
 (0)