Skip to content

Conversation

@alexeyr-ci2
Copy link
Collaborator

@alexeyr-ci2 alexeyr-ci2 commented Jul 2, 2025

Summary

There were incorrect warnings when using RC versions with Popmenu:

**WARNING** ReactOnRails: Your node package version for react-on-rails contains a ^ or ~
Detected: 15.0.0-rc.1
     gem: 15.0.0.rc.1
Ensure the installed version of the gem is the same as the version of
your installed node package. Do not use >= or ~> in your Gemfile for react_on_rails.
Do not use ^ or ~ in your package.json for react-on-rails.

This PR fixes them.

Also adds warnings when e.g. using 15.0.0-rc.0 Node package with 15.0.0.rc.1 gem, since those may have breaking changes between them.

Pull Request checklist

  • Add/update test to cover these changes
  • [ ] Update documentation
  • Update CHANGELOG file

This change is Reviewable

Summary by CodeRabbit

  • Bug Fixes

    • Resolved incorrect warnings about version mismatches when using pre-release versions of React on Rails.
    • Improved detection and messaging for non-exact version specifications in configuration files.
    • Enhanced version parsing and comparison accuracy for better compatibility checks.
    • Updated compatibility checks and error messages related to Shakapacker version requirements.
  • Documentation

    • Updated changelog to document the fix for version mismatch warnings and improved guidance on version specification requirements.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 2, 2025

Walkthrough

This change updates the version checking logic in React on Rails to better handle pre-release and non-exact version specifications. It refines regular expressions, improves version comparison to include pre-release identifiers, updates warning messages, and revises related tests. Additionally, it simplifies version parsing and comparison in packer utilities and updates version constants from arrays to strings.

Changes

File(s) Change Summary
CHANGELOG.md Added an entry documenting the fix for invalid and missing warnings related to pre-release version mismatches.
lib/react_on_rails/version_checker.rb Replaced version parsing regex with comprehensive pattern; extended version comparison to full version parts including pre-release; updated detection of non-exact version specifiers; renamed methods for clarity; refined warning messages.
spec/react_on_rails/version_checker_spec.rb Updated tests to reflect method renaming (major_minor_patchparts), adjusted expectations for version parts, and revised warning message checks.
lib/react_on_rails/packer_utils.rb Modified version parsing to include optional pre-release parts using updated regex; simplified semver string conversion; improved version display in warnings; removed unused helper method; changed version comparisons to string-based.
lib/react_on_rails/configuration.rb Changed version requirement argument from array to string in Shakapacker version check.
lib/react_on_rails/packs_generator.rb Changed MINIMUM_SHAKAPACKER_VERSION constant from array to string.
spec/react_on_rails/configuration_spec.rb Updated test mocks to use string version requirements instead of arrays for Shakapacker version checks.
spec/react_on_rails/packer_utils_spec.rb Updated tests to use string version requirements instead of arrays for Shakapacker version checks; adjusted mocked version strings.

Sequence Diagram(s)

sequenceDiagram
    participant Gem as ReactOnRails Gem
    participant NodePkg as Node Package
    participant VersionChecker
    participant Logger

    Gem->>VersionChecker: Provide gem version
    NodePkg->>VersionChecker: Provide node package version
    VersionChecker->>VersionChecker: Parse versions (incl. pre-release)
    VersionChecker->>VersionChecker: Compare full version parts
    alt Non-exact or mismatched version
        VersionChecker->>Logger: Log warning or error
    end
Loading

Possibly related PRs

Suggested reviewers

  • alexeyr-ci2
  • Judahmeek

Poem

A bunny hopped through code so neat,
Fixing versions—no small feat!
Pre-release quirks now in the past,
Warnings and checks are built to last.
With regex sharp and logic bright,
The version checker’s future’s right!
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 813f0a8 and 767edc8.

