Skip to content

Commit 40bb3f2

Browse files
justin808claude
andcommitted
Security and code quality improvements
Addresses security issues and optimizations identified in code review: Security Fixes: - Fix command injection risk in precompile hook by using IO.popen with array form instead of backticks (spec/dummy/bin/shakapacker-precompile-hook:76) - Improve regex patterns to exclude commented configuration lines using negative lookahead to prevent false matches Code Quality: - Optimize webpack configuration to process rules in single pass instead of double iteration, improving build performance - Combine SCSS loader addition and CSS Modules configuration into one loop Documentation: - Add comprehensive CHANGELOG.md entry documenting Shakapacker 9.0.0 upgrade including configuration changes, precompile hook, and compatibility notes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 0d51136 commit 40bb3f2

File tree

3 files changed

+23
-14
lines changed

3 files changed

+23
-14
lines changed

CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,15 @@ Changes since the last non-beta release.
3131

3232
- **Improved RSC Payload Error Handling**: Errors that happen during generation of RSC payload are transferred properly to rails side and logs the error message and stack. [PR #1888](https://github.com/shakacode/react_on_rails/pull/1888) by [AbanoubGhadban](https://github.com/AbanoubGhadban).
3333

34+
#### Changed
35+
36+
- **Shakapacker 9.0.0 Upgrade**: Upgraded Shakapacker from 8.2.0 to 9.0.0 with Babel transpiler configuration for compatibility. Key changes include:
37+
- Configured `javascript_transpiler: babel` in shakapacker.yml (Shakapacker 9.0 defaults to SWC which has PropTypes handling issues)
38+
- Added precompile hook support via `bin/shakapacker-precompile-hook` for ReScript builds and pack generation
39+
- Configured CSS Modules to use default exports (`namedExport: false`) for backward compatibility with existing `import styles from` syntax
40+
- Fixed webpack configuration to process SCSS rules and CSS loaders in a single pass for better performance
41+
[PR 1904](https://github.com/shakacode/react_on_rails/pull/1904) by [justin808](https://github.com/justin808).
42+
3443
#### Bug Fixes
3544

3645
- **Use as Git dependency**: All packages can now be installed as Git dependencies. This is useful for development and testing purposes. See [CONTRIBUTING.md](./CONTRIBUTING.md#git-dependencies) for documentation. [PR #1873](https://github.com/shakacode/react_on_rails/pull/1873) by [alexeyr-ci2](https://github.com/alexeyr-ci2).

spec/dummy/bin/shakapacker-precompile-hook

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,9 @@ def generate_packs_if_needed
6161

6262
# Check if auto-pack generation is configured (match actual config assignments, not comments)
6363
config_file = File.read(initializer_path)
64-
has_auto_load = config_file =~ /^\s*config\.auto_load_bundle\s*=/
65-
has_components_subdir = config_file =~ /^\s*config\.components_subdirectory\s*=/
64+
# Match uncommented configuration lines only (lines not starting with #)
65+
has_auto_load = config_file =~ /^\s*(?!#).*config\.auto_load_bundle\s*=/
66+
has_components_subdir = config_file =~ /^\s*(?!#).*config\.components_subdirectory\s*=/
6667
return unless has_auto_load || has_components_subdir
6768

6869
puts "📦 Generating React on Rails packs..."
@@ -71,8 +72,8 @@ def generate_packs_if_needed
7172
bundle_available = system("bundle", "--version", out: File::NULL, err: File::NULL)
7273
return unless bundle_available
7374

74-
# Check if rake task exists (cross-platform)
75-
task_list = `bundle exec rails -T 2>&1`
75+
# Check if rake task exists (use array form for security)
76+
task_list = IO.popen(["bundle", "exec", "rails", "-T"], err: [:child, :out], &:read)
7677
return unless task_list.include?("react_on_rails:generate_packs")
7778

7879
# Use array form for better cross-platform support

spec/dummy/config/webpack/commonWebpackConfig.js

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,17 @@ const sassLoaderConfig = {
2121
},
2222
};
2323

24-
// Add sass-resources-loader to all SCSS rules (both .scss and .module.scss)
25-
baseClientWebpackConfig.module.rules.forEach((rule) => {
26-
if (rule.test && '.scss'.match(rule.test) && Array.isArray(rule.use)) {
27-
rule.use.push(sassLoaderConfig);
28-
}
29-
});
30-
31-
// Configure CSS Modules to use default exports (Shakapacker 9.0 compatibility)
32-
// Shakapacker 9.0 defaults to namedExport: true, but we use default imports
33-
// To restore backward compatibility with existing code using `import styles from`
24+
// Process webpack rules in single pass for efficiency
3425
baseClientWebpackConfig.module.rules.forEach((rule) => {
3526
if (Array.isArray(rule.use)) {
27+
// Add sass-resources-loader to all SCSS rules (both .scss and .module.scss)
28+
if (rule.test && '.scss'.match(rule.test)) {
29+
rule.use.push(sassLoaderConfig);
30+
}
31+
32+
// Configure CSS Modules to use default exports (Shakapacker 9.0 compatibility)
33+
// Shakapacker 9.0 defaults to namedExport: true, but we use default imports
34+
// To restore backward compatibility with existing code using `import styles from`
3635
rule.use.forEach((loader) => {
3736
if (
3837
loader &&

0 commit comments

Comments
 (0)