Skip to content

Commit 06e1fa6

Browse files
justin808claude
andcommitted
Address code review comments for configuration documentation
This commit addresses all 8 code review comments on the configuration simplification PR, improving documentation clarity and adding comprehensive testing guidance. ## Documentation Improvements - Fix: Changed "Deprecated:" to "Renamed to:" in configuration-deprecated.md for better clarity - Add: Filename heading for /config/shakapacker.yml in Prerequisites section - Add: Migration note in configuration-pro.md explaining Pro docs will be consolidated in future PR ## Testing Configuration Enhancements - Enhance: Completely rewrote build_test_command section in configuration.md - Clarified the two approaches are mutually exclusive - Added clear pros/cons for each approach - Provided detailed examples for both methods - Linked to comprehensive testing guide - NEW: Created comprehensive testing configuration guide (docs/guides/testing-configuration.md) - 390 lines of detailed guidance - Covers both Shakapacker auto-compilation and React on Rails test helper - Migration guides between approaches - Troubleshooting section - Performance considerations - CI/CD guidance - Decision matrix for choosing approaches ## Generator Template Updates - Update: Commented out build_test_command in generator template by default - Recommends simpler Shakapacker compile: true approach - Provides clear alternative with test helper for advanced users - Links to comprehensive testing guide - Prevents confusion for new users ## Doctor Program Enhancements - NEW: Added check_server_bundle_prerender_consistency() to doctor.rb - Detects when prerender is enabled but server_bundle_js_file not configured - Detects when server_bundle_js_file is set but prerender never used - Scans view files for prerender: true usage - Provides actionable recommendations - NEW: Added check_build_test_configuration() to doctor.rb - Detects conflicting test asset compilation configurations - Checks for both compile: true and build_test_command being set - Validates alignment between config and test helpers - Detects missing test asset compilation setup - Links to testing configuration guide ## Files Changed - docs/api-reference/configuration-deprecated.md - docs/api-reference/configuration-pro.md - docs/api-reference/configuration.md - docs/guides/testing-configuration.md (NEW) - lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.tt - lib/react_on_rails/doctor.rb All changes pass RuboCop and Prettier validation. Addresses comments: #1 (not applicable), #2, #3, #4, #5, #6, #7, #8, #9 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 3473b57 commit 06e1fa6

File tree

6 files changed

+577
-11
lines changed

6 files changed

+577
-11
lines changed

docs/api-reference/configuration-deprecated.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ See [CHANGELOG.md](../CHANGELOG.md) for details.
3131
**Default:** `false`
3232
**Status:** ⚠️ DEPRECATED
3333

34-
**Deprecated:** Use `generated_component_packs_loading_strategy = :defer` instead.
34+
**Renamed to:** `generated_component_packs_loading_strategy = :defer`
3535

3636
**Migration:**
3737

docs/api-reference/configuration-pro.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ React Server Components and Streaming SSR are React on Rails Pro features.
1111
For detailed configuration of RSC and streaming features, see the Pro package documentation:
1212
[react_on_rails_pro/docs/configuration.md](https://github.com/shakacode/react_on_rails/blob/master/react_on_rails_pro/docs/configuration.md)
1313

14+
> **Note:** The Pro documentation is currently maintained separately in the `react_on_rails_pro` directory. We plan to migrate and consolidate Pro documentation into the main docs structure in a future PR for better discoverability and consistency.
15+
1416
### Key Pro Configurations
1517

1618
These options are configured in the `ReactOnRailsPro.configure` block:

docs/api-reference/configuration.md

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ See [Essential Configuration](#essential-configuration) below for the minimal co
1010

1111
## Prerequisites
1212

13+
### `/config/shakapacker.yml`
14+
1315
First, you should have a `/config/shakapacker.yml` setup.
1416

1517
Here is the setup when using the recommended `/` directory for your `node_modules` and source files:
@@ -115,21 +117,60 @@ Note: There should be ONE server bundle that can render all your server-rendered
115117
**Default:** `""`
116118
**Used with:** `ReactOnRails::TestHelper.configure_rspec_to_compile_assets(config)`
117119

118-
**Note:** It's generally preferred to set `compile: true` in your `config/shakapacker.yml` for the test environment instead of using this configuration option. Use this option only if you need to customize the build command or want explicit control over test asset compilation.
120+
**Important:** This option is only needed if you're using the React on Rails test helper. The two approaches below are **mutually exclusive** - use one or the other, not both.
121+
122+
#### Recommended Approach: Shakapacker Auto-Compilation
123+
124+
Set `compile: true` in `config/shakapacker.yml` for the test environment. Shakapacker will automatically compile assets before running tests:
125+
126+
```yaml
127+
test:
128+
compile: true
129+
public_output_path: webpack/test
130+
```
131+
132+
**Pros:**
133+
134+
- Simpler configuration (no extra setup in spec helpers)
135+
- Managed by Shakapacker directly
136+
- Automatically integrates with Rails test environment
137+
138+
**Cons:**
119139
120-
The command to run to build webpack assets during test runs:
140+
- Less explicit control over when compilation happens
141+
- May compile more often than necessary
142+
143+
#### Alternative Approach: React on Rails Test Helper
144+
145+
Use `build_test_command` with `ReactOnRails::TestHelper.configure_rspec_to_compile_assets(config)` if you need explicit control:
121146

122147
```ruby
148+
# config/initializers/react_on_rails.rb
123149
config.build_test_command = "RAILS_ENV=test bin/shakapacker"
124150
```
125151

126-
**Preferred alternative:** Set `compile: true` in `config/shakapacker.yml`:
152+
```ruby
153+
# spec/rails_helper.rb (or spec_helper.rb)
154+
require "react_on_rails/test_helper"
127155
128-
```yaml
129-
test:
130-
compile: true
156+
RSpec.configure do |config|
157+
ReactOnRails::TestHelper.configure_rspec_to_compile_assets(config)
158+
end
131159
```
132160

161+
**Pros:**
162+
163+
- Explicit control over compilation timing
164+
- Only compiles once before test suite runs
165+
- Can customize the build command
166+
167+
**Cons:**
168+
169+
- Requires additional setup in spec helpers
170+
- More configuration to maintain
171+
172+
For more details on testing configuration, see the [Testing Configuration Guide](../guides/testing-configuration.md).
173+
133174
## File-Based Component Registry
134175

135176
If you have many components and want to avoid manually managing webpack entry points for each one, React on Rails can automatically generate component packs based on your file system structure. This feature is particularly useful for large applications with dozens of components.

0 commit comments

Comments
 (0)