📒 Files selected for processing (1)
  • lib/react_on_rails/packer_utils.rb (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/react_on_rails/packer_utils.rb
⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: dummy-app-integration-tests (oldest)
  • GitHub Check: dummy-app-integration-tests (newest)
  • GitHub Check: build
  • GitHub Check: rspec-package-tests (newest)
  • GitHub Check: rspec-package-tests (oldest)
✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@alexeyr-ci alexeyr-ci force-pushed the alexeyr/fix-version-checker branch 2 times, most recently from 87e20ea to eb6a68b Compare July 2, 2025 12:39
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 98fcbc9 and eb6a68b.

📒 Files selected for processing (3)
  • CHANGELOG.md (1 hunks)
  • lib/react_on_rails/version_checker.rb (5 hunks)
  • spec/react_on_rails/version_checker_spec.rb (9 hunks)
🧰 Additional context used
🧠 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.
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
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.
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1644
File: node_package/src/ReactOnRailsRSC.ts:87-87
Timestamp: 2024-12-12T13:07:09.929Z
Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
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.
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
CHANGELOG.md (5)
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1644
File: node_package/src/ReactOnRailsRSC.ts:87-87
Timestamp: 2024-12-12T13:07:09.929Z
Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
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.
spec/react_on_rails/version_checker_spec.rb (10)
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: spec/react_on_rails/utils_spec.rb:218-218
Timestamp: 2025-02-13T14:29:49.267Z
Learning: In RSpec tests, prefer using local variables over constants within test blocks to avoid constant redefinition warnings and maintain better test isolation.
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1644
File: node_package/src/ReactOnRailsRSC.ts:87-87
Timestamp: 2024-12-12T13:07:09.929Z
Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.
Learnt from: justin808
PR: shakacode/react_on_rails#1622
File: spec/dummy/spec/rake/assets_precompile_rake_spec.rb:12-12
Timestamp: 2024-10-08T20:53:47.076Z
Learning: When stubbing environment variables in RSpec tests, use `before` and `after` hooks to ensure that the original values are restored after the tests, preventing any side effects on other tests. Example provided by justin808:
```ruby
describe "My test" do
  before do
    @original_value = ENV["VARIABLE_NAME"]
    allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_return("stubbed_value")
  end

  after do
    allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_call_original
    ENV["VARIABLE_NAME"] = @original_value
  end

  it "tests something" do
    # Your test code here
  end
end
```
This practice ensures test isolation and reliability.
Learnt from: justin808
PR: shakacode/react_on_rails#1622
File: spec/dummy/spec/rake/assets_precompile_rake_spec.rb:12-12
Timestamp: 2024-06-27T05:22:46.156Z
Learning: When stubbing environment variables in RSpec tests, use `before` and `after` hooks to ensure that the original values are restored after the tests, preventing any side effects on other tests. Example provided by justin808:
```ruby
describe "My test" do
  before do
    @original_value = ENV["VARIABLE_NAME"]
    allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_return("stubbed_value")
  end

  after do
    allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_call_original
    ENV["VARIABLE_NAME"] = @original_value
  end

  it "tests something" do
    # Your test code here
  end
end
```
This practice ensures test isolation and reliability.
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
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.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
Learnt from: theforestvn88
PR: shakacode/react_on_rails#1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-10-08T20:53:47.076Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.
lib/react_on_rails/version_checker.rb (5)
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1644
File: node_package/src/ReactOnRailsRSC.ts:87-87
Timestamp: 2024-12-12T13:07:09.929Z
Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
🪛 RuboCop (1.75.5)
lib/react_on_rails/version_checker.rb

[warning] 59-59: Do not chain ordinary method call after safe navigation operator.

(Lint/SafeNavigationChain)

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: rspec-package-tests (newest)
  • GitHub Check: build
🔇 Additional comments (15)
lib/react_on_rails/version_checker.rb (7)

9-10: Excellent regex improvement for handling both SemVer and RubyGems pre-release formats.

The updated regex properly handles pre-release identifiers with both dash (SemVer) and dot (RubyGems) separators, which directly addresses the PR objective of fixing invalid warnings for RC versions.


27-27: Good update to use full version parts for comparison.

Changing from major_minor_patch to parts enables proper comparison of pre-release versions, which was the core issue being fixed.


37-38: Improved warning message clarity.

The updated messages provide better guidance by explicitly mentioning package.json and using consistent capitalization.


44-44: Consistent capitalization improvement.

Capitalizing "Node package" maintains consistency with other warning messages in the codebase.


49-50: Clearer warning message structure.

The reformatted message improves readability while maintaining the same essential information.


97-97: More comprehensive wildcard detection logic.

The updated logic properly handles npm's various dependency specification formats including ranges (" - ") and alternatives (" || "), which is more accurate than the previous regex-only approach.


107-116: Well-implemented method rename with enhanced functionality.

The rename from major_minor_patch to parts accurately reflects the expanded functionality, and the implementation properly handles both exact versions and pre-release identifiers while maintaining error handling for invalid formats.

spec/react_on_rails/version_checker_spec.rb (7)

21-21: Correct test double update with pre-release example.

The test properly demonstrates the new functionality with a beta pre-release version that includes the fourth part in the parts array.


35-35: Consistent test double update for semver wildcard scenario.

The test maintains the same logic while using the renamed parts attribute.


42-42: Updated warning message expectation matches implementation.

The regex pattern correctly matches the updated warning message format from the main code.


50-50: Consistent test updates for version mismatch scenarios.

All warning message expectations have been updated to match the new "Node package" capitalization, maintaining test accuracy.

Also applies to: 57-57, 65-65, 72-72, 80-80, 87-87


95-95: Correct expectation for local path scenario.

Setting parts: nil for local paths is consistent with the implementation where the parts method returns nil for local paths.


119-119: Test helper method updated consistently.

The double_package_version helper method has been updated to use parts instead of major_minor_patch, maintaining consistency across all tests.

Also applies to: 123-123


190-192: All describe blocks updated to test the renamed method.

The test descriptions and expectations have been consistently updated from #major_minor_patch to #parts, properly testing the new method interface.

Also applies to: 206-208, 222-224, 238-240, 254-256, 270-272

CHANGELOG.md (1)

26-29: Changelog entry looks good

The new “Fixed” bullet follows the existing style conventions and links back to the PR correctly.

Comment on lines 58 to 60
def gem_version_parts
gem_version.match(VERSION_PARTS_REGEX)&.captures.compact
end
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

Fix the safe navigation chaining issue.

The RuboCop warning is valid - chaining .captures.compact after safe navigation could cause issues if match returns nil.

Apply this diff to handle the potential nil case properly:

-def gem_version_parts
-  gem_version.match(VERSION_PARTS_REGEX)&.captures.compact
-end
+def gem_version_parts
+  match = gem_version.match(VERSION_PARTS_REGEX)
+  match ? match.captures.compact : []
+end
📝 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
def gem_version_parts
gem_version.match(VERSION_PARTS_REGEX)&.captures.compact
end
def gem_version_parts
match = gem_version.match(VERSION_PARTS_REGEX)
match ? match.captures.compact : []
end
🧰 Tools
🪛 RuboCop (1.75.5)

[warning] 59-59: Do not chain ordinary method call after safe navigation operator.

(Lint/SafeNavigationChain)

🤖 Prompt for AI Agents
In lib/react_on_rails/version_checker.rb around lines 58 to 60, the method
gem_version_parts uses safe navigation on match but then calls .captures.compact
directly, which can cause errors if match returns nil. Refactor the code to
first assign the result of gem_version.match(VERSION_PARTS_REGEX) to a variable,
check if it is nil, and only then call .captures.compact on it to safely handle
the nil case without chaining methods after safe navigation.

@alexeyr-ci alexeyr-ci force-pushed the alexeyr/fix-version-checker branch from eb6a68b to c4d953a Compare July 2, 2025 12:46
@alexeyr-ci alexeyr-ci force-pushed the alexeyr/fix-version-checker branch from c4d953a to 4ca3592 Compare July 2, 2025 12:50
match = shakapacker_version.match(ReactOnRails::VersionChecker::MAJOR_MINOR_PATCH_VERSION_REGEX)

@shakapacker_version_as_array = [match[1].to_i, match[2].to_i, match[3].to_i]
Gem::Version.new(shakapacker_version).segments
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why don’t we use the VERSION_PARTS_REGEX to extract the parts instead? Gem::Version#segments can return different results from the regex parts in certain cases.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, please check the semver_to_string function at lib/react_on_rails/packer_utils.rb and check if it needs updating.

Copy link
Collaborator

@AbanoubGhadban AbanoubGhadban left a comment

Choose a reason for hiding this comment

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

LGTM, I just left one comment

@alexeyr-ci alexeyr-ci force-pushed the alexeyr/fix-version-checker branch from 63760be to 9c74cfa Compare July 4, 2025 13:23
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (1)
lib/react_on_rails/packer_utils.rb (1)

60-65: Great improvements to version parsing with proper memoization.

The addition of memoization and the use of VERSION_PARTS_REGEX from VersionChecker addresses previous feedback. The enhanced handling of pre-release versions (match[4]) is well-documented and correctly implemented.

Consider adding an empty line after the guard clause for better readability:

 def self.shakapacker_version_as_array
   return @shakapacker_version_as_array if defined?(@shakapacker_version_as_array)
+
   match = shakapacker_version.match(ReactOnRails::VersionChecker::VERSION_PARTS_REGEX)
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 01debc9 and 813f0a8.

📒 Files selected for processing (5)
  • lib/react_on_rails/configuration.rb (1 hunks)
  • lib/react_on_rails/packer_utils.rb (4 hunks)
  • lib/react_on_rails/packs_generator.rb (1 hunks)
  • spec/react_on_rails/configuration_spec.rb (2 hunks)
  • spec/react_on_rails/packer_utils_spec.rb (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • lib/react_on_rails/packs_generator.rb
🧰 Additional context used
🧠 Learnings (5)
📓 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.
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
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.
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1644
File: node_package/src/ReactOnRailsRSC.ts:87-87
Timestamp: 2024-12-12T13:07:09.929Z
Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
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.
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: spec/react_on_rails/utils_spec.rb:218-218
Timestamp: 2025-02-13T14:29:49.267Z
Learning: In RSpec tests, prefer using local variables over constants within test blocks to avoid constant redefinition warnings and maintain better test isolation.
spec/react_on_rails/configuration_spec.rb (12)
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.
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: spec/react_on_rails/utils_spec.rb:218-218
Timestamp: 2025-02-13T14:29:49.267Z
Learning: In RSpec tests, prefer using local variables over constants within test blocks to avoid constant redefinition warnings and maintain better test isolation.
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
Learnt from: theforestvn88
PR: shakacode/react_on_rails#1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-10-08T20:53:47.076Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.
Learnt from: theforestvn88
PR: shakacode/react_on_rails#1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-07-27T10:08:35.868Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.
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.
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1644
File: node_package/src/ReactOnRailsRSC.ts:87-87
Timestamp: 2024-12-12T13:07:09.929Z
Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
Learnt from: justin808
PR: shakacode/react_on_rails#1622
File: spec/dummy/spec/rake/assets_precompile_rake_spec.rb:12-12
Timestamp: 2024-10-08T20:53:47.076Z
Learning: When stubbing environment variables in RSpec tests, use `before` and `after` hooks to ensure that the original values are restored after the tests, preventing any side effects on other tests. Example provided by justin808:
```ruby
describe "My test" do
  before do
    @original_value = ENV["VARIABLE_NAME"]
    allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_return("stubbed_value")
  end

  after do
    allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_call_original
    ENV["VARIABLE_NAME"] = @original_value
  end

  it "tests something" do
    # Your test code here
  end
end
```
This practice ensures test isolation and reliability.
Learnt from: justin808
PR: shakacode/react_on_rails#1622
File: spec/dummy/spec/rake/assets_precompile_rake_spec.rb:12-12
Timestamp: 2024-06-27T05:22:46.156Z
Learning: When stubbing environment variables in RSpec tests, use `before` and `after` hooks to ensure that the original values are restored after the tests, preventing any side effects on other tests. Example provided by justin808:
```ruby
describe "My test" do
  before do
    @original_value = ENV["VARIABLE_NAME"]
    allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_return("stubbed_value")
  end

  after do
    allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_call_original
    ENV["VARIABLE_NAME"] = @original_value
  end

  it "tests something" do
    # Your test code here
  end
end
```
This practice ensures test isolation and reliability.
lib/react_on_rails/configuration.rb (4)
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.
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
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.
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
spec/react_on_rails/packer_utils_spec.rb (6)
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: spec/react_on_rails/utils_spec.rb:218-218
Timestamp: 2025-02-13T14:29:49.267Z
Learning: In RSpec tests, prefer using local variables over constants within test blocks to avoid constant redefinition warnings and maintain better test isolation.
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.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
Learnt from: justin808
PR: shakacode/react_on_rails#1622
File: spec/dummy/spec/rake/assets_precompile_rake_spec.rb:12-12
Timestamp: 2024-10-08T20:53:47.076Z
Learning: When stubbing environment variables in RSpec tests, use `before` and `after` hooks to ensure that the original values are restored after the tests, preventing any side effects on other tests. Example provided by justin808:
```ruby
describe "My test" do
  before do
    @original_value = ENV["VARIABLE_NAME"]
    allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_return("stubbed_value")
  end

  after do
    allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_call_original
    ENV["VARIABLE_NAME"] = @original_value
  end

  it "tests something" do
    # Your test code here
  end
end
```
This practice ensures test isolation and reliability.
Learnt from: justin808
PR: shakacode/react_on_rails#1622
File: spec/dummy/spec/rake/assets_precompile_rake_spec.rb:12-12
Timestamp: 2024-06-27T05:22:46.156Z
Learning: When stubbing environment variables in RSpec tests, use `before` and `after` hooks to ensure that the original values are restored after the tests, preventing any side effects on other tests. Example provided by justin808:
```ruby
describe "My test" do
  before do
    @original_value = ENV["VARIABLE_NAME"]
    allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_return("stubbed_value")
  end

  after do
    allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_call_original
    ENV["VARIABLE_NAME"] = @original_value
  end

  it "tests something" do
    # Your test code here
  end
end
```
This practice ensures test isolation and reliability.
lib/react_on_rails/packer_utils.rb (9)
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
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.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
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.
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1644
File: node_package/src/ReactOnRailsRSC.ts:87-87
Timestamp: 2024-12-12T13:07:09.929Z
Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/clientStartup.ts:18-21
Timestamp: 2025-02-13T16:50:47.848Z
Learning: In the react_on_rails module, the `reactOnRailsPageUnloaded` function in clientStartup.ts is intentionally kept private as it's only used internally as a callback for `onPageUnloaded`.
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.
🧬 Code Graph Analysis (1)
lib/react_on_rails/configuration.rb (1)
lib/react_on_rails/packer_utils.rb (1)
  • shakapacker_version_requirement_met? (67-69)
🪛 RuboCop (1.75.5)
lib/react_on_rails/packer_utils.rb

[convention] 60-60: Add empty line after guard clause.

(Layout/EmptyLineAfterGuardClause)

⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: dummy-app-integration-tests (oldest)
  • GitHub Check: dummy-app-integration-tests (newest)
  • GitHub Check: rspec-package-tests (oldest)
  • GitHub Check: rspec-package-tests (newest)
  • GitHub Check: build
🔇 Additional comments (7)
spec/react_on_rails/configuration_spec.rb (1)

382-382: LGTM! Consistent test updates for string-based version handling.

The changes from array [8, 2, 0] to string "8.2.0" properly align the tests with the updated shakapacker_version_requirement_met? method that now expects version strings instead of arrays.

Also applies to: 429-429

spec/react_on_rails/packer_utils_spec.rb (2)

8-8: LGTM! Test updated to use string-based version format.

The change from array [6, 5, 3] to string "6.5.3" correctly aligns the test with the updated version handling that now uses version strings throughout the codebase.


34-34: Minor test version adjustment looks good.

The version update from "7.7.7" to "7.0.0" maintains the test logic while using a more realistic version number.

lib/react_on_rails/configuration.rb (1)

183-183: LGTM! Production code updated for string-based version handling.

The change from array [8, 2, 0] to string "8.2.0" correctly updates the production code to use the simplified version comparison API.

lib/react_on_rails/packer_utils.rb (3)

13-13: LGTM! Consistent version string usage.

The change from array-based to string-based version checking aligns with the broader refactor and simplifies the API.


67-69: Excellent simplification of version comparison.

The direct use of Gem::Version comparison is much cleaner than the previous array-based approach and leverages Ruby's built-in semantic version handling.


174-177: Direct string interpolation improves readability.

The removal of the semver_to_string helper function and direct interpolation of version strings makes the error messages cleaner and more maintainable.

Also applies to: 185-185

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants