Skip to content

Commit 763520d

Browse files
justin808claude
andauthored
Make RSC Rspack doctor lazyCompilation warning honest about its limitation (#4249)
## Summary Fixes #4243. Follow-up to #4227 and #4234. The `react_on_rails:doctor:rsc` lazyCompilation check only recognizes the generated literal pattern: ```js clientWebpackConfig.lazyCompilation = false; ``` When an app disables lazyCompilation an **equivalent** way, the check still emitted a warning whose wording **asserted** the config was wrong: ``` config/rspack/development.js does not appear to disable lazyCompilation while the dev server is running. ``` That is a false positive for advanced users whose effective dev-server config is correct (e.g. object form, `Object.assign`, a helper, a ternary, or disabling it in a different config file — see the matrix in #4243). ## What this changes This is the **wording fix** the issue explicitly lists as an acceptable resolution ("make the warning wording explicit that it only failed to find the generated literal config pattern"). The not-found branch now: - states doctor **could not confirm** lazyCompilation is disabled (instead of asserting it is *enabled*); - explains it only detects the generated literal assignment, so equivalent configs read as *unconfirmed*, not *broken*; - tells users with equivalent configs they can ignore the warning after confirming their effective config. The high-confidence literal-pattern **success** path is unchanged, and the genuine "no disable control at all" case still gets the same actionable warning + fix snippet. No detector logic or severity change — only honest wording, so there are no new false *positives* (claiming "disabled" when it isn't). Internally, the warning emission moved into a small private helper to keep `check_rsc_rspack_lazy_compilation` within the `Metrics/AbcSize` budget after the added guidance line. ## Why not detect the effective config? The issue's other option — evaluate the real final Rspack config (e.g. via `pack-config-diff`) — is heavyweight (spawns Node, evaluates arbitrary user config), adds a dependency to a static diagnostic, and is fragile across toolchains. For a P3 doctor-polish issue that is over-scoped; this PR removes the user-facing harm (the misleading assertion) at near-zero risk. Accurate effective-config evaluation remains the documented future path if maintainers want deeper detection later. ## Testing - `cd react_on_rails && bundle exec rspec spec/lib/react_on_rails/doctor_spec.rb -e "check_rsc_rspack_lazy_compilation"` — 8 examples, 0 failures (7 pre-existing assertions on the warning/info substrings still pass, confirming no behavior regression; 1 new regression test). - `cd react_on_rails && bundle exec rspec spec/lib/react_on_rails/doctor_rake_task_spec.rb` — 12 examples, 0 failures. - `bundle exec rubocop react_on_rails/lib/react_on_rails/doctor.rb react_on_rails/spec/lib/react_on_rails/doctor_spec.rb` — no offenses. - `codex review --base origin/main` — no findings. The new test uses the `Object.assign(clientWebpackConfig, { lazyCompilation: false })` variant from the issue and asserts the warning no longer says "does not appear to disable" and now says "could not confirm lazyCompilation is disabled". ## Labels `Labels: ready-for-hosted-ci` — `script/ci-changes-detector` sets `run_generators=true` for this `lib/react_on_rails/doctor.rb` change, so `required-pr-gate` requires hosted CI. Optimized selection (not `force-full`) is sufficient: this is a localized diagnostic-wording change, not a CI-detector/runtime/broad-generator change. ## Decision Log - **Non-blocking — scope:** Issue offers (1) evaluate the effective Rspack config or (2) honest wording. Chose (2). **Why:** (1) is heavyweight (spawns Node, evaluates arbitrary user config, adds a dependency to a static diagnostic, fragile across toolchains) and over-scoped for a P3; (2) removes the user-facing harm at near-zero risk. **Review later:** accurate effective-config detection if maintainers want deeper coverage. - **Non-blocking — detection breadth:** Did not broaden the static regex to claim "success" on more variants. **Why:** it would resolve ~1 of 7 variants while adding false-*positive* risk (object form can be partial, e.g. `{ entries: true }`), the exact risk #4234 avoided. Honest wording covers all undetectable variants uniformly. - **Non-blocking — hosted CI:** Requested optimized hosted CI via `+ci-run-hosted` after local validation. **Why:** generator gate requires it; documented flow. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed the RSC Rspack doctor to avoid false failure when `lazyCompilation` is disabled via semantically equivalent (non-literal) configuration patterns. * Improved messaging to clearly indicate when the setting can’t be confirmed statically, with guidance on how to verify it in the dev-server setup. * **Tests** * Added spec coverage for an equivalent `lazyCompilation: false` configuration pattern to ensure the correct warning and guidance are shown. * **Documentation** * Updated the changelog with the latest doctor behavior note. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 77e1b02 commit 763520d

3 files changed

Lines changed: 59 additions & 10 deletions

File tree

CHANGELOG.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,18 @@ After a release, run `/update-changelog` in Claude Code to analyze commits, writ
4848

4949
#### Fixed
5050

51+
- **[Pro]** **RSC Rspack doctor no longer false-warns on equivalent `lazyCompilation` configs**:
52+
`react_on_rails:doctor:rsc` only recognizes the generated literal
53+
`clientWebpackConfig.lazyCompilation = false` assignment. Apps that disable lazy
54+
compilation an equivalent way (object form, `Object.assign`, a helper, a ternary,
55+
or a different config file) now get a warning that says doctor could not _confirm_
56+
the setting rather than asserting lazy compilation is still enabled, plus guidance
57+
to confirm the effective dev-server config and ignore the warning if it is already
58+
disabled. Follow-up to
59+
[PR 4234](https://github.com/shakacode/react_on_rails/pull/4234).
60+
[PR 4249](https://github.com/shakacode/react_on_rails/pull/4249) by
61+
[justin808](https://github.com/justin808).
62+
5163
- **[Pro]** **Rspack RSC dev-server setup is easier to diagnose and customize**:
5264
Generated RSC helper code now verifies client-reference discovery support
5365
through the sibling `rscWebpackConfig.js` file instead of assuming

react_on_rails/lib/react_on_rails/doctor.rb

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3641,21 +3641,35 @@ def check_rsc_rspack_lazy_compilation
36413641
if development_config.match?(RSC_RSPACK_CLIENT_LAZY_COMPILATION_DISABLED_PATTERN)
36423642
checker.add_success("✅ Rspack lazy compilation appears disabled for RSC dev-server")
36433643
else
3644-
checker.add_warning(<<~MSG.strip)
3645-
⚠️ Rspack lazyCompilation can leave the RSC client manifest empty in normal bin/dev mode.
3646-
3647-
#{development_config_path} does not appear to disable lazyCompilation while the dev server is running.
3648-
MSG
3649-
checker.add_info(" 💡 Add this inside the Rspack WEBPACK_SERVE branch:")
3650-
checker.add_info(" clientWebpackConfig.lazyCompilation = false;")
3651-
checker.add_info(
3652-
" 💡 See docs/oss/migrating/rsc-troubleshooting.md#empty-client-manifest-with-rspack-dev-server"
3653-
)
3644+
warn_rsc_rspack_lazy_compilation_unconfirmed(development_config_path)
36543645
end
36553646
rescue StandardError => e
36563647
checker.add_warning("⚠️ Could not inspect RSC Rspack lazyCompilation config: #{e.message}")
36573648
end
36583649

3650+
# The static check only recognizes the generated literal
3651+
# `clientWebpackConfig.lazyCompilation = false` assignment. Equivalent custom configs (object form,
3652+
# Object.assign, a helper, a ternary, or a different config file) cannot be confirmed statically, so the
3653+
# wording stays honest about that limitation instead of asserting lazyCompilation is enabled.
3654+
def warn_rsc_rspack_lazy_compilation_unconfirmed(development_config_path)
3655+
checker.add_warning(<<~MSG.strip)
3656+
⚠️ Rspack lazyCompilation can leave the RSC client manifest empty in normal bin/dev mode.
3657+
3658+
Doctor could not confirm lazyCompilation is disabled in #{development_config_path}. This check only
3659+
recognizes the generated `clientWebpackConfig.lazyCompilation = false` assignment, so configs that
3660+
disable it another way are reported as unconfirmed even when they are correct.
3661+
MSG
3662+
checker.add_info(" 💡 If lazyCompilation is not disabled, add this inside the Rspack WEBPACK_SERVE branch:")
3663+
checker.add_info(" clientWebpackConfig.lazyCompilation = false;")
3664+
checker.add_info(
3665+
" 💡 If you disable lazyCompilation another way (object form, Object.assign, a helper, a ternary, " \
3666+
"or a different config file), confirm your effective dev-server config and you can ignore this warning."
3667+
)
3668+
checker.add_info(
3669+
" 💡 See docs/oss/migrating/rsc-troubleshooting.md#empty-client-manifest-with-rspack-dev-server"
3670+
)
3671+
end
3672+
36593673
def rsc_rspack_config_directory
36603674
explicit_config_path = shakapacker_assets_bundler_config_path
36613675
return bundler_config_directory(explicit_config_path) if explicit_config_path && File.file?(explicit_config_path)

react_on_rails/spec/lib/react_on_rails/doctor_spec.rb

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5397,6 +5397,29 @@ def self.upload(_hash, **_opts); end
53975397
expect(warning_messages)
53985398
.to include(a_string_including("Rspack lazyCompilation can leave the RSC client manifest empty"))
53995399
end
5400+
5401+
it "describes the detector limitation instead of asserting lazyCompilation is enabled for equivalent configs" do
5402+
# Issue #4243: equivalent configs (here Object.assign) effectively set
5403+
# lazyCompilation: false, but the static check only recognizes the literal
5404+
# generated assignment. The warning must not claim lazyCompilation is still enabled.
5405+
File.write(
5406+
"config/rspack/development.js",
5407+
"Object.assign(clientWebpackConfig, { lazyCompilation: false });\n"
5408+
)
5409+
5410+
doctor.send(:check_rsc_rspack_lazy_compilation)
5411+
5412+
warning_messages = checker.messages.select { |msg| msg[:type] == :warning }.map { |msg| msg[:content] }
5413+
info_messages = checker.messages.select { |msg| msg[:type] == :info }.map { |msg| msg[:content] }
5414+
success_messages = checker.messages.select { |msg| msg[:type] == :success }.map { |msg| msg[:content] }
5415+
expect(warning_messages)
5416+
.to include(a_string_including("could not confirm lazyCompilation is disabled"))
5417+
expect(warning_messages).not_to include(a_string_including("does not appear to disable"))
5418+
expect(info_messages)
5419+
.to include(a_string_including("disable lazyCompilation another way"))
5420+
# An equivalent config must not also produce a spurious success alongside the warning.
5421+
expect(success_messages).to be_empty
5422+
end
54005423
end
54015424

54025425
describe "check_rsc_react_version" do

0 commit comments

Comments
 (0)