Skip to content
Merged
Show file tree
Hide file tree
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
8 changes: 6 additions & 2 deletions .github/workflows/lint-js-and-ruby.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,12 @@ jobs:
run: bundle check --path=vendor/bundle || bundle _2.5.9_ install --path=vendor/bundle --jobs=4 --retry=3
- name: Lint Ruby
run: bundle exec rubocop
- name: Install Node modules with Yarn for dummy app
run: cd spec/dummy && yarn install --no-progress --no-emoji --frozen-lockfile
- name: Validate RBS type signatures
run: bundle exec rake rbs:validate
# TODO: Re-enable Steep once RBS signatures are complete for all checked files
# Currently disabled because 374 type errors need to be fixed first
# - name: Run Steep type checker
# run: bundle exec rake rbs:steep
- name: Save dummy app ruby gems to cache
uses: actions/cache@v4
with:
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/pro-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,9 @@ jobs:
- name: Lint Ruby
run: bundle exec rubocop

- name: Validate RBS type signatures
run: bundle exec rake rbs:validate

- name: Lint JS
run: yarn run nps eslint

Expand Down
1 change: 1 addition & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ Naming/FileName:
Exclude:
- '**/Gemfile'
- '**/Rakefile'
- '**/Steepfile'

Layout/LineLength:
Max: 120
Expand Down
59 changes: 59 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ Pre-commit hooks automatically run:
- Check formatting without fixing: `yarn start format.listDifferent`
- **Build**: `yarn run build` (compiles TypeScript to JavaScript in packages/react-on-rails/lib)
- **Type checking**: `yarn run type-check`
- **RBS Type Checking**:
- Validate RBS signatures: `bundle exec rake rbs:validate`
- Run Steep type checker: `bundle exec rake rbs:steep`
- Run both: `bundle exec rake rbs:all`
- List RBS files: `bundle exec rake rbs:list`
- **⚠️ MANDATORY BEFORE GIT PUSH**: `bundle exec rubocop` and fix ALL violations + ensure trailing newlines
- Never run `npm` commands, only equivalent Yarn Classic ones

Expand Down Expand Up @@ -117,6 +122,60 @@ This script:
- 🔄 **Deduplicates** - removes duplicate specs
- 📁 **Auto-detects directory** - runs from spec/dummy when needed

## RBS Type Checking

React on Rails uses RBS (Ruby Signature) for static type checking with Steep.

### Quick Start

- **Validate signatures**: `bundle exec rake rbs:validate` (run by CI)
- **Run type checker**: `bundle exec rake rbs:steep` (currently disabled in CI due to existing errors)
- **Runtime checking**: Enabled by default in tests when `rbs` gem is available

### Runtime Type Checking

Runtime type checking is **ENABLED BY DEFAULT** during test runs for:
- `rake run_rspec:gem` - Unit tests
- `rake run_rspec:dummy` - Integration tests
- `rake run_rspec:dummy_no_turbolinks` - Integration tests without Turbolinks

**Performance Impact**: Runtime type checking adds overhead (typically 5-15%) to test execution. This is acceptable during development and CI as it catches type errors in actual execution paths that static analysis might miss.

To disable runtime checking (e.g., for faster test iterations during development):
```bash
DISABLE_RBS_RUNTIME_CHECKING=true rake run_rspec:gem
```

**When to disable**: Consider disabling during rapid test-driven development cycles where you're running tests frequently. Re-enable before committing to catch type violations.

### Adding Type Signatures

When creating new Ruby files in `lib/react_on_rails/`:

1. **Create RBS signature**: Add `sig/react_on_rails/filename.rbs`
2. **Add to Steepfile**: Include `check "lib/react_on_rails/filename.rb"` in Steepfile
3. **Validate**: Run `bundle exec rake rbs:validate`
4. **Type check**: Run `bundle exec rake rbs:steep`
5. **Fix errors**: Address any type errors before committing

### Files Currently Type-Checked

See `Steepfile` for the complete list. Core files include:
- `lib/react_on_rails.rb`
- `lib/react_on_rails/configuration.rb`
- `lib/react_on_rails/helper.rb`
- `lib/react_on_rails/packer_utils.rb`
- `lib/react_on_rails/server_rendering_pool.rb`
- And 5 more (see Steepfile for full list)

### Pro Package Type Checking

The Pro package has its own RBS signatures in `react_on_rails_pro/sig/`.

Validate Pro signatures:
```bash
cd react_on_rails_pro && bundle exec rake rbs:validate
```
## Changelog

- **Update CHANGELOG.md for user-visible changes only** (features, bug fixes, breaking changes, deprecations, performance improvements)
Expand Down
1 change: 1 addition & 0 deletions Gemfile.development_dependencies
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ group :development, :test do
gem "pry-rails"
gem "pry-rescue"
gem "rbs", require: false
gem "steep", require: false
gem "rubocop", "1.61.0", require: false
gem "rubocop-performance", "~>1.20.0", require: false
gem "rubocop-rspec", "~>2.26", require: false
Expand Down
25 changes: 24 additions & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ GEM
thor (>= 0.19.4, < 2.0)
tins (~> 1.6)
crass (1.0.6)
cypress-on-rails (1.19.0)
csv (3.3.5)
cypress-on-rails (1.20.0)
rack
date (3.3.4)
debug (1.9.2)
Expand All @@ -135,6 +136,7 @@ GEM
erubi (1.13.1)
execjs (2.9.1)
ffi (1.16.3)
fileutils (1.8.0)
gem-release (2.2.2)
generator_spec (0.10.0)
activesupport (>= 3.0.0)
Expand Down Expand Up @@ -349,6 +351,7 @@ GEM
sass (~> 3.5, >= 3.5.5)
sdoc (2.6.1)
rdoc (>= 5.0)
securerandom (0.4.1)
selenium-webdriver (4.9.0)
rexml (~> 3.2, >= 3.2.5)
rubyzip (>= 1.2.2, < 3.0)
Expand All @@ -375,11 +378,29 @@ GEM
sprockets (>= 3.0.0)
sqlite3 (1.7.3)
mini_portile2 (~> 2.8.0)
steep (1.9.4)
activesupport (>= 5.1)
concurrent-ruby (>= 1.1.10)
csv (>= 3.0.9)
fileutils (>= 1.1.0)
json (>= 2.1.0)
language_server-protocol (>= 3.15, < 4.0)
listen (~> 3.0)
logger (>= 1.3.0)
parser (>= 3.1)
rainbow (>= 2.2.2, < 4.0)
rbs (~> 3.8)
securerandom (>= 0.1)
strscan (>= 1.0.0)
terminal-table (>= 2, < 4)
uri (>= 0.12.0)
stringio (3.1.7)
strscan (3.1.0)
sync (0.5.0)
term-ansicolor (1.8.0)
tins (~> 1.0)
terminal-table (3.0.2)
unicode-display_width (>= 1.1.1, < 3)
thor (1.4.0)
tilt (2.3.0)
timeout (0.4.1)
Expand All @@ -399,6 +420,7 @@ GEM
uglifier (4.2.0)
execjs (>= 0.3.0, < 3)
unicode-display_width (2.5.0)
uri (1.1.1)
webdrivers (5.3.0)
nokogiri (~> 1.6)
rubyzip (>= 1.3.0)
Expand Down Expand Up @@ -458,6 +480,7 @@ DEPENDENCIES
spring (~> 4.0)
sprockets (~> 4.0)
sqlite3 (~> 1.6)
steep
turbo-rails
turbolinks
uglifier
Expand Down
51 changes: 51 additions & 0 deletions Steepfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# frozen_string_literal: true

# Steepfile - Configuration for Steep type checker
# See https://github.com/soutaro/steep for documentation
#
# IMPORTANT: This file lists only the files that are ready for type checking.
# We use a positive list (explicit check statements) rather than checking all files
# because not all files have RBS signatures yet.
#
# Files/directories intentionally excluded (no RBS signatures yet):
# - lib/generators/**/* - Rails generators (complex Rails integration)
# - lib/react_on_rails/engine.rb - Rails engine setup
# - lib/react_on_rails/doctor.rb - Diagnostic tool
# - lib/react_on_rails/locales/**/* - I18n files
# - lib/react_on_rails/props_js_builder.rb - TODO: Add signature
# - lib/react_on_rails/shakapacker/**/* - Shakapacker integration (complex)
#
# To add a new file to type checking:
# 1. Create corresponding RBS signature in sig/react_on_rails/filename.rbs
# 2. Add `check "lib/react_on_rails/filename.rb"` below
# 3. Run `bundle exec rake rbs:steep` to verify
# 4. Fix any type errors before committing

D = Steep::Diagnostic

target :lib do
# Core files with RBS signatures (alphabetically ordered for easy maintenance)
check "lib/react_on_rails.rb"
check "lib/react_on_rails/configuration.rb"
check "lib/react_on_rails/controller.rb"
check "lib/react_on_rails/git_utils.rb"
check "lib/react_on_rails/helper.rb"
check "lib/react_on_rails/packer_utils.rb"
check "lib/react_on_rails/server_rendering_pool.rb"
check "lib/react_on_rails/test_helper.rb"
check "lib/react_on_rails/utils.rb"
check "lib/react_on_rails/version_checker.rb"
Comment on lines +28 to +37
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's not have to maintain this list, just iterate over all files in lib and check for each if it has a signature in sig.


# Specify RBS signature directories
signature "sig"

# Configure libraries (gems) - Steep will load their RBS signatures
configure_code_diagnostics(D::Ruby.default)

# Library configuration - standard library gems used by checked files
library "pathname"
library "singleton"
library "logger"
library "monitor"
library "securerandom"
end
48 changes: 40 additions & 8 deletions rakelib/rbs.rake
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# frozen_string_literal: true

require "open3"
require "timeout"

# rubocop:disable Metrics/BlockLength
namespace :rbs do
desc "Validate RBS type signatures"
Expand All @@ -9,17 +12,21 @@ namespace :rbs do

puts "Validating RBS type signatures..."

