Remove immediate_hydration feature from codebase#2834
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR removes the Changes
Sequence Diagram(s)sequenceDiagram
participant Server as Rails Server
participant Client as Browser (ClientStartup)
participant Renderer as ClientSideRenderer
participant Pro as ReactOnRails Pro (rorPro)
Server->>Client: Render HTML + <script class="js-react-on-rails-component"> (no data-immediate-hydration)
Client->>Client: clientStartup() runs
Client->>Pro: getRailsContext() -> { rorPro: true/false }
alt rorPro == true
Client->>Renderer: renderOrHydrateCompleteComponents() / hydrateCompleteStores()
Renderer->>Client: hydrate/render as streaming scripts complete
else rorPro == false
Client->>Client: wait for DOMContentLoaded
Client->>Renderer: renderOrHydrateAllComponents() / hydrateAllStores()
Renderer->>Client: hydrate/render on DOMContentLoaded
end
Note right of Server: stream_react_component warns if passed :immediate_hydration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Review: Remove immediate_hydration featureOverall this is a clean, well-scoped removal. The logic simplification is welcome and the test coverage cleanup correctly mirrors what is being deleted. A few things worth addressing: Stale comment in ClientSideRenderer.ts Line 271 (inside the forAllCompleteElementsAsync JSDoc) still reads: 'Elements without a nextSibling will be hydrated later when their immediate hydration script executes and calls reactOnRailsComponentLoaded().' Since the per-component opt-in script is gone, 'immediate hydration script' should be updated (e.g. 'inline Pro hydration script') to stay accurate. react_component silently ignores leftover immediate_hydration option redux_store (helper + controller) removed the explicit named parameter, so passing immediate_hydration: there will correctly raise ArgumentError: unknown keyword. However react_component takes a generic options = {} hash, so callers who still pass immediate_hydration: true get no error and no warning — the key is just never read. Worth adding a one-time deprecation warn in RenderOptions or the helper so apps get a clear signal during the upgrade window. Behaviour change scope for renderOrHydrateCompleteComponents The old function selected only [data-immediate-hydration=true] elements. The new one selects ALL .js-react-on-rails-component elements that have a nextSibling. For Pro users every complete component is now eligible for pre-page-load hydration regardless of the original opt-in. The CHANGELOG covers the intent, but a sentence explicitly noting this all-or-nothing shift would help upgraders understand the broader scope. Minor: changelog section order The new Changed section was inserted above the existing Added section. Keep-a-Changelog convention orders: Added -> Changed -> Deprecated -> Removed. Swapping the two sections would keep it consistent. |
Greptile SummaryThis PR completes the removal of the Key changes:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Browser
participant JSBundle as JS Bundle (Pro)
participant ReactOnRails as ReactOnRails Client
participant DOM
Note over Browser,DOM: Old flow (with immediate_hydration flag)
Browser->>DOM: Parse HTML stream
DOM-->>Browser: Component script tag with data-immediate-hydration="true"
JSBundle->>ReactOnRails: clientStartup()
ReactOnRails->>DOM: Query [data-immediate-hydration="true"] components
DOM-->>ReactOnRails: Only flagged components
ReactOnRails->>Browser: Hydrate flagged components immediately
Browser->>ReactOnRails: DOMContentLoaded
ReactOnRails->>DOM: Query ALL .js-react-on-rails-component
ReactOnRails->>Browser: Hydrate remaining components
Note over Browser,DOM: New flow (Pro always-on)
Browser->>DOM: Parse HTML stream
JSBundle->>ReactOnRails: clientStartup()
ReactOnRails->>DOM: Query ALL .js-react-on-rails-component (nextSibling != null)
DOM-->>ReactOnRails: All complete components
ReactOnRails->>Browser: Hydrate all complete components immediately
Browser->>ReactOnRails: DOMContentLoaded
ReactOnRails->>DOM: Query ALL .js-react-on-rails-component
ReactOnRails->>Browser: Hydrate any remaining components
|
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
react_on_rails/spec/dummy/spec/helpers/react_on_rails_helper_spec.rb (1)
430-451: Test logic correctly validates Pro-only hydration behavior.The tests properly verify that the inline hydration script is present only when
react_on_rails_pro?returns true.Consider using named subjects for better test readability, as per coding guidelines:
♻️ Suggested improvement for named subjects
describe "Pro inline hydration script" do let(:hydration_script) do %(typeof ReactOnRails === 'object' && ReactOnRails.reactOnRailsComponentLoaded('App-react-component-0');) .html_safe end context "with Pro gem installed" do - subject { react_component("App") } + subject(:rendered_component) { react_component("App") } - it { is_expected.to include hydration_script } + it { expect(rendered_component).to include hydration_script } end context "without Pro gem installed" do - subject { react_component("App") } + subject(:rendered_component) { react_component("App") } before do allow(ReactOnRails::Utils).to receive(:react_on_rails_pro?).and_return(false) end - it { is_expected.not_to include hydration_script } + it { expect(rendered_component).not_to include hydration_script } end endAs per coding guidelines: "Use named subjects in RSpec tests (e.g.,
subject(:method_result) { ... }instead of unnamedsubject { ... })"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@react_on_rails/spec/dummy/spec/helpers/react_on_rails_helper_spec.rb` around lines 430 - 451, Replace unnamed subjects with named subjects to improve readability: in the "Pro inline hydration script" examples change the anonymous subject { react_component("App") } to a named subject (e.g., subject(:component_html) { react_component("App") }) and update the examples to use that name in expectations; keep references to hydration_script and to mocking ReactOnRails::Utils.react_on_rails_pro? as-is so it’s clear which behavior is being tested.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 27-29: Update the CHANGELOG entry to mention that removal of
immediate_hydration affects initializer, helper calls, and the controller API by
noting that react_on_rails/lib/react_on_rails/controller.rb drops the
immediate_hydration: keyword from redux_store (reference immediate_hydration and
redux_store), and append the standard PR/author footer formatted as
“[PR_NUMBER](PR_url) by [username](user_url)” (no hash before PR number) to
match other entries.
---
Nitpick comments:
In `@react_on_rails/spec/dummy/spec/helpers/react_on_rails_helper_spec.rb`:
- Around line 430-451: Replace unnamed subjects with named subjects to improve
readability: in the "Pro inline hydration script" examples change the anonymous
subject { react_component("App") } to a named subject (e.g.,
subject(:component_html) { react_component("App") }) and update the examples to
use that name in expectations; keep references to hydration_script and to
mocking ReactOnRails::Utils.react_on_rails_pro? as-is so it’s clear which
behavior is being tested.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 77ed90ba-6a3f-4209-a952-c82a4c76e3d8
📒 Files selected for processing (23)
CHANGELOG.mdpackages/react-on-rails-pro/src/ClientSideRenderer.tspackages/react-on-rails-pro/src/createReactOnRailsPro.tspackages/react-on-rails/src/clientStartup.tsreact_on_rails/lib/react_on_rails/configuration.rbreact_on_rails/lib/react_on_rails/controller.rbreact_on_rails/lib/react_on_rails/doctor.rbreact_on_rails/lib/react_on_rails/helper.rbreact_on_rails/lib/react_on_rails/pro_helper.rbreact_on_rails/lib/react_on_rails/react_component/render_options.rbreact_on_rails/lib/react_on_rails/utils.rbreact_on_rails/sig/react_on_rails/configuration.rbsreact_on_rails/sig/react_on_rails/controller.rbsreact_on_rails/spec/dummy/app/views/pages/turbo_stream_send_hello_world.turbo_stream.erbreact_on_rails/spec/dummy/e2e/playwright/e2e/react_on_rails/basic_components.spec.jsreact_on_rails/spec/dummy/spec/helpers/react_on_rails_helper_spec.rbreact_on_rails/spec/dummy/spec/support/selenium_logger.rbreact_on_rails/spec/react_on_rails/configuration_spec.rbreact_on_rails/spec/react_on_rails/react_component/render_options_spec.rbreact_on_rails/spec/react_on_rails/utils_spec.rbreact_on_rails_pro/app/helpers/react_on_rails_pro_helper.rbreact_on_rails_pro/spec/dummy/app/views/layouts/application.html.erbreact_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
💤 Files with no reviewable changes (8)
- react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
- react_on_rails/lib/react_on_rails/configuration.rb
- react_on_rails/spec/react_on_rails/react_component/render_options_spec.rb
- react_on_rails/sig/react_on_rails/configuration.rbs
- react_on_rails/lib/react_on_rails/react_component/render_options.rb
- react_on_rails/spec/react_on_rails/configuration_spec.rb
- react_on_rails/lib/react_on_rails/utils.rb
- react_on_rails/spec/react_on_rails/utils_spec.rb
|
Addressed review feedback in 383a5f2: Fixed:
Skipped (with rationale):
|
|
Overall this is a clean, well-scoped removal. The logic simplification in both Ruby and TypeScript is a clear win, and the renamed JS exports ( Concern 1: Silent API break for view-layer callers Users who currently pass Concern 2: The old Concern 3: CI integration tests still pending The test-plan checkboxes for "CI: dummy app integration tests" and "CI: Pro integration tests" are unchecked. Given that this changes hydration ordering and the set of elements hydrated early, merging before those pass is risky. The deleted unit tests were well-targeted but do not cover the streaming/timing path end-to-end. Minor: The silent delete protects against callers who still pass the old option — correct behavior — but a brief comment noting this is a backward-compat shim for a removed option would prevent the line from looking like dead code to future readers. Positives: Removal of the mutex-protected |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react-on-rails-pro/src/ClientSideRenderer.ts (1)
283-284: Please lock this selector broadening down with an integration test.These two selectors are the client-side contract change for removing
data-immediate-hydration. I don't see coverage in the provided context for a streamed component/store without the removed attribute hydrating beforeDOMContentLoaded, and that's the path most likely to regress.Also applies to: 318-319
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-on-rails-pro/src/ClientSideRenderer.ts` around lines 283 - 284, Add an integration test that locks the new broad selector usage ('.js-react-on-rails-component') by verifying the client-side contract in renderOrHydrateCompleteComponents (which calls forAllCompleteElementsAsync with renderOrHydrateComponent) and the equivalent place at lines ~318-319: build a fixture that simulates a streamed component/store element without the removed data-immediate-hydration attribute, load the client bundle in a test harness that can assert whether hydration occurs prior to dispatching DOMContentLoaded (or simulate/document when DOMContentLoaded fires), and fail the test if the element is not hydrated early; this ensures the selector change does not regress streamed hydration behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/react-on-rails-pro/src/ClientSideRenderer.ts`:
- Around line 283-284: Add an integration test that locks the new broad selector
usage ('.js-react-on-rails-component') by verifying the client-side contract in
renderOrHydrateCompleteComponents (which calls forAllCompleteElementsAsync with
renderOrHydrateComponent) and the equivalent place at lines ~318-319: build a
fixture that simulates a streamed component/store element without the removed
data-immediate-hydration attribute, load the client bundle in a test harness
that can assert whether hydration occurs prior to dispatching DOMContentLoaded
(or simulate/document when DOMContentLoaded fires), and fail the test if the
element is not hydrated early; this ensures the selector change does not regress
streamed hydration behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dfe4116c-de45-4344-8bbe-fc9d25663cda
📒 Files selected for processing (4)
CHANGELOG.mdpackages/create-react-on-rails-app/src/create-app.tspackages/create-react-on-rails-app/src/index.tspackages/react-on-rails-pro/src/ClientSideRenderer.ts
✅ Files skipped from review due to trivial changes (2)
- packages/create-react-on-rails-app/src/create-app.ts
- packages/create-react-on-rails-app/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
justin808
force-pushed
the
jg/2142-rm-immediate-hydration
branch
from
a946d87 to
da3e0db
Compare
coderabbitai
Bot
added
review-needed
docs-cleanup
|
Review: Remove immediate_hydration feature The overall direction is correct and the implementation is clean. The deduplication concern (components processed by both renderOrHydrateCompleteComponents and then again by renderOrHydrateAllComponents) is safely handled by the renderedRoots Map in renderOrHydrateComponent. Good. Summary of inline comments posted:
Key concern: Pro users who relied on What is handled well:
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb (1)
126-130:⚠️ Potential issue | 🟡 MinorNormalize the removed option before cached paths see it.
Line 130 strips
:immediate_hydrationtoo late for the cached streaming flow:fetch_stream_react_componentalready derives the cache key fromraw_optionson Line 295. Upgraded callers that still pass the removed option will create separate cache entries for identical streamed output.♻️ Proposed normalization
def cached_stream_react_component(component_name, raw_options = {}, &block) + raw_options = raw_options.dup + raw_options.delete(:immediate_hydration) + ReactOnRailsPro::Utils.with_trace(component_name) do check_caching_options!(raw_options, block) fetch_stream_react_component(component_name, raw_options, &block) end end def stream_react_component(component_name, options = {}) + options = options.dup + # stream_react_component doesn't have the prerender option # Because setting prerender to false is equivalent to calling react_component with prerender: false options[:prerender] = true options.delete(:immediate_hydration)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb` around lines 126 - 130, The cached streaming flow creates cache keys from raw_options before the helper strips :immediate_hydration, so callers that still pass that removed option produce different cache entries; update stream_react_component (and/or the code that builds raw_options used by fetch_stream_react_component) to normalize options early by removing :immediate_hydration (and any other deprecated keys) before the cache key is derived so identical streamed output shares the same cache entry (reference: stream_react_component, fetch_stream_react_component, raw_options).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@react_on_rails/lib/react_on_rails/pro_helper.rb`:
- Around line 41-44: The attribute value for "data-js-react-on-rails-store" is
being marked html_safe (redux_store_data[:store_name].html_safe), which can
bypass escaping and allow attribute/script injection; change the content_tag
invocation so the store name is passed without html_safe (e.g., use
redux_store_data[:store_name].to_s or the raw value) and let content_tag handle
escaping of attribute values; keep the script body handling
(json_safe_and_pretty(...).html_safe) as-is.
---
Outside diff comments:
In `@react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb`:
- Around line 126-130: The cached streaming flow creates cache keys from
raw_options before the helper strips :immediate_hydration, so callers that still
pass that removed option produce different cache entries; update
stream_react_component (and/or the code that builds raw_options used by
fetch_stream_react_component) to normalize options early by removing
:immediate_hydration (and any other deprecated keys) before the cache key is
derived so identical streamed output shares the same cache entry (reference:
stream_react_component, fetch_stream_react_component, raw_options).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e98cc500-e834-440a-92d9-718ab1020f0f
📒 Files selected for processing (24)
CHANGELOG.mdpackages/create-react-on-rails-app/src/create-app.tspackages/react-on-rails-pro/src/ClientSideRenderer.tspackages/react-on-rails-pro/src/createReactOnRailsPro.tspackages/react-on-rails/src/clientStartup.tsreact_on_rails/lib/react_on_rails/configuration.rbreact_on_rails/lib/react_on_rails/controller.rbreact_on_rails/lib/react_on_rails/doctor.rbreact_on_rails/lib/react_on_rails/helper.rbreact_on_rails/lib/react_on_rails/pro_helper.rbreact_on_rails/lib/react_on_rails/react_component/render_options.rbreact_on_rails/lib/react_on_rails/utils.rbreact_on_rails/sig/react_on_rails/configuration.rbsreact_on_rails/sig/react_on_rails/controller.rbsreact_on_rails/spec/dummy/app/views/pages/turbo_stream_send_hello_world.turbo_stream.erbreact_on_rails/spec/dummy/e2e/playwright/e2e/react_on_rails/basic_components.spec.jsreact_on_rails/spec/dummy/spec/helpers/react_on_rails_helper_spec.rbreact_on_rails/spec/dummy/spec/support/selenium_logger.rbreact_on_rails/spec/react_on_rails/configuration_spec.rbreact_on_rails/spec/react_on_rails/react_component/render_options_spec.rbreact_on_rails/spec/react_on_rails/utils_spec.rbreact_on_rails_pro/app/helpers/react_on_rails_pro_helper.rbreact_on_rails_pro/spec/dummy/app/views/layouts/application.html.erbreact_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
💤 Files with no reviewable changes (8)
- react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
- react_on_rails/lib/react_on_rails/react_component/render_options.rb
- react_on_rails/spec/react_on_rails/utils_spec.rb
- react_on_rails/lib/react_on_rails/configuration.rb
- react_on_rails/spec/react_on_rails/configuration_spec.rb
- react_on_rails/sig/react_on_rails/configuration.rbs
- react_on_rails/spec/react_on_rails/react_component/render_options_spec.rb
- react_on_rails/lib/react_on_rails/utils.rb
✅ Files skipped from review due to trivial changes (6)
- packages/react-on-rails/src/clientStartup.ts
- react_on_rails/spec/dummy/app/views/pages/turbo_stream_send_hello_world.turbo_stream.erb
- react_on_rails_pro/spec/dummy/app/views/layouts/application.html.erb
- packages/create-react-on-rails-app/src/create-app.ts
- react_on_rails/lib/react_on_rails/helper.rb
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (5)
- react_on_rails/spec/dummy/e2e/playwright/e2e/react_on_rails/basic_components.spec.js
- react_on_rails/spec/dummy/spec/support/selenium_logger.rb
- react_on_rails/lib/react_on_rails/controller.rb
- react_on_rails/sig/react_on_rails/controller.rbs
- react_on_rails/spec/dummy/spec/helpers/react_on_rails_helper_spec.rb
Review: Remove immediate_hydration feature (#2834)Overall this is a clean, well-scoped removal — the architecture is sound and the test coverage for the new behavior is solid. A few issues to address before merge: Issues1. RBS type signatures still include Both 2. In 3. Silent no-op in When 4. CHANGELOG should note per-component opt-out is gone The current changelog entry mentions the removal but doesn't explicitly state that Minor observations
|
The immediate_hydration parameter was still present in the RBS type signatures for controller.rbs and helper.rbs, which would let type checkers silently accept calls with the removed option. Removing them ensures Steep/RBS flag any lingering usage. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Review address summaryI (Claude Code) addressed the latest round of review comments: Fixed (2):
Declined with rationale (3):
All 5 review threads resolved. |
| this.state = 'hydrated'; | ||
| } | ||
|
|
||
| hasStartedHydrating(): boolean { |
There was a problem hiding this comment.
State inconsistency when railsContext is null
Both ComponentRenderer and StoreRenderer set this.state = 'rendering'/'hydrating' at the top of their constructors, then early-return when railsContext is null, leaving renderPromise/hydratePromise undefined. The hasStartedHydrating() / hasStartedRendering() guards in the callers correctly detect this and discard the instance — so it's benign in practice.
However, if waitUntilHydrated() were ever called on a discarded instance (e.g. by a future code path), it would check this.state === 'hydrating' && this.hydratePromise → the condition fails and it silently returns Promise.resolve() without hydrating anything. A small defensive improvement would be to initialise state to a distinct sentinel like 'idle' when railsContext is absent, so any such mistaken call would be more visible.
| // (identified by having a nextSibling). On non-streaming pages this matches ALL components, | ||
| // but ClientSideRenderer memoizes by DOM node id so the later DOMContentLoaded sweep is a no-op. | ||
| export const renderOrHydrateCompleteComponents = () => | ||
| forAllCompleteElementsAsync('.js-react-on-rails-component', renderOrHydrateComponent); |
There was a problem hiding this comment.
Behavior change: now sweeps ALL Pro components, not just data-immediate-hydration="true" ones
This is a meaningful semantic change: the old selector '.js-react-on-rails-component[data-immediate-hydration="true"]' only targeted components that were explicitly opted in. The new selector matches every rendered component.
On a non-streaming page, when clientStartup runs with railsContext available, forAllCompleteElementsAsync sees all components as complete (they all have a nextSibling) and triggers early hydration for every single one — previously this only happened for explicitly marked components.
This is the intended goal (all Pro users get immediate hydration automatically), but it should be verified that no existing Pro apps relied on selectively disabling immediate hydration for expensive components. The comment above the function explains the memoisation safety net, which is correct.
| if (railsContext === null) { | ||
| // Context element not yet in DOM — expected in streaming scenarios. | ||
| // Early Pro hydration skipped; the page-loaded sweep will recover all components. | ||
| if (process.env.NODE_ENV !== 'production' && typeof console !== 'undefined') { |
There was a problem hiding this comment.
typeof console !== 'undefined' is always true in a browser context — console is part of the Web API spec and guaranteed to exist. The check can be dropped:
| if (process.env.NODE_ENV !== 'production' && typeof console !== 'undefined') { | |
| if (process.env.NODE_ENV !== 'production') { |
| def redux_store(store_name, props: {}, **rest) | ||
| immediate_hydration_present = rest.key?(:immediate_hydration) |
There was a problem hiding this comment.
Duplicated **rest validation between controller.rb and helper.rb
Lines 15–24 here are byte-for-byte identical to helper.rb lines 201–209 (which handles the same redux_store method on the view-helper side). Both files will need to be updated if a new removed keyword is ever added or the error message changes. Consider extracting to a shared private method, e.g. in ReactOnRails::Utils:
# utils.rb
def self.validate_no_unknown_kwargs(rest, known_removed: [])
if rest.key?(:immediate_hydration) || known_removed.any? { |k| rest.key?(k) }
ReactOnRails::Helper.warn_removed_immediate_hydration_option(...)
end
unknown_keys = rest.keys - known_removed
if unknown_keys.any?
plural = unknown_keys.one? ? "" : "s"
raise ArgumentError, "unknown keyword#{plural}: #{unknown_keys.map { |k| ":#{k}" }.join(', ')}"
end
endNot a blocker, but worth a follow-up ticket to avoid drift.
Review: Remove
|
…e-dedup * origin/main: (51 commits) Fix Content-Length mismatch and null renderingRequest errors in node renderer (#3069) Improve memory debugging docs with simpler heap snapshot approach (#3072) Enforce strict version matching in doctor and recommend doctor on errors (#3070) Remove immediate_hydration feature from codebase (#2834) Fix infinite fork loop when node renderer worker fails to bind port (#2881) Fix TanStack Router SSR hydration mismatch in Pro async path (#2932) Improve node renderer error messages for malformed render requests (#3068) Add interactive mode prompt to create-react-on-rails-app (#3063) docs: replace hardcoded version numbers with unversioned install commands (#2893) Remove Contributing section from docs sidebar (#3064) Consolidate docs comparison pages into single evaluation entry (#3065) Docs route cleanup: canonicalize worst verbose URL slugs (#3067) Add agent summary blocks to high-value docs pages (#3066) Fix brittle positional assertions in create-app tests (#2923) Auto-resolve renderer password from ENV in Rails Pro (#2921) Standardize bundle env vars from =yes to =true (#2925) Fix Pro generator multiline and template-literal rewrites (#2918) Fix spec/dummy Procfile.dev SERVER_BUNDLE_ONLY to match template convention (#2922) Add llms docs entry points (#2916) Bump version to 16.6.0.rc.0 ... # Conflicts: # packages/create-react-on-rails-app/src/create-app.ts
…lacement-2835-main * origin/main: (44 commits) Fix Content-Length mismatch and null renderingRequest errors in node renderer (#3069) Improve memory debugging docs with simpler heap snapshot approach (#3072) Enforce strict version matching in doctor and recommend doctor on errors (#3070) Remove immediate_hydration feature from codebase (#2834) Fix infinite fork loop when node renderer worker fails to bind port (#2881) Fix TanStack Router SSR hydration mismatch in Pro async path (#2932) Improve node renderer error messages for malformed render requests (#3068) Add interactive mode prompt to create-react-on-rails-app (#3063) docs: replace hardcoded version numbers with unversioned install commands (#2893) Remove Contributing section from docs sidebar (#3064) Consolidate docs comparison pages into single evaluation entry (#3065) Docs route cleanup: canonicalize worst verbose URL slugs (#3067) Add agent summary blocks to high-value docs pages (#3066) Fix brittle positional assertions in create-app tests (#2923) Auto-resolve renderer password from ENV in Rails Pro (#2921) Standardize bundle env vars from =yes to =true (#2925) Fix Pro generator multiline and template-literal rewrites (#2918) Fix spec/dummy Procfile.dev SERVER_BUNDLE_ONLY to match template convention (#2922) Add llms docs entry points (#2916) Bump version to 16.6.0.rc.0 ... # Conflicts: # CHANGELOG.md
### Summary Adds missing changelog entries to the `[Unreleased]` section for PRs merged since `v16.6.0.rc.0`. Analyzed 19 commits on `origin/main` since the last tag, identified 1 missing user-visible PR (#3069), and skipped 10 non-user-visible PRs (docs, tests, internal refactors). **New entries added:** - **Added**: `[Pro] Configurable HTTP keep-alive timeout for node renderer connections` (PR #3069) - **Fixed**: `[Pro] Fixed SSR failures from stale persistent HTTP/2 connections` (PR #3069) **Entries already present** (added in prior changelog updates): PRs #2834, #2881, #2918, #2921, #2923, #2932, #3063, #3068, #3070. **Skipped** (not user-visible): #2893 (docs), #2916 (docs), #2922 (test fix), #2923 (test fix), #2925 (internal refactor), #3064 (docs), #3065 (docs), #3066 (docs), #3067 (docs), #3072 (docs). ### Pull Request checklist - [x] ~Add/update test to cover these changes~ - [x] ~Update documentation~ - [x] Update CHANGELOG file ### Other Information No code changes — CHANGELOG.md only. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk because this PR only changes documentation (`CHANGELOG.md`) and does not modify runtime code. > > **Overview** > Updates `CHANGELOG.md` *[Unreleased]* to include missing Pro release notes for PR `#3069`, documenting the new `renderer_http_keep_alive_timeout` config and the associated fix for SSR failures from stale persistent HTTP/2 connections to the node renderer. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 8d1a480. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Interactive mode selection prompt for CLI tool when no explicit mode is specified * Configurable keep-alive timeout setting for Pro users (default 30 seconds) * **Bug Fixes** * Enhanced validation and error handling for invalid request payloads * Improved template literal handling in code generation * Better HTTP connection stability with enhanced diagnostic messaging <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
## Summary - Removes the `immediate_hydration` configuration option, helper parameter, `data-immediate-hydration` HTML attribute, and all related normalization/validation logic - Immediate hydration is now always enabled for Pro users and disabled for non-Pro users — no per-component toggle - The `config.immediate_hydration` deprecated setter/getter (added in 16.2.0) is fully removed - Pro `stream_react_component` no longer injects `immediate_hydration: true`; Pro inline hydration scripts are now unconditionally added based on `react_on_rails_pro?` - JS Pro package: renamed `renderOrHydrateImmediateHydratedComponents` → `renderOrHydrateCompleteComponents` (now selects all components, not just those with the removed attribute) Closes #2142 ## Test plan - [x] RuboCop: 0 offenses across all changed files (173 inspected) - [x] RBS validation passed - [x] TypeScript type-check passed (all 4 packages) - [x] Gem unit tests: 160 examples, 0 failures - [ ] CI: dummy app integration tests (helper spec, E2E) - [ ] CI: Pro integration tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Medium risk because it removes a previously supported configuration/keyword option and changes client hydration triggering logic for streaming/non-streaming pages, which could affect render timing or cause missed hydration if edge cases exist. > > **Overview** > **Removes the `immediate_hydration` feature end-to-end**: drops `config.immediate_hydration`, per-helper keyword args, and the `data-immediate-hydration` DOM attribute, with Pro now hydrating early automatically and non-Pro hydrating on page load (no per-component override). > > **Updates Pro streaming hydration** to hydrate any *complete* streamed component/store (nextSibling-present) rather than filtering by the removed attribute, and avoids caching renderers created before `railsContext` exists; `stream_react_component` now warns if `immediate_hydration:` is provided. > > **Hardens/cleans up behavior and tests**: adds one-time warnings when removed options are passed, tightens keyword-arg validation for `redux_store`, fixes HTML attribute escaping for store names to prevent attribute injection, and updates/extends Ruby, TS, and dummy app tests/docs accordingly. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 389ac3a. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Removed the immediate_hydration option and per-component HTML attribute; immediate hydration is now Pro‑gated (enabled for Pro, disabled otherwise). * **Bug Fixes / Behavior** * Passing immediate_hydration to streaming calls now emits a warning; store name serialization escaping fixed to prevent attribute injection. * **Documentation** * Changelog and inline docs updated to reflect the new Pro‑gated hydration behavior. * **Tests** * Tests updated to reflect removed option and to verify store-name escaping. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
### Summary Adds missing changelog entries to the `[Unreleased]` section for PRs merged since `v16.6.0.rc.0`. Analyzed 19 commits on `origin/main` since the last tag, identified 1 missing user-visible PR (#3069), and skipped 10 non-user-visible PRs (docs, tests, internal refactors). **New entries added:** - **Added**: `[Pro] Configurable HTTP keep-alive timeout for node renderer connections` (PR #3069) - **Fixed**: `[Pro] Fixed SSR failures from stale persistent HTTP/2 connections` (PR #3069) **Entries already present** (added in prior changelog updates): PRs #2834, #2881, #2918, #2921, #2923, #2932, #3063, #3068, #3070. **Skipped** (not user-visible): #2893 (docs), #2916 (docs), #2922 (test fix), #2923 (test fix), #2925 (internal refactor), #3064 (docs), #3065 (docs), #3066 (docs), #3067 (docs), #3072 (docs). ### Pull Request checklist - [x] ~Add/update test to cover these changes~ - [x] ~Update documentation~ - [x] Update CHANGELOG file ### Other Information No code changes — CHANGELOG.md only. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk because this PR only changes documentation (`CHANGELOG.md`) and does not modify runtime code. > > **Overview** > Updates `CHANGELOG.md` *[Unreleased]* to include missing Pro release notes for PR `#3069`, documenting the new `renderer_http_keep_alive_timeout` config and the associated fix for SSR failures from stale persistent HTTP/2 connections to the node renderer. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 8d1a480. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Interactive mode selection prompt for CLI tool when no explicit mode is specified * Configurable keep-alive timeout setting for Pro users (default 30 seconds) * **Bug Fixes** * Enhanced validation and error handling for invalid request payloads * Improved template literal handling in code generation * Better HTTP connection stability with enhanced diagnostic messaging <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
## Summary - Removes the `immediate_hydration` configuration option, helper parameter, `data-immediate-hydration` HTML attribute, and all related normalization/validation logic - Immediate hydration is now always enabled for Pro users and disabled for non-Pro users — no per-component toggle - The `config.immediate_hydration` deprecated setter/getter (added in 16.2.0) is fully removed - Pro `stream_react_component` no longer injects `immediate_hydration: true`; Pro inline hydration scripts are now unconditionally added based on `react_on_rails_pro?` - JS Pro package: renamed `renderOrHydrateImmediateHydratedComponents` → `renderOrHydrateCompleteComponents` (now selects all components, not just those with the removed attribute) Closes #2142 ## Test plan - [x] RuboCop: 0 offenses across all changed files (173 inspected) - [x] RBS validation passed - [x] TypeScript type-check passed (all 4 packages) - [x] Gem unit tests: 160 examples, 0 failures - [ ] CI: dummy app integration tests (helper spec, E2E) - [ ] CI: Pro integration tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Medium risk because it removes a previously supported configuration/keyword option and changes client hydration triggering logic for streaming/non-streaming pages, which could affect render timing or cause missed hydration if edge cases exist. > > **Overview** > **Removes the `immediate_hydration` feature end-to-end**: drops `config.immediate_hydration`, per-helper keyword args, and the `data-immediate-hydration` DOM attribute, with Pro now hydrating early automatically and non-Pro hydrating on page load (no per-component override). > > **Updates Pro streaming hydration** to hydrate any *complete* streamed component/store (nextSibling-present) rather than filtering by the removed attribute, and avoids caching renderers created before `railsContext` exists; `stream_react_component` now warns if `immediate_hydration:` is provided. > > **Hardens/cleans up behavior and tests**: adds one-time warnings when removed options are passed, tightens keyword-arg validation for `redux_store`, fixes HTML attribute escaping for store names to prevent attribute injection, and updates/extends Ruby, TS, and dummy app tests/docs accordingly. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 389ac3a. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Removed the immediate_hydration option and per-component HTML attribute; immediate hydration is now Pro‑gated (enabled for Pro, disabled otherwise). * **Bug Fixes / Behavior** * Passing immediate_hydration to streaming calls now emits a warning; store name serialization escaping fixed to prevent attribute injection. * **Documentation** * Changelog and inline docs updated to reflect the new Pro‑gated hydration behavior. * **Tests** * Tests updated to reflect removed option and to verify store-name escaping. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
### Summary Adds missing changelog entries to the `[Unreleased]` section for PRs merged since `v16.6.0.rc.0`. Analyzed 19 commits on `origin/main` since the last tag, identified 1 missing user-visible PR (#3069), and skipped 10 non-user-visible PRs (docs, tests, internal refactors). **New entries added:** - **Added**: `[Pro] Configurable HTTP keep-alive timeout for node renderer connections` (PR #3069) - **Fixed**: `[Pro] Fixed SSR failures from stale persistent HTTP/2 connections` (PR #3069) **Entries already present** (added in prior changelog updates): PRs #2834, #2881, #2918, #2921, #2923, #2932, #3063, #3068, #3070. **Skipped** (not user-visible): #2893 (docs), #2916 (docs), #2922 (test fix), #2923 (test fix), #2925 (internal refactor), #3064 (docs), #3065 (docs), #3066 (docs), #3067 (docs), #3072 (docs). ### Pull Request checklist - [x] ~Add/update test to cover these changes~ - [x] ~Update documentation~ - [x] Update CHANGELOG file ### Other Information No code changes — CHANGELOG.md only. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk because this PR only changes documentation (`CHANGELOG.md`) and does not modify runtime code. > > **Overview** > Updates `CHANGELOG.md` *[Unreleased]* to include missing Pro release notes for PR `#3069`, documenting the new `renderer_http_keep_alive_timeout` config and the associated fix for SSR failures from stale persistent HTTP/2 connections to the node renderer. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 8d1a480. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Interactive mode selection prompt for CLI tool when no explicit mode is specified * Configurable keep-alive timeout setting for Pro users (default 30 seconds) * **Bug Fixes** * Enhanced validation and error handling for invalid request payloads * Improved template literal handling in code generation * Better HTTP connection stability with enhanced diagnostic messaging <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
The `immediate_hydration` option was removed from the codebase in #2834. The behavior it controlled (Pro users hydrate components before DOMContentLoaded) still exists, but it's now always-on for Pro and always-off for OSS — there is no per-component toggle. Several docs still showed `immediate_hydration: true/false` in code examples or advised users to "enable" the option. This reframes those sections as descriptions of Pro's built-in early hydration behavior and drops the stale code examples. Files updated: - docs/pro/streaming-ssr.md — rewrite "Immediate Hydration" as "Early Hydration (Pro)", remove code example disabling the option - docs/pro/troubleshooting.md — drop the stale bullet - docs/oss/building-features/turbolinks.md — reframe Turbo Streams and async+Turbolinks sections; remove `immediate_hydration: true` from code examples - docs/oss/api-reference/ruby-api-pro.md — remove `:immediate_hydration` from stream_react_component options Fixes #3139
The `immediate_hydration` option was removed from the codebase in #2834. The behavior it controlled (Pro users hydrate components before DOMContentLoaded) still exists, but it's now always-on for Pro and always-off for OSS — there is no per-component toggle. Several docs still showed `immediate_hydration: true/false` in code examples or advised users to "enable" the option. This reframes those sections as descriptions of Pro's built-in early hydration behavior and drops the stale code examples. Files updated: - docs/pro/streaming-ssr.md — rewrite "Immediate Hydration" as "Early Hydration (Pro)", remove code example disabling the option - docs/pro/troubleshooting.md — drop the stale bullet - docs/oss/building-features/turbolinks.md — reframe Turbo Streams and async+Turbolinks sections; remove `immediate_hydration: true` from code examples - docs/oss/api-reference/ruby-api-pro.md — remove `:immediate_hydration` from stream_react_component options Fixes #3139
The troubleshooting entry for the immediate_hydration deprecation warning told users to "use component-level overrides if needed," but per-component immediate_hydration: keys have been no-ops since #2834 and configuration- deprecated.md now says to delete them. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
## Summary The `immediate_hydration` option was removed from the codebase in #2834. The behavior it controlled (Pro users hydrate components before `DOMContentLoaded`) still exists, but it's now always-on for Pro and always-off for OSS — there is no per-component toggle. Several docs still presented `immediate_hydration` as a configurable option, which confused users who tried to set it and found it didn't exist. This PR reframes those sections as descriptions of Pro's built-in early hydration behavior and drops stale code examples. ### Files updated - `docs/pro/streaming-ssr.md` — renamed "Immediate Hydration" to "Early Hydration (Pro)"; reworded surrounding prose; removed the `immediate_hydration: false` code example; updated migration-timeline bullet - `docs/pro/troubleshooting.md` — removed the "Verify `immediate_hydration` option" bullet from the hydration-failure checklist - `docs/oss/building-features/turbolinks.md` — reframed the Turbo Streams section (Pro's early hydration is automatic, not a per-call option); removed `immediate_hydration: true` from the code example; reworded the async+Turbolinks warning box; removed the v16.0 migration note that referenced the removed option; fixed GitHub `[!WARNING]` alert syntax so the callout renders - `docs/oss/api-reference/ruby-api-pro.md` — removed `:immediate_hydration` from the `stream_react_component` options list and split the options/hydration note for clarity - `docs/oss/configuration/configuration-deprecated.md` — tightened the migration step (delete both config and per-component `immediate_hydration:` keys — all are no-ops) and corrected the removal version to v16.6.0 - `docs/oss/upgrading/release-notes/16.2.0.md` — dropped the stale "use component-level overrides if needed" hint from the troubleshooting section (per-component keys were also removed in v16.6.0). The historical "Component-level overrides still work" section is preserved as-is since it accurately describes behavior at the 16.2.0 release point. ### Files intentionally left alone Per the issue, release notes and migration guidance still correctly reference `immediate_hydration` as a historical/deprecated option: - `docs/oss/upgrading/release-notes/16.0.0.md` - `docs/oss/configuration/README.md` - `docs/oss/upgrading/upgrading-react-on-rails.md` (migration guidance) Fixes #3139 ## Test plan - [x] `grep -rn immediate_hydration docs/pro docs/oss/building-features docs/oss/api-reference` returns no live/example references (only migration/deprecation guidance remains in upgrade-notes and deprecation docs, as intended) - [x] Prettier formatting passes on edited files - [x] pre-commit hooks (trailing newlines, markdown link check, prettier) all pass - [ ] Reviewer: visually scan the updated sections in `streaming-ssr.md` and `turbolinks.md` to confirm the reframing reads well 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
2 participants
Summary
immediate_hydrationconfiguration option, helper parameter,data-immediate-hydrationHTML attribute, and all related normalization/validation logicconfig.immediate_hydrationdeprecated setter/getter (added in 16.2.0) is fully removedstream_react_componentno longer injectsimmediate_hydration: true; Pro inline hydration scripts are now unconditionally added based onreact_on_rails_pro?renderOrHydrateImmediateHydratedComponents→renderOrHydrateCompleteComponents(now selects all components, not just those with the removed attribute)Closes #2142
Test plan
🤖 Generated with Claude Code
Note
Medium Risk
Medium risk because it removes a previously supported configuration/keyword option and changes client hydration triggering logic for streaming/non-streaming pages, which could affect render timing or cause missed hydration if edge cases exist.
Overview
Removes the
immediate_hydrationfeature end-to-end: dropsconfig.immediate_hydration, per-helper keyword args, and thedata-immediate-hydrationDOM attribute, with Pro now hydrating early automatically and non-Pro hydrating on page load (no per-component override).Updates Pro streaming hydration to hydrate any complete streamed component/store (nextSibling-present) rather than filtering by the removed attribute, and avoids caching renderers created before
railsContextexists;stream_react_componentnow warns ifimmediate_hydration:is provided.Hardens/cleans up behavior and tests: adds one-time warnings when removed options are passed, tightens keyword-arg validation for
redux_store, fixes HTML attribute escaping for store names to prevent attribute injection, and updates/extends Ruby, TS, and dummy app tests/docs accordingly.Reviewed by Cursor Bugbot for commit 389ac3a. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit