Skip to content

Commit 014f35e

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 <noreply@anthropic.com>
1 parent 94655e0 commit 014f35e

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
@@ -12,17 +12,19 @@ ReactOnRails.configure do |config|
1212
# Set to "" if you're not using server rendering
1313
config.server_bundle_js_file = "server-bundle.js"
1414

15-
# ⚠️ IMPORTANT: This must match output.path in config/webpack/serverWebpackConfig.js
15+
# ⚠️ RECOMMENDED: Use Shakapacker 9.0+ private_output_path instead
1616
#
17-
# Both are currently set to 'ssr-generated' (relative to Rails.root)
18-
# Keeping these in sync ensures React on Rails can find the server bundle at runtime.
17+
# If using Shakapacker 9.0+, add to config/shakapacker.yml:
18+
# private_output_path: ssr-generated
1919
#
20-
# Run 'rails react_on_rails:doctor' to verify these configs are in sync.
20+
# React on Rails will auto-detect this value, eliminating the need to set it here.
21+
# This keeps your webpack and Rails configs in sync automatically.
2122
#
22-
# Configure where server bundles are output. Defaults to "ssr-generated".
23-
# This path is relative to Rails.root and should point to a private directory
24-
# (outside of public/) for security.
25-
config.server_bundle_output_path = "ssr-generated"
23+
# For older Shakapacker versions or custom setups, manually configure:
24+
# config.server_bundle_output_path = "ssr-generated"
25+
#
26+
# The path is relative to Rails.root and should point to a private directory
27+
# (outside of public/) for security. Run 'rails react_on_rails:doctor' to verify.
2628

2729
# Enforce that server bundles are only loaded from private (non-public) directories.
2830
# 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
@@ -184,6 +184,7 @@ def setup_config_values
184184
check_component_registry_timeout
185185
validate_generated_component_packs_loading_strategy
186186
validate_enforce_private_server_bundles
187+
auto_detect_server_bundle_path_from_shakapacker
187188
end
188189

189190
private
@@ -257,6 +258,35 @@ def validate_enforce_private_server_bundles
257258
"the public directory. Please set it to a directory outside of public."
258259
end
259260

261+
# Auto-detect server_bundle_output_path from Shakapacker 9.0+ private_output_path
262+
# Only sets if user hasn't explicitly configured server_bundle_output_path
263+
def auto_detect_server_bundle_path_from_shakapacker
264+
# Skip if user explicitly set server_bundle_output_path to something other than default
265+
return if server_bundle_output_path != "ssr-generated"
266+
267+
# Skip if Shakapacker is not available
268+
return unless defined?(::Shakapacker)
269+
270+
# Check if Shakapacker config has private_output_path method (9.0+)
271+
return unless ::Shakapacker.config.respond_to?(:private_output_path)
272+
273+
begin
274+
private_path = ::Shakapacker.config.private_output_path
275+
return unless private_path
276+
277+
# Convert from Pathname to relative string path
278+
relative_path = private_path.to_s.sub("#{Rails.root}/", "")
279+
self.server_bundle_output_path = relative_path
280+
281+
Rails.logger.info("ReactOnRails: Auto-detected server_bundle_output_path from " \
282+
"shakapacker.yml private_output_path: '#{relative_path}'")
283+
rescue StandardError => e
284+
# Fail gracefully - if auto-detection fails, keep the default
285+
Rails.logger.debug("ReactOnRails: Could not auto-detect server bundle path from " \
286+
"Shakapacker: #{e.message}")
287+
end
288+
end
289+
260290
def check_minimum_shakapacker_version
261291
ReactOnRails::PackerUtils.raise_shakapacker_version_incompatible_for_basic_pack_generation unless
262292
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
@@ -688,8 +688,8 @@ def analyze_server_rendering_config(content)
688688
enforce_private_match = content.match(/config\.enforce_private_server_bundles\s*=\s*([^\s\n,]+)/)
689689
checker.add_info(" enforce_private_server_bundles: #{enforce_private_match[1]}") if enforce_private_match
690690

691-
# Validate webpack config matches Rails config
692-
validate_server_bundle_path_sync(rails_bundle_path)
691+
# Check Shakapacker integration and provide recommendations
692+
check_shakapacker_private_output_path(rails_bundle_path)
693693

694694
# RSC bundle file (Pro feature)
695695
rsc_bundle_match = content.match(/config\.rsc_bundle_js_file\s*=\s*["']([^"']+)["']/)
@@ -1405,91 +1405,69 @@ def log_debug(message)
14051405
Rails.logger.debug(message)
14061406
end
14071407

1408-
# Validates that webpack serverWebpackConfig.js output.path matches
1409-
# React on Rails config.server_bundle_output_path
1410-
def validate_server_bundle_path_sync(rails_bundle_path)
1411-
webpack_config_path = "config/webpack/serverWebpackConfig.js"
1412-
1413-
unless File.exist?(webpack_config_path)
1414-
checker.add_info("\n ℹ️ Webpack server config not found - skipping path validation")
1408+
# Check Shakapacker private_output_path integration and provide recommendations
1409+
# rubocop:disable Metrics/MethodLength
1410+
def check_shakapacker_private_output_path(rails_bundle_path)
1411+
unless defined?(::Shakapacker)
1412+
checker.add_info("\n ℹ️ Shakapacker not detected - using manual configuration")
14151413
return
14161414
end
14171415

1418-
begin
1419-
webpack_content = File.read(webpack_config_path)
1416+
# Check if Shakapacker 9.0+ with private_output_path support
1417+
unless ::Shakapacker.config.respond_to?(:private_output_path)
1418+
checker.add_info(<<~MSG.strip)
1419+
\n 💡 Recommendation: Upgrade to Shakapacker 9.0+
14201420
1421-
# Try to extract the path from webpack config
1422-
webpack_bundle_path = extract_webpack_output_path(webpack_content, webpack_config_path)
1421+
Shakapacker 9.0+ adds 'private_output_path' in shakapacker.yml for server bundles.
1422+
This eliminates the need to configure server_bundle_output_path separately.
14231423
1424-
return unless webpack_bundle_path
1424+
Benefits:
1425+
- Single source of truth in shakapacker.yml
1426+
- Automatic detection by React on Rails
1427+
- No configuration duplication
1428+
MSG
1429+
return
1430+
end
14251431

1426-
# Normalize and compare paths
1427-
normalized_webpack_path = normalize_path(webpack_bundle_path)
1428-
normalized_rails_path = normalize_path(rails_bundle_path)
1432+
# Shakapacker 9.0+ is available
1433+
begin
1434+
private_path = ::Shakapacker.config.private_output_path
14291435

1430-
if normalized_webpack_path == normalized_rails_path
1431-
checker.add_success("\n ✅ Webpack and Rails configs are in sync (both use '#{rails_bundle_path}')")
1432-
else
1433-
checker.add_warning(<<~MSG.strip)
1434-
\n ⚠️ Configuration mismatch detected!
1436+
if private_path
1437+
relative_path = private_path.to_s.sub("#{Rails.root}/", "")
14351438

1436-
React on Rails config (config/initializers/react_on_rails.rb):
1437-
server_bundle_output_path = "#{rails_bundle_path}"
1439+
if relative_path == rails_bundle_path
1440+
checker.add_success("\n ✅ Using Shakapacker 9.0+ private_output_path: '#{relative_path}'")
1441+
checker.add_info(" Auto-detected from shakapacker.yml - no manual config needed")
1442+
else
1443+
checker.add_warning(<<~MSG.strip)
1444+
\n ⚠️ Configuration mismatch detected!
14381445
1439-
Webpack config (#{webpack_config_path}):
1440-
output.path = "#{webpack_bundle_path}" (relative to Rails.root)
1446+
Shakapacker private_output_path: '#{relative_path}'
1447+
React on Rails server_bundle_output_path: '#{rails_bundle_path}'
14411448
1442-
These must match for server rendering to work correctly.
1449+
Recommendation: Remove server_bundle_output_path from your React on Rails
1450+
initializer and let it auto-detect from shakapacker.yml private_output_path.
1451+
MSG
1452+
end
1453+
else
1454+
checker.add_info(<<~MSG.strip)
1455+
\n 💡 Recommendation: Configure private_output_path in shakapacker.yml
14431456
1444-
To fix:
1445-
1. Update server_bundle_output_path in config/initializers/react_on_rails.rb, OR
1446-
2. Update output.path in #{webpack_config_path}
1457+
Add to config/shakapacker.yml:
1458+
private_output_path: #{rails_bundle_path}
14471459
1448-
Make sure both point to the same directory relative to Rails.root.
1460+
This will:
1461+
- Keep webpack and Rails configs in sync automatically
1462+
- Enable auto-detection by React on Rails
1463+
- Serve as single source of truth for server bundle location
14491464
MSG
14501465
end
14511466
rescue StandardError => e
1452-
checker.add_info("\n ℹ️ Could not validate webpack config: #{e.message}")
1453-
end
1454-
end
1455-
1456-
# Extract output.path from webpack config, supporting multiple patterns
1457-
def extract_webpack_output_path(webpack_content, _webpack_config_path)
1458-
# Pattern 1: path: require('path').resolve(__dirname, '../../ssr-generated')
1459-
# Explicitly match ../../path pattern for clarity
1460-
hardcoded_pattern = %r{path:\s*require\(['"]path['"]\)\.resolve\(__dirname,\s*['"]\.\./\.\./([^'"]+)['"]\)}
1461-
if (match = webpack_content.match(hardcoded_pattern))
1462-
return match[1]
1463-
end
1464-
1465-
# Pattern 2: path: config.outputPath (can't validate - runtime value)
1466-
if webpack_content.match?(/path:\s*config\.outputPath/)
1467-
checker.add_info(<<~MSG.strip)
1468-
\n ℹ️ Webpack config uses config.outputPath (from shakapacker.yml)
1469-
Cannot validate sync with Rails config as this is resolved at build time.
1470-
Ensure your shakapacker.yml public_output_path matches server_bundle_output_path.
1471-
MSG
1472-
return nil
1467+
checker.add_info("\n ℹ️ Could not check Shakapacker config: #{e.message}")
14731468
end
1474-
1475-
# Pattern 3: path: some_variable (can't validate)
1476-
if webpack_content.match?(/path:\s*[a-zA-Z_]\w*/)
1477-
checker.add_info("\n ℹ️ Webpack config uses a variable for output.path - cannot validate")
1478-
return nil
1479-
end
1480-
1481-
checker.add_info("\n ℹ️ Could not parse webpack server bundle path - skipping validation")
1482-
nil
1483-
end
1484-
1485-
# Normalize path for comparison (remove leading ./, trailing /)
1486-
def normalize_path(path)
1487-
return path unless path.is_a?(String)
1488-
1489-
normalized = path.strip
1490-
normalized = normalized.sub(%r{^\.?/}, "") # Remove leading ./ or /
1491-
normalized.sub(%r{/$}, "") # Remove trailing /
14921469
end
1470+
# rubocop:enable Metrics/MethodLength
14931471
end
14941472
# rubocop:enable Metrics/ClassLength
14951473
end

0 commit comments

Comments
 (0)