# Run RBS validate
result = system("bundle exec rbs -I sig validate")
# Use Open3 for better error handling - captures stdout, stderr, and exit status separately
# This allows us to distinguish between actual validation errors and warnings
# Note: Must use bundle exec even though rake runs in bundle context because
# spawned shell commands via Open3.capture3() do NOT inherit bundle context
# Wrap in Timeout to prevent hung processes in CI environments (60 second timeout)
stdout, stderr, status = Timeout.timeout(60) do
Open3.capture3("bundle exec rbs -I sig validate")
end

case result
when true
if status.success?
puts "✓ RBS validation passed"
when false
else
puts "✗ RBS validation failed"
exit 1
when nil
puts "✗ RBS command not found or could not be executed"
puts stdout unless stdout.empty?
warn stderr unless stderr.empty?
exit 1
end
end
Expand All @@ -34,5 +41,30 @@ namespace :rbs do
sig_files.each { |f| puts " #{f}" }
puts "\nTotal: #{sig_files.count} files"
end

desc "Run Steep type checker"
task :steep do
puts "Running Steep type checker..."

# Use Open3 for better error handling
# Note: Must use bundle exec even though rake runs in bundle context because
# spawned shell commands via Open3.capture3() do NOT inherit bundle context
# Wrap in Timeout to prevent hung processes in CI environments (60 second timeout)
stdout, stderr, status = Timeout.timeout(60) do
Open3.capture3("bundle exec steep check")
end

if status.success?
puts "✓ Steep type checking passed"
else
puts "✗ Steep type checking failed"
puts stdout unless stdout.empty?
warn stderr unless stderr.empty?
exit 1
end
end

desc "Run all RBS checks (validate + steep)"
task all: %i[validate steep]
end
# rubocop:enable Metrics/BlockLength
45 changes: 42 additions & 3 deletions rakelib/run_rspec.rake
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,59 @@ namespace :run_rspec do

spec_dummy_dir = File.join("spec", "dummy")

# RBS Runtime Type Checking Configuration
# ========================================
# Runtime type checking is ENABLED BY DEFAULT when RBS gem is available
# Use ENV["DISABLE_RBS_RUNTIME_CHECKING"] = "true" to disable
#
# Coverage Strategy:
# - :gem task - Enables checking for ReactOnRails::* (direct gem unit tests)
# - :dummy tasks - Enables checking (integration tests exercise gem code paths)
# - :example tasks - No checking (examples are user-facing demo apps)
#
# Rationale per Evil Martians best practices:
# Runtime checking catches type errors in actual execution paths that static
# analysis might miss. Dummy/integration tests exercise more code paths than
# unit tests alone, providing comprehensive type safety validation.
def rbs_runtime_env_vars
return "" if ENV["DISABLE_RBS_RUNTIME_CHECKING"] == "true"

begin
require "rbs"
# Preserve existing RUBYOPT flags (e.g., --enable-yjit, --jit, warnings toggles)
# by appending RBS runtime hook instead of replacing
existing_rubyopt = ENV.fetch("RUBYOPT", nil)
rubyopt_parts = ["-rrbs/test/setup", existing_rubyopt].compact.reject(&:empty?)
"RBS_TEST_TARGET='ReactOnRails::*' RUBYOPT='#{rubyopt_parts.join(' ')}'"
rescue LoadError
# RBS not available - silently skip runtime checking
# This is expected in environments without the rbs gem
""
end
end

desc "Run RSpec for top level only"
task :gem do
run_tests_in("", rspec_args: File.join("spec", "react_on_rails"))
run_tests_in("",
rspec_args: File.join("spec", "react_on_rails"),
env_vars: rbs_runtime_env_vars)
end

desc "Runs dummy rspec with turbolinks"
task dummy: ["dummy_apps:dummy_app"] do
run_tests_in(spec_dummy_dir)
run_tests_in(spec_dummy_dir,
env_vars: rbs_runtime_env_vars)
end

desc "Runs dummy rspec without turbolinks"
task dummy_no_turbolinks: ["dummy_apps:dummy_app"] do
# Build env vars array for robustness with complex environment variables
env_vars_array = []
env_vars_array << rbs_runtime_env_vars unless rbs_runtime_env_vars.empty?
env_vars_array << "DISABLE_TURBOLINKS=TRUE"
env_vars = env_vars_array.join(" ")
run_tests_in(spec_dummy_dir,
env_vars: "DISABLE_TURBOLINKS=TRUE",
env_vars: env_vars,
command_name: "dummy_no_turbolinks")
end

Expand Down
1 change: 1 addition & 0 deletions react_on_rails_pro/Gemfile.development_dependencies
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ group :development, :test do
gem 'pry-rails' # Causes rails console to open pry. `DISABLE_PRY_RAILS=1 rails c` can still open with IRB
gem 'pry-theme' # An easy way to customize Pry colors via theme files

gem "rbs", require: false
gem "rubocop", "1.36.0", require: false
gem 'rubocop-performance', "1.15.0", require: false
gem 'rubocop-rspec', "2.13.2", require: false
Expand Down
Loading
Loading