Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions react_on_rails/lib/generators/react_on_rails/pro_setup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,20 @@ def setup_pro
puts Rainbow("=" * 80).cyan
end

# Check if Pro gem is missing.
#
# @param force [Boolean] When true, always performs the check.
# When false (default), only checks if Pro is required (use_pro? returns true).
# Use force: true in standalone generators where Pro is always required.
# @return [Boolean] true if Pro gem is missing
# Check if Pro gem is missing. Attempts auto-install via bundle add.
# @param force [Boolean] When true, always checks (default: only if use_pro?).
# @return [Boolean] true if Pro gem is missing and could not be installed
def missing_pro_gem?(force: false)
return false unless force || use_pro?
return false if pro_gem_installed?

# Detect context: install_generator defines :pro/:rsc options, standalone generators don't
# Try auto-installing (similar to ensure_shakapacker_in_gemfile in install_generator)
puts Rainbow("📝 Adding react_on_rails_pro to Gemfile...").yellow
if Bundler.with_unbundled_env { system("bundle add react_on_rails_pro --strict") }
@pro_gem_installed = nil
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memoization invalidation is incorrect

Setting @pro_gem_installed = nil does not truly clear the memoization in pro_gem_installed? (defined in generator_helper.rb). The guard clause return @pro_gem_installed if defined?(@pro_gem_installed) checks defined?, which returns truthy even when the variable is nil. So subsequent calls to pro_gem_installed? would return nil (falsy), meaning the gem appears uninstalled even after a successful bundle add.

In practice this is low-risk because missing_pro_gem? is only called once per generator run, but the intent is clearly to allow re-detection. Use remove_instance_variable to fully clear the memo, or simply set it to true since the install just succeeded:

Suggested change
@pro_gem_installed = nil
remove_instance_variable(:@pro_gem_installed) if defined?(@pro_gem_installed)

return false
Comment on lines +58 to +60
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's find and read the pro_setup.rb file to see the relevant code
fd "pro_setup.rb" --type f

Repository: shakacode/react_on_rails

Length of output: 125


🏁 Script executed:

# Read the pro_setup.rb file to understand the context of lines 58-60
cat -n react_on_rails/lib/generators/react_on_rails/pro_setup.rb | head -100

Repository: shakacode/react_on_rails

Length of output: 4504


🏁 Script executed:

# Also search for the pro_gem_installed? method definition
rg -n "def pro_gem_installed?" -A 5 --type ruby

Repository: shakacode/react_on_rails

Length of output: 687


🏁 Script executed:

# And find the missing_pro_gem? method to understand the full context
rg -n "def missing_pro_gem?" -A 20 --type ruby

Repository: shakacode/react_on_rails

Length of output: 2281


🏁 Script executed:

#!/bin/bash
# Find all callers of missing_pro_gem?
rg -n "missing_pro_gem?" --type ruby -B 2 -A 2

Repository: shakacode/react_on_rails

Length of output: 7403


🏁 Script executed:

#!/bin/bash
# Also check what happens after missing_pro_gem? is called - look at the calling methods
rg -n "missing_pro_gem\?" -B 5 -A 5 --type ruby

Repository: shakacode/react_on_rails

Length of output: 12125


🏁 Script executed:

#!/bin/bash
# Check the generator flow - see if pro_gem_installed? is called after missing_pro_gem? returns
rg -n "pro_gem_installed?" -B 3 -A 3 lib/generators/react_on_rails/install_generator.rb

Repository: shakacode/react_on_rails

Length of output: 158


🏁 Script executed:

#!/bin/bash
# Check if pro_gem_installed? is called in the setup flow or initialization
rg -n "pro_gem_installed?" lib/generators/react_on_rails/ --type ruby

Repository: shakacode/react_on_rails

Length of output: 138


🏁 Script executed:

#!/bin/bash
# Find the correct path for install_generator
find . -name "install_generator.rb" -type f

Repository: shakacode/react_on_rails

Length of output: 135


🏁 Script executed:

#!/bin/bash
# Search for pro_gem_installed? calls in the codebase
rg -n "pro_gem_installed?" --type ruby -l

Repository: shakacode/react_on_rails

Length of output: 190


🏁 Script executed:

#!/bin/bash
# Check all calls to pro_gem_installed? in the relevant files
rg -n "pro_gem_installed?" react_on_rails/lib/generators/react_on_rails/pro_setup.rb react_on_rails/lib/generators/react_on_rails/generator_helper.rb -B 2 -A 2

Repository: shakacode/react_on_rails

Length of output: 2471


🏁 Script executed:

#!/bin/bash
# Check if install_generator has any references to pro_gem_installed? or missing_pro_gem?
rg -n "pro_gem_installed?|missing_pro_gem?" ./react_on_rails/lib/generators/react_on_rails/install_generator.rb -B 2 -A 2

Repository: shakacode/react_on_rails

Length of output: 328


🏁 Script executed:

#!/bin/bash
# Check where installation_prerequisites_met? is called to understand generator flow
rg -n "installation_prerequisites_met?" ./react_on_rails/lib/generators/react_on_rails/install_generator.rb -B 3 -A 3

Repository: shakacode/react_on_rails

Length of output: 855


🏁 Script executed:

#!/bin/bash
# Check if pro_setup is called and if it uses pro_gem_installed?
rg -n "setup_pro|include ProSetup" ./react_on_rails/lib/generators/react_on_rails/install_generator.rb -B 2 -A 2

Repository: shakacode/react_on_rails

Length of output: 193


🏁 Script executed:

#!/bin/bash
# Look at the perform method to understand the full generator execution flow
grep -n "def perform" ./react_on_rails/lib/generators/react_on_rails/install_generator.rb -A 20

Repository: shakacode/react_on_rails

Length of output: 50


🏁 Script executed:

#!/bin/bash
# Check the invoke_generators method to see what happens after prerequisites are met
rg -n "def invoke_generators" ./react_on_rails/lib/generators/react_on_rails/install_generator.rb -A 15

Repository: shakacode/react_on_rails

Length of output: 894


🏁 Script executed:

#!/bin/bash
# Check if any template or subsequent operation calls pro_gem_installed? after prerequisites check
rg -n "setup_pro" ./react_on_rails/lib/generators/react_on_rails/install_generator.rb -B 2 -A 2

Repository: shakacode/react_on_rails

Length of output: 50


🏁 Script executed:

#!/bin/bash
# Search for all calls to setup_pro in the entire generators directory
rg -n "setup_pro" ./react_on_rails/lib/generators/react_on_rails/ -B 2 -A 2

Repository: shakacode/react_on_rails

Length of output: 1206


🏁 Script executed:

#!/bin/bash
# Check the base generator to see if it calls setup_pro
find ./react_on_rails/lib/generators -name "*.rb" -exec grep -l "setup_pro" {} \;

Repository: shakacode/react_on_rails

Length of output: 191


🏁 Script executed:

#!/bin/bash
# Check the pro_generator prerequisites_met? method
rg -n "def prerequisites_met?" ./react_on_rails/lib/generators/react_on_rails/pro_generator.rb -A 5

Repository: shakacode/react_on_rails

Length of output: 288


🏁 Script executed:

#!/bin/bash
# Check how pro_generator is invoked from install_generator
rg -n "invoke.*pro_generator\|:pro" ./react_on_rails/lib/generators/react_on_rails/install_generator.rb -B 2 -A 2

Repository: shakacode/react_on_rails

Length of output: 50


🏁 Script executed:

#!/bin/bash
# Search for invoke calls in install_generator that might trigger pro_generator
rg -n "invoke.*pro\|--pro" ./react_on_rails/lib/generators/react_on_rails/install_generator.rb -B 1 -A 1

Repository: shakacode/react_on_rails

Length of output: 50


🏁 Script executed:

#!/bin/bash
# Check base generator to see how it handles pro flag
rg -n "def perform|setup_pro\|--pro" ./react_on_rails/lib/generators/react_on_rails/base_generator.rb -B 2 -A 5

Repository: shakacode/react_on_rails

Length of output: 50


🏁 Script executed:

#!/bin/bash
# Find all generator files to understand the structure
find ./react_on_rails/lib/generators -name "*.rb" -type f | head -20

Repository: shakacode/react_on_rails

Length of output: 1562


🏁 Script executed:

#!/bin/bash
# Search for how Pro is set up in the invoke flow
rg -n "options\[:pro\]|options\[:rsc\]" ./react_on_rails/lib/generators/react_on_rails/install_generator.rb -B 2 -A 2

Repository: shakacode/react_on_rails

Length of output: 50


🏁 Script executed:

#!/bin/bash
# Search for pro flag handling in install_generator
cat -n ./react_on_rails/lib/generators/react_on_rails/install_generator.rb | grep -A 5 -B 5 "pro\|rsc" | head -100

Repository: shakacode/react_on_rails

Length of output: 4833


🏁 Script executed:

#!/bin/bash
# Look at the base generator to see how it handles pro/rsc
cat -n ./react_on_rails/lib/generators/react_on_rails/base_generator.rb | head -150

Repository: shakacode/react_on_rails

Length of output: 6916


🏁 Script executed:

#!/bin/bash
# Check test files to see if they reuse generator instances
rg -n "missing_pro_gem?" ./react_on_rails/spec/react_on_rails/generators/ -B 3 -A 5

Repository: shakacode/react_on_rails

Length of output: 8195


Reset memoization correctly by removing the instance variable instead of setting it to nil.

After a successful bundle add react_on_rails_pro, the code sets @pro_gem_installed = nil to clear the memoization. However, pro_gem_installed? guards with return @pro_gem_installed if defined?(@pro_gem_installed). Since nil keeps the variable defined?, subsequent calls return nil rather than re-evaluating whether the gem is installed.

Proposed fix
         if Bundler.with_unbundled_env { system("bundle add react_on_rails_pro --strict") }
-          `@pro_gem_installed` = nil
+          remove_instance_variable(:`@pro_gem_installed`) if defined?(`@pro_gem_installed`)
           return false
         end

Alternatively, set @pro_gem_installed = true since the bundle add succeeded.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if Bundler.with_unbundled_env { system("bundle add react_on_rails_pro --strict") }
@pro_gem_installed = nil
return false
if Bundler.with_unbundled_env { system("bundle add react_on_rails_pro --strict") }
remove_instance_variable(:`@pro_gem_installed`) if defined?(`@pro_gem_installed`)
return false
🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In `@react_on_rails/lib/generators/react_on_rails/pro_setup.rb` around lines 58 -
60, The memoization reset is incorrect: in the block that runs `bundle add
react_on_rails_pro` (inside the `pro_gem_installed?` flow) remove the memoized
instance variable rather than assigning nil so subsequent calls will
re-evaluate; specifically, after a successful Bundler.with_unbundled_env system
call, call remove_instance_variable(:`@pro_gem_installed`) (or alternatively set
`@pro_gem_installed = true`) instead of `@pro_gem_installed = nil` so the
`defined?(`@pro_gem_installed`)` guard in `pro_gem_installed?` behaves correctly.

end
Comment on lines +57 to +61
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Existing tests will break — auto-install not stubbed and assertions outdated

Two issues with existing specs:

  1. Tests will attempt a real bundle add call. The specs in install_generator_spec.rb (lines 1113-1138) and pro_generator_spec.rb (lines 31-46) stub Gem.loaded_specs and gem_in_lockfile? but do not stub Bundler.with_unbundled_env / system. When missing_pro_gem? is called in tests, it will now attempt a real bundle add react_on_rails_pro --strict command, which will either fail (making the test pass accidentally by falling through to the error path) or hang/produce side effects in CI.

  2. install_generator_spec.rb:1124 asserts expect(error_text).to include("Try Pro free!"). This string was removed from the error message in this PR, so the test will fail.

These tests need to be updated to stub the auto-install attempt and remove the outdated assertion. For example:

before do
  allow(Bundler).to receive(:with_unbundled_env).and_return(false)
end


context_line = if options.key?(:pro) || options.key?(:rsc)
flag = options[:rsc] ? "--rsc" : "--pro"
"You specified #{flag}, which requires the react_on_rails_pro gem."
Expand All @@ -65,16 +68,15 @@ def missing_pro_gem?(force: false)
end

GeneratorMessages.add_error(<<~MSG.strip)
🚫 React on Rails Pro gem is not installed.
🚫 Failed to auto-install react_on_rails_pro gem.

#{context_line}

Add to your Gemfile:
Please add manually to your Gemfile:
gem 'react_on_rails_pro', '>= 16.3.0'

Then run: bundle install

Try Pro free! Email justin@shakacode.com for an evaluation license.
More info: https://www.shakacode.com/react-on-rails-pro/
MSG
true
Expand Down
Loading