-
-
Notifications
You must be signed in to change notification settings - Fork 638
Show warning badge when force_load feature is enabled and RORP is not used #1780
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Show warning badge when force_load feature is enabled and RORP is not used #1780
Conversation
WalkthroughAdds Pro-license detection and a conditional "React On Rails Pro Required" badge; the helper now injects the badge when force_load is requested but the Pro licence is not valid. Introduces Utils.react_on_rails_pro_licence_valid? and adds tests covering badge rendering and warning logs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor View
participant Helper as ReactOnRails::Helper
participant Utils as ReactOnRails::Utils
participant Renderer as Server/Client Renderer
View->>Helper: react_component / react_component_hash / redux_store(..., force_load?)
Helper->>Utils: react_on_rails_pro_licence_valid?
alt Pro valid OR force_load is false
Helper->>Renderer: render content
Renderer-->>Helper: server_rendered_html (String/Hash)
Helper-->>View: html (no badge)
else Pro missing AND force_load true
Helper->>Helper: pro_warning_badge_if_needed(force_load) -> badge HTML
Helper->>Renderer: render content
Renderer-->>Helper: server_rendered_html (String/Hash)
Helper-->>View: badge + html (prepended or placed in COMPONENT_HTML_KEY)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
Code Review for PR #1780: Show warning badge when force_load feature is enabled and RORP is not used📋 Overall AssessmentThe implementation adds a visible warning badge when the ✅ Strengths
🔴 Critical Issues1. Method VisibilityThe # lib/react_on_rails/helper.rb:450
def pro_warning_badge_if_needed(force_load) # Should be privateThis exposes internal implementation details. Move it to the private section. 2. Performance Concern - Repeated Badge GenerationThe badge HTML is generated on every call when conditions are met. Consider memoizing or caching the badge HTML: def pro_warning_badge_if_needed(force_load)
@pro_warning_badge ||= begin
# Generate badge HTML once
end
end🟡 Code Quality Issues1. Inline StylesThe extensive inline CSS (lines 461-467) violates separation of concerns. Consider:
2. Console Output Side EffectLine 455:
3. HTML SafetyWhile # Current
(badge + html).html_safe
# Better
safe_join([badge, html])🟠 Security Considerations
📊 Test CoverageExcellent test coverage! The specs properly test:
Minor suggestion: Add a test for multiple components on the same page to ensure badge doesn't duplicate. 🚀 Performance Implications
📝 Documentation & PR Description
🔧 Suggested Improvements
class ProLicenseWarningBadge
def self.render(force_load_enabled)
return "" unless should_show_warning?(force_load_enabled)
# Badge generation logic
end
end
ReactOnRails.configure do |config|
config.suppress_pro_warning = true # For users who acknowledge but can't upgrade
end
✅ Action Items Before Merge
SummaryThe feature successfully warns users about Pro requirements, but needs refinement in code organization, performance, and user experience. The persistent fixed-position badge might be too intrusive for production use - consider making it configurable or less prominent. Great work on the test coverage! With the suggested improvements, this will be a valuable addition to help users understand Pro feature requirements. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/react_on_rails/helper.rb (1)
473-497: Critical: Method visibility has been inadvertently changed.The removal of the
privatekeyword beforerun_stream_inside_fiberhas made this method and all subsequent methods (registered_stores, registered_stores_defer_render, registered_stores_including_deferred, create_render_options, etc.) public instead of private. This is a breaking change that exposes internal implementation details.Add the
privatekeyword beforerun_stream_inside_fiberto maintain proper encapsulation:+ private + def run_stream_inside_fiber unless ReactOnRails::Utils.react_on_rails_pro? raise ReactOnRails::Error, "You must use React on Rails Pro to use the stream_react_component method." endAlso applies to: 499-509
🧹 Nitpick comments (4)
spec/dummy/spec/helpers/react_on_rails_helper_spec.rb (1)
25-29: Good test setup, but Pro stub at the global level may affect all tests.The global stubbing of ReactOnRails::Utils to simulate a Pro license installed affects all tests in this file, even those that don't explicitly test Pro features. This could mask issues where non-Pro behavior should be tested. Consider moving this stub to specific contexts that require Pro functionality.
Consider moving the Pro stubs to specific contexts:
before do allow(self).to receive(:request) { Struct.new("Request", :original_url, :env) Struct::Request.new( "http://foobar.com/development", { "HTTP_ACCEPT_LANGUAGE" => "en" } ) } - - allow(ReactOnRails::Utils).to receive_messages( - react_on_rails_pro?: true, - react_on_rails_pro_version: "4.0.0", - rsc_support_enabled?: false - ) endThen add the Pro stubs only to contexts that need them:
context "with Pro license features" do before do allow(ReactOnRails::Utils).to receive_messages( react_on_rails_pro?: true, react_on_rails_pro_version: "4.0.0", rsc_support_enabled?: false ) end # ... Pro-specific tests endlib/react_on_rails/helper.rb (3)
450-471: Consider extracting badge HTML to a constant and improving console output.The method implementation is solid, but could benefit from some refinements:
- The badge HTML string could be extracted to a constant for maintainability
- Using
putsfor warnings may not be appropriate in production environments- The inline styles make the badge difficult to customize
+ PRO_WARNING_BADGE_STYLES = { + container: "position: fixed; top: 0; right: 0; width: 180px; height: 180px; overflow: hidden; z-index: 9999; pointer-events: none;", + badge: "position: absolute; top: 50px; right: -40px; transform: rotate(45deg); background-color: rgba(220, 53, 69, 0.85); color: white; padding: 7px 40px; text-align: center; font-weight: bold; font-family: sans-serif; font-size: 12px; box-shadow: 0 2px 8px rgba(0,0,0,0.3); pointer-events: auto;" + }.freeze + def pro_warning_badge_if_needed(force_load) return "".html_safe if !force_load && !ReactOnRails.configuration.force_load return "".html_safe if ReactOnRails::Utils.react_on_rails_pro? warning_message = "[REACT ON RAILS] The 'force_load' feature requires a React on Rails Pro license. " \ "Please visit https://shakacode.com/react-on-rails-pro to learn more." - puts warning_message + Rails.logger.info(warning_message) unless Rails.env.test? Rails.logger.warn warning_message tooltip_text = "The 'force_load' feature requires a React on Rails Pro license. Click to learn more." badge_html = <<~HTML <a href="https://shakacode.com/react-on-rails-pro" target="_blank" rel="noopener noreferrer" title="#{tooltip_text}"> - <div style="position: fixed; top: 0; right: 0; width: 180px; height: 180px; overflow: hidden; z-index: 9999; pointer-events: none;"> - <div style="position: absolute; top: 50px; right: -40px; transform: rotate(45deg); background-color: rgba(220, 53, 69, 0.85); color: white; padding: 7px 40px; text-align: center; font-weight: bold; font-family: sans-serif; font-size: 12px; box-shadow: 0 2px 8px rgba(0,0,0,0.3); pointer-events: auto;"> + <div style="#{PRO_WARNING_BADGE_STYLES[:container]}"> + <div style="#{PRO_WARNING_BADGE_STYLES[:badge]}"> React On Rails Pro Required </div> </div> </a> HTML badge_html.strip.html_safe end
450-471: Consider performance impact of badge injection.The badge HTML is being generated and injected for every component, store, and hash render when force_load is enabled. This could impact performance if there are many components on a page.
Consider implementing a singleton pattern to ensure the badge is only rendered once per page:
def pro_warning_badge_if_needed(force_load) return "".html_safe if !force_load && !ReactOnRails.configuration.force_load return "".html_safe if ReactOnRails::Utils.react_on_rails_pro? # Only render badge once per request return "".html_safe if @pro_warning_badge_rendered @pro_warning_badge_rendered = true # ... rest of the implementation end
456-457: Duplicate logging may cause confusion.The warning message is being output twice - once with
putsand once withRails.logger.warn. This could lead to duplicate messages in logs.Remove the
putsstatement as Rails.logger is sufficient:warning_message = "[REACT ON RAILS] The 'force_load' feature requires a React on Rails Pro license. " \ "Please visit https://shakacode.com/react-on-rails-pro to learn more." - puts warning_message Rails.logger.warn warning_message
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/react_on_rails/helper.rb(5 hunks)spec/dummy/spec/helpers/react_on_rails_helper_spec.rb(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
lib/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
Ruby source code must adhere to RuboCop rules (project uses RuboCop for Ruby linting)
Files:
lib/react_on_rails/helper.rb
spec/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
Ruby tests should be written for RSpec (project uses RSpec for Ruby testing)
Files:
spec/dummy/spec/helpers/react_on_rails_helper_spec.rb
🧠 Learnings (2)
📓 Common learnings
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
Applied to files:
lib/react_on_rails/helper.rbspec/dummy/spec/helpers/react_on_rails_helper_spec.rb
🧬 Code graph analysis (2)
lib/react_on_rails/helper.rb (3)
lib/react_on_rails/react_component/render_options.rb (1)
force_load(98-100)lib/react_on_rails/configuration.rb (1)
configuration(16-57)lib/react_on_rails/utils.rb (1)
react_on_rails_pro?(199-203)
spec/dummy/spec/helpers/react_on_rails_helper_spec.rb (3)
lib/react_on_rails/helper.rb (3)
react_component(56-94)react_component_hash(211-242)redux_store(257-273)lib/react_on_rails/react_component/render_options.rb (2)
props(26-28)force_load(98-100)lib/react_on_rails/controller.rb (1)
redux_store(15-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: dummy-app-integration-tests (oldest)
- GitHub Check: dummy-app-integration-tests (newest)
- GitHub Check: build
- GitHub Check: rspec-package-tests (oldest)
- GitHub Check: rspec-package-tests (newest)
🔇 Additional comments (7)
spec/dummy/spec/helpers/react_on_rails_helper_spec.rb (4)
380-451: Comprehensive Pro license warning test coverage.The tests thoroughly cover various scenarios with Pro license presence/absence and force_load configuration. The test structure properly isolates contexts and verifies both badge inclusion and logger warnings.
453-507: Good test coverage for react_component_hash Pro warnings.The tests properly validate that the Pro warning badge is injected into the componentHtml key when needed, maintaining consistency with other rendering methods.
511-511: Test updated correctly to include force_load parameter.The redux_store call now properly passes the force_load parameter, aligning with the new API signature.
533-577: Well-structured Pro license warning tests for redux_store.The tests follow the same comprehensive pattern as other Pro warning tests, checking badge inclusion and logger warnings across different Pro license and force_load scenarios.
lib/react_on_rails/helper.rb (3)
61-61: Pro warning badge correctly integrated into react_component.The badge is properly computed and prepended to the HTML output for server-rendered components, ensuring it appears before the component content.
Also applies to: 71-71
217-217: Badge properly added to component hash output.The Pro warning badge is correctly injected into the COMPONENT_HTML_KEY of the hash result, maintaining consistency with the react_component implementation.
Also applies to: 230-230
259-259: Badge correctly integrated with redux_store rendering.The badge is properly computed and prepended to the rendered redux store data along with the Rails context.
Also applies to: 271-271
Code Review for PR #1780: Show warning badge when force_load feature is enabled and RORP is not used✅ Strengths
🔍 Areas for Improvement1. Method Visibility Issue 🚨The 2. Performance Concern
|
Code Review for PR #1780: Show warning badge when force_load feature is enabled and RORP is not usedOverall AssessmentThis PR successfully implements a visual warning system for unauthorized use of the 🟢 Strengths
🟡 Code Quality & Best Practices
🟠 Potential Issues
🔴 Security Considerations
📊 Performance Considerations
🧪 Test Coverage
📝 Suggestions for Improvement
✅ Checklist Items
💬 Final ThoughtsThis is a well-implemented feature that achieves its goal of clearly warning users about Pro license requirements. The main improvements would be around code organization (moving the method to private section, extracting constants) and addressing the minor UX/accessibility concerns. The test coverage is excellent. Great work on the implementation! With the suggested improvements, this will be a solid addition to the codebase. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
lib/react_on_rails/utils.rb (1)
10-10: Scope RuboCop disable or justify it.Consider limiting
Metrics/ModuleLengthdisable to the smallest scope or add a brief justification. Long-term, splittingUtilsinto cohesive modules would remove the need to disable the cop.spec/dummy/spec/helpers/react_on_rails_helper_spec.rb (3)
377-447: Strengthen assertions for badge prepend and add log check for global flag.
- Verify the badge is prepended (not just included) by asserting order relative to the component HTML.
- In the “global force_load” context, also assert a warning is logged for parity with the per‑component case.
@@ context "when Pro license is NOT installed and force_load is true" do subject(:react_app) { react_component("App", props: props, force_load: true) } @@ it { is_expected.to include(badge_html_string) } + + it "prepends the badge before the component HTML" do + html = react_app.to_s + expect(html.index(badge_html_string)).to be < html.index(react_component_div) + end @@ context "when Pro license is NOT installed and global force_load is true" do subject(:react_app) { react_component("App", props: props) } @@ it { is_expected.to include(badge_html_string) } + + it "logs a warning" do + react_app + expect(Rails.logger).to have_received(:warn).with(a_string_matching(/The 'force_load' feature requires/)) + endConsider extracting shared_examples for the badge/log behavior across helpers to reduce duplication.
450-504: react_component_hash: assert prepend order and logging for invalid license.Add an order check to ensure badge is prepended to
componentHtml, and mirror the logging assertion.@@ context "when Pro license is NOT installed and force_load is true" do subject(:react_app) { react_component_hash("App", props: props, force_load: true) } @@ - it "adds badge to componentHtml" do - expect(react_app["componentHtml"]).to include(badge_html_string) - end + it "adds badge to componentHtml" do + expect(react_app["componentHtml"]).to include(badge_html_string) + end + + it "prepends the badge before the component HTML" do + html = react_app["componentHtml"] + expect(html.index(badge_html_string)).to be < html.index("<div>Test</div>") + end + + it "logs a warning" do + react_app + expect(Rails.logger).to have_received(:warn).with(a_string_matching(/The 'force_load' feature requires/)) + end
529-572: redux_store: add prepend order, global flag, and defer-path coverage.
- Verify badge is prepended ahead of the store script.
- Add a global
force_loadtrue context similar to react_component coverage.- Add a
defer: truecase to assert that no badge is injected on deferred registration.@@ context "when Pro license is NOT installed and force_load is true" do subject(:store) { redux_store("reduxStore", props: props, force_load: true) } @@ it { is_expected.to include(badge_html_string) } + + it "prepends the badge before the store script" do + html = store.to_s + expect(html.index(badge_html_string)).to be < html.index('<script type="application/json" data-js-react-on-rails-store="reduxStore"') + end @@ context "when Pro license IS installed and force_load is true" do subject(:store) { redux_store("reduxStore", props: props, force_load: true) } @@ it { is_expected.not_to include(badge_html_string) } end + + context "when Pro license is NOT installed and global force_load is true" do + subject(:store) { redux_store("reduxStore", props: props) } + before { allow(ReactOnRails::Utils).to receive(:react_on_rails_pro_licence_valid?).and_return(false) } + around do |example| + ReactOnRails.configure { |config| config.force_load = true } + example.run + ReactOnRails.configure { |config| config.force_load = false } + end + it { is_expected.to include(badge_html_string) } + it "logs a warning" do + store + expect(Rails.logger).to have_received(:warn).with(a_string_matching(/The 'force_load' feature requires/)) + end + end + + context "when Pro license is NOT installed with defer: true" do + subject(:store) { redux_store("reduxStore", props: props, force_load: true, defer: true) } + before { allow(ReactOnRails::Utils).to receive(:react_on_rails_pro_licence_valid?).and_return(false) } + it { is_expected.not_to include(badge_html_string) } + end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/react_on_rails/helper.rb(5 hunks)lib/react_on_rails/utils.rb(2 hunks)spec/dummy/spec/helpers/react_on_rails_helper_spec.rb(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/react_on_rails/helper.rb
🧰 Additional context used
📓 Path-based instructions (2)
lib/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
Ruby source code must adhere to RuboCop rules (project uses RuboCop for Ruby linting)
Files:
lib/react_on_rails/utils.rb
spec/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
Ruby tests should be written for RSpec (project uses RSpec for Ruby testing)
Files:
spec/dummy/spec/helpers/react_on_rails_helper_spec.rb
🧠 Learnings (4)
📓 Common learnings
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
Applied to files:
lib/react_on_rails/utils.rbspec/dummy/spec/helpers/react_on_rails_helper_spec.rb
📚 Learning: 2025-04-09T12:56:10.756Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1696
File: node_package/src/RSCPayloadContainer.ts:0-0
Timestamp: 2025-04-09T12:56:10.756Z
Learning: In the react_on_rails codebase, RSC payloads are already stringified using `JSON.stringify()` before being processed by the `escapeScript` function, which handles escaping of special characters. The function only needs to handle specific HTML markers like comments and closing script tags.
Applied to files:
lib/react_on_rails/utils.rb
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation is handled in deeper level calls of the React on Rails Pro codebase, so it doesn't need to be validated again in the `rsc_payload_react_component` helper method.
Applied to files:
lib/react_on_rails/utils.rb
🧬 Code graph analysis (1)
spec/dummy/spec/helpers/react_on_rails_helper_spec.rb (2)
lib/react_on_rails/helper.rb (3)
react_component(56-94)react_component_hash(211-242)redux_store(257-273)lib/react_on_rails/react_component/render_options.rb (2)
props(26-28)force_load(98-100)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: dummy-app-integration-tests (newest)
- GitHub Check: dummy-app-integration-tests (oldest)
- GitHub Check: rspec-package-tests (oldest)
- GitHub Check: rspec-package-tests (newest)
- GitHub Check: claude-review
- GitHub Check: build
🔇 Additional comments (2)
spec/dummy/spec/helpers/react_on_rails_helper_spec.rb (2)
24-27: Good default stub to isolate tests.Stubbing
react_on_rails_pro_licence_valid?to true by default reduces noise for unrelated specs. LGTM.
507-507: Subject defaults force_load: true – confirm no unintended coupling.This default is fine, but ensure other contexts override as needed so badge expectations don’t depend on implicit state.
| def self.react_on_rails_pro_licence_valid? | ||
| return @react_on_rails_pro_licence_valid if defined?(@react_on_rails_pro_licence_valid) | ||
|
|
||
| @react_on_rails_pro_licence_valid = begin | ||
| return false unless react_on_rails_pro? | ||
|
|
||
| # Maintain compatibility with legacy versions of React on Rails Pro: | ||
| # Earlier releases did not require license validation, as they were distributed as private gems. | ||
| # This check ensures that the method works correctly regardless of the installed version. | ||
| return true unless ReactOnRailsPro::Utils.respond_to?(:licence_valid?) | ||
|
|
||
| ReactOnRailsPro::Utils.licence_valid? | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Memoization bug: early returns bypass the cache assignment.
return inside the begin block exits the method before assigning @react_on_rails_pro_licence_valid, so calls where Pro isn’t installed (or legacy without licence_valid?) won’t be cached. Refactor to compute and assign without early returns.
Apply:
- def self.react_on_rails_pro_licence_valid?
- return @react_on_rails_pro_licence_valid if defined?(@react_on_rails_pro_licence_valid)
-
- @react_on_rails_pro_licence_valid = begin
- return false unless react_on_rails_pro?
-
- # Maintain compatibility with legacy versions of React on Rails Pro:
- # Earlier releases did not require license validation, as they were distributed as private gems.
- # This check ensures that the method works correctly regardless of the installed version.
- return true unless ReactOnRailsPro::Utils.respond_to?(:licence_valid?)
-
- ReactOnRailsPro::Utils.licence_valid?
- end
- end
+ def self.react_on_rails_pro_licence_valid?
+ return @react_on_rails_pro_licence_valid if defined?(@react_on_rails_pro_licence_valid)
+
+ @react_on_rails_pro_licence_valid =
+ if react_on_rails_pro?
+ # Maintain compatibility with legacy versions of React on Rails Pro:
+ # Earlier releases did not require license validation, as they were distributed as private gems.
+ # This check ensures that the method works correctly regardless of the installed version.
+ if ReactOnRailsPro::Utils.respond_to?(:licence_valid?)
+ ReactOnRailsPro::Utils.licence_valid?
+ else
+ true
+ end
+ else
+ false
+ end
+ endOptionally add a test-only reset helper to avoid cross‑example leakage:
def self.reset_react_on_rails_pro_licence_cache!
remove_instance_variable(:@react_on_rails_pro_licence_valid) if instance_variable_defined?(:@react_on_rails_pro_licence_valid)
end🤖 Prompt for AI Agents
In lib/react_on_rails/utils.rb around lines 216 to 229, the method currently
returns early inside the begin block which prevents assigning the computed value
to @react_on_rails_pro_licence_valid; refactor so you compute a local result (no
early returns), assign it to @react_on_rails_pro_licence_valid, and then return
it — specifically: check react_on_rails_pro? and whether ReactOnRailsPro::Utils
responds_to?(:licence_valid?) using conditional expressions that set a local
variable (true/false or the call to licence_valid?), assign that variable to the
instance variable, and return it; optionally add the suggested
reset_react_on_rails_pro_licence_cache! helper to clear the cache in tests by
removing the instance variable if defined.
Summary
Remove this paragraph and provide a general description of the code changes in your pull
request... were there any bugs you had fixed? If so, mention them. If
these bugs have open GitHub issues, be sure to tag them here as well,
to keep the conversation linked together.
Pull Request checklist
Remove this line after checking all the items here. If the item is not applicable to the PR, both check it out and wrap it by
~.Add the CHANGELOG entry at the top of the file.
Other Information
Remove this paragraph and mention any other important and relevant information such as benchmarks.
This change is
Summary by CodeRabbit