Conversation
WalkthroughThis pull request migrates the repository from Yarn Classic to pnpm: updates CI workflows, root and package manifests, build/rake scripts, pre-commit hooks, tooling configs, and documentation; adds pnpm workspace and .npmrc settings; converts workspace dependency references to Changes
Sequence Diagram(s)sequenceDiagram
actor Dev as Developer
participant Local as Local Repo
participant PNPM as pnpm (local manager)
participant CI as GitHub Actions
participant PackageB as Dependent Package
rect rgba(220,240,255,0.8)
Note over Dev,PackageB: Before (Yarn Classic + yalc)
Dev->>Local: yarn install
Dev->>Local: modify PackageA
Dev->>Local: yarn yalc publish
Local->>PackageB: manual yalc link
PackageB->>PackageB: uses published artifact (manual cycle)
end
rect rgba(220,255,230,0.8)
Note over Dev,PackageB: After (pnpm workspaces)
Dev->>Local: pnpm install
PNPM->>Local: resolve `workspace:*` → symlink
Dev->>Local: modify PackageA
PackageB->>PNPM: uses workspace symlink (immediate)
PackageB->>PackageB: uses local changes (no publish)
end
rect rgba(255,250,220,0.6)
Note over CI,PNPM: CI flow (changed)
CI->>PNPM: setup pnpm action & pnpm install
CI->>PNPM: derive pnpm store path -> cache pnpm-store
CI->>PNPM: run pnpm --filter / -r build/test
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas to focus during review:
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (10)📓 Common learnings📚 Learning: 2025-11-25T04:54:35.286ZApplied to files:
📚 Learning: 2025-04-26T21:55:55.874ZApplied to files:
📚 Learning: 2025-11-25T04:54:35.286ZApplied to files:
📚 Learning: 2025-11-25T04:54:35.286ZApplied to files:
📚 Learning: 2025-02-12T16:38:06.537ZApplied to files:
📚 Learning: 2025-09-16T08:01:11.146ZApplied to files:
📚 Learning: 2025-11-25T04:54:35.286ZApplied to files:
📚 Learning: 2025-07-08T05:57:29.630ZApplied to files:
📚 Learning: 2025-11-25T04:54:35.286ZApplied to files:
⏰ 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). (9)
🔇 Additional comments (1)
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: Yarn Classic to pnpm MigrationThis is a well-executed migration from Yarn Classic to pnpm. The PR is comprehensive and follows best practices. Here is my detailed feedback: Critical Issue FoundCRITICAL: Release Task Still Uses Yarn (Priority: HIGH) rakelib/release.rake lines 261, 266, 279 still use yarn workspace and yarn publish commands. This will cause the release process to fail completely. Lines that need fixing:
These need to be updated to use pnpm commands:
Strengths
Testing RequirementsPer CLAUDE.md guidelines, these tests must be verified:
PR description shows install and build tested, but yalc workflow and CI need verification. Minor Suggestions
SummaryMust fix before merge: Release rake task still uses yarn commands Great work overall! Once the release rake task is fixed and testing verified, this will be ready to merge. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CONTRIBUTING.md (1)
603-606: Convert npm commands to pnpm in testing examples for consistency with the migration.Lines 603-606 and 623-627 still use
npm run build,npm pack, andnpm installwhile the rest of the file has been migrated to pnpm. SincepackageManageris set to[email protected], these should be updated:
- Line 603:
npm run build→pnpm run build- Line 603:
npx yalc publish→npx yalc publish(remains unchanged; yalc is independent)- Line 606:
npx yalc push→npx yalc push(orpnpm exec yalc pushfor consistency)- Line 606:
npm install→pnpm install- Line 624:
npm run build→pnpm run build- Line 625:
npm pack→pnpm pack- Line 627:
npm install ../path/to/...→pnpm install ../path/to/...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlreact_on_rails_pro/spec/dummy/yarn.lockis excluded by!**/yarn.lock,!**/*.lockreact_on_rails_pro/spec/execjs-compatible-dummy/yarn.lockis excluded by!**/yarn.lock,!**/*.lockreact_on_rails_pro/yarn.lockis excluded by!**/yarn.lock,!**/*.lockspec/dummy/yarn.lockis excluded by!**/yarn.lock,!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
.claude/docs/pr-splitting-strategy.md(0 hunks).claude/docs/testing-build-scripts.md(7 hunks).github/workflows/examples.yml(2 hunks).github/workflows/gem-tests.yml(0 hunks).github/workflows/integration-tests.yml(5 hunks).github/workflows/lint-js-and-ruby.yml(2 hunks).github/workflows/package-js-tests.yml(1 hunks).github/workflows/playwright.yml(2 hunks).github/workflows/pro-integration-tests.yml(13 hunks).github/workflows/pro-lint.yml(4 hunks).github/workflows/pro-test-package-and-gem.yml(5 hunks).npmrc(1 hunks)CHANGELOG.md(0 hunks)CLAUDE.md(8 hunks)CODING_AGENTS.md(0 hunks)CONTRIBUTING.md(16 hunks)TODO.md(0 hunks)package.json(2 hunks)packages/react-on-rails-pro-node-renderer/package.json(3 hunks)packages/react-on-rails-pro/package.json(2 hunks)packages/react-on-rails/package.json(1 hunks)pnpm-workspace.yaml(1 hunks)rakelib/dummy_apps.rake(1 hunks)rakelib/lint.rake(3 hunks)rakelib/node_package.rake(1 hunks)rakelib/run_rspec.rake(1 hunks)react_on_rails_pro/spec/dummy/package.json(2 hunks)react_on_rails_pro/spec/execjs-compatible-dummy/package.json(2 hunks)spec/dummy/package.json(1 hunks)
💤 Files with no reviewable changes (5)
- TODO.md
- CHANGELOG.md
- .github/workflows/gem-tests.yml
- .claude/docs/pr-splitting-strategy.md
- CODING_AGENTS.md
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS ensure files end with a newline character before every commit/push
Files:
packages/react-on-rails/package.jsonCONTRIBUTING.mdrakelib/run_rspec.rakepackage.jsonrakelib/node_package.rakerakelib/dummy_apps.rakerakelib/lint.rakepackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/dummy/package.jsonspec/dummy/package.jsonpnpm-workspace.yamlpackages/react-on-rails-pro/package.jsonreact_on_rails_pro/spec/execjs-compatible-dummy/package.jsonCLAUDE.md
**/*.{js,ts,jsx,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,ts,jsx,tsx,json,md,yml,yaml}: ALWAYS let Prettier and RuboCop handle ALL formatting - never manually format code
Install Prettier with auto-format capability via git hooks for automatic formatting on changed files before each commit
Files:
packages/react-on-rails/package.jsonCONTRIBUTING.mdpackage.jsonpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/dummy/package.jsonspec/dummy/package.jsonpnpm-workspace.yamlpackages/react-on-rails-pro/package.jsonreact_on_rails_pro/spec/execjs-compatible-dummy/package.jsonCLAUDE.md
package.json
📄 CodeRabbit inference engine (CLAUDE.md)
After resolving merge conflicts affecting
package.json, verify no hardcoded paths need updating
Files:
package.json
react_on_rails_pro/**/*.{js,ts,jsx,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
The
react_on_rails_pro/directory has its own Prettier/ESLint configuration; both root and Pro directories are linted separately by CI
Files:
react_on_rails_pro/spec/dummy/package.jsonreact_on_rails_pro/spec/execjs-compatible-dummy/package.json
spec/dummy/**
📄 CodeRabbit inference engine (CLAUDE.md)
The dummy app in
spec/dummy/is a full Rails app for integration testing of the React on Rails gem
Files:
spec/dummy/package.json
🧠 Learnings (44)
📓 Common learnings
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to package-scripts.yml : After resolving merge conflicts affecting `package-scripts.yml`, verify no hardcoded paths need updating and test with `yarn run prepack`
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.958Z
Learning: Never use `npm` commands; only use equivalent Yarn Classic commands for package management
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to package-scripts.yml : After resolving merge conflicts affecting `package-scripts.yml`, verify no hardcoded paths need updating and test with `yarn run prepack`
Applied to files:
.github/workflows/integration-tests.ymlpackages/react-on-rails/package.jsonCONTRIBUTING.mdrakelib/run_rspec.rake.claude/docs/testing-build-scripts.md.github/workflows/pro-test-package-and-gem.yml.github/workflows/lint-js-and-ruby.ymlpackage.jsonrakelib/node_package.rake.github/workflows/pro-lint.yml.github/workflows/package-js-tests.ymlrakelib/dummy_apps.rake.github/workflows/examples.ymlrakelib/lint.rake.github/workflows/pro-integration-tests.yml.github/workflows/playwright.ymlpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/dummy/package.jsonspec/dummy/package.jsonpnpm-workspace.yamlpackages/react-on-rails-pro/package.jsonreact_on_rails_pro/spec/execjs-compatible-dummy/package.jsonCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to **/*webpack* : After resolving merge conflicts affecting webpack configs, verify hardcoded paths and test affected scripts
Applied to files:
.github/workflows/integration-tests.yml.claude/docs/testing-build-scripts.md.github/workflows/lint-js-and-ruby.yml.github/workflows/pro-lint.yml.github/workflows/pro-integration-tests.ymlpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/execjs-compatible-dummy/package.jsonCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.958Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.958Z
Learning: Run `yarn playwright install --with-deps` in spec/dummy to install Playwright browsers before running E2E tests
Applied to files:
.github/workflows/integration-tests.ymlCONTRIBUTING.md.claude/docs/testing-build-scripts.md.github/workflows/pro-test-package-and-gem.yml.github/workflows/lint-js-and-ruby.yml.github/workflows/pro-lint.ymlrakelib/dummy_apps.rake.github/workflows/examples.yml.github/workflows/pro-integration-tests.yml.github/workflows/playwright.ymlreact_on_rails_pro/spec/dummy/package.jsonspec/dummy/package.jsonreact_on_rails_pro/spec/execjs-compatible-dummy/package.jsonCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Use `bin/ci-switch-config` to switch between CI configurations (Latest: Ruby 3.4, Node 22, Shakapacker 9.3.0, React 19 vs Minimum: Ruby 3.2, Node 20, Shakapacker 8.2.0, React 18)
Applied to files:
.github/workflows/integration-tests.ymlCONTRIBUTING.md.github/workflows/pro-test-package-and-gem.yml.github/workflows/lint-js-and-ruby.yml.github/workflows/pro-lint.yml.github/workflows/examples.yml.github/workflows/pro-integration-tests.ymlCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to spec/dummy/** : The dummy app in `spec/dummy/` is a full Rails app for integration testing of the React on Rails gem
Applied to files:
.github/workflows/integration-tests.ymlCONTRIBUTING.md.github/workflows/pro-test-package-and-gem.yml.github/workflows/pro-lint.ymlrakelib/dummy_apps.rake.github/workflows/pro-integration-tests.ymlpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/dummy/package.jsonreact_on_rails_pro/spec/execjs-compatible-dummy/package.jsonCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to react_on_rails_pro/sig/**/*.rbs : Validate Pro package RBS signatures with `cd react_on_rails_pro && bundle exec rake rbs:validate`
Applied to files:
.github/workflows/integration-tests.ymlCONTRIBUTING.mdrakelib/run_rspec.rake.github/workflows/pro-test-package-and-gem.yml.github/workflows/lint-js-and-ruby.yml.github/workflows/pro-lint.ymlrakelib/dummy_apps.rake.github/workflows/examples.yml.github/workflows/pro-integration-tests.ymlpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/dummy/package.jsonpackages/react-on-rails-pro/package.jsonreact_on_rails_pro/spec/execjs-compatible-dummy/package.jsonCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to spec/dummy/e2e/playwright/**/*.spec.js : Write Playwright E2E tests in `spec/dummy/e2e/playwright/e2e/` directory using the `cypress-on-rails` gem helpers for Rails integration
Applied to files:
.github/workflows/integration-tests.ymlCONTRIBUTING.mdrakelib/run_rspec.rake.github/workflows/pro-test-package-and-gem.yml.github/workflows/lint-js-and-ruby.yml.github/workflows/pro-lint.ymlrakelib/dummy_apps.rake.github/workflows/pro-integration-tests.yml.github/workflows/playwright.ymlreact_on_rails_pro/spec/dummy/package.jsonCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to packages/react-on-rails/src/**/*.{ts,tsx,js,jsx} : TypeScript in `packages/react-on-rails/src/` compiles to JavaScript in `packages/react-on-rails/lib/` during build process
Applied to files:
.github/workflows/integration-tests.ymlpackages/react-on-rails/package.json.npmrcCONTRIBUTING.mdrakelib/run_rspec.rake.claude/docs/testing-build-scripts.md.github/workflows/pro-test-package-and-gem.yml.github/workflows/lint-js-and-ruby.ymlpackage.json.github/workflows/pro-lint.yml.github/workflows/examples.yml.github/workflows/pro-integration-tests.ymlpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/dummy/package.jsonspec/dummy/package.jsonpnpm-workspace.yamlpackages/react-on-rails-pro/package.jsonreact_on_rails_pro/spec/execjs-compatible-dummy/package.jsonCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to spec/dummy/e2e/playwright/app_commands/**/*.rb : Create custom Rails helper commands for Playwright tests in `spec/dummy/e2e/playwright/app_commands/` directory
Applied to files:
.github/workflows/integration-tests.ymlCONTRIBUTING.mdrakelib/run_rspec.rake.github/workflows/pro-test-package-and-gem.yml.github/workflows/pro-lint.ymlrakelib/dummy_apps.rakerakelib/lint.rake.github/workflows/pro-integration-tests.yml.github/workflows/playwright.ymlreact_on_rails_pro/spec/dummy/package.jsonCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.958Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.958Z
Learning: Exclude `/coverage`, `/tmp`, `/gen-examples`, `/packages/react-on-rails/lib`, `/node_modules`, `/spec/dummy/node_modules`, `/spec/dummy/tmp`, `/spec/dummy/app/assets/webpack`, `/spec/dummy/log`, `/spec/dummy/e2e/playwright-report`, `/spec/dummy/test-results` directories in IDE configuration to prevent slowdowns
Applied to files:
.github/workflows/integration-tests.yml.claude/docs/testing-build-scripts.md.github/workflows/lint-js-and-ruby.ymlpackage.json.github/workflows/pro-lint.yml.github/workflows/pro-integration-tests.ymlpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/dummy/package.jsonpnpm-workspace.yamlCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.958Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.958Z
Learning: Use `yalc` for local development when testing React on Rails with external applications
Applied to files:
.github/workflows/integration-tests.ymlpackages/react-on-rails/package.json.npmrcCONTRIBUTING.md.claude/docs/testing-build-scripts.md.github/workflows/pro-test-package-and-gem.yml.github/workflows/lint-js-and-ruby.ymlpackage.jsonrakelib/node_package.rake.github/workflows/pro-lint.ymlrakelib/dummy_apps.rake.github/workflows/examples.yml.github/workflows/pro-integration-tests.yml.github/workflows/playwright.ymlpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/dummy/package.jsonspec/dummy/package.jsonpackages/react-on-rails-pro/package.jsonreact_on_rails_pro/spec/execjs-compatible-dummy/package.jsonCLAUDE.md
📚 Learning: 2025-01-23T18:20:45.824Z
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 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.
Applied to files:
.github/workflows/integration-tests.ymlpackages/react-on-rails/package.jsonrakelib/run_rspec.rake.claude/docs/testing-build-scripts.md.github/workflows/pro-test-package-and-gem.ymlpackage.jsonrakelib/dummy_apps.rake.github/workflows/pro-integration-tests.ymlpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/dummy/package.jsonspec/dummy/package.jsonpackages/react-on-rails-pro/package.jsonreact_on_rails_pro/spec/execjs-compatible-dummy/package.jsonCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.958Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.958Z
Learning: Never use `npm` commands; only use equivalent Yarn Classic commands for package management
Applied to files:
.github/workflows/integration-tests.ymlpackages/react-on-rails/package.jsonCONTRIBUTING.md.claude/docs/testing-build-scripts.md.github/workflows/pro-test-package-and-gem.ymlrakelib/node_package.rake.github/workflows/examples.ymlrakelib/lint.rakepackages/react-on-rails-pro-node-renderer/package.jsonspec/dummy/package.jsonpackages/react-on-rails-pro/package.jsonreact_on_rails_pro/spec/execjs-compatible-dummy/package.jsonCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to /CHANGELOG_PRO.md : Update `/CHANGELOG_PRO.md` for Pro-only features, bug fixes, and changes specific to the react_on_rails_pro package
Applied to files:
packages/react-on-rails/package.jsonCONTRIBUTING.md.github/workflows/pro-test-package-and-gem.yml.github/workflows/pro-lint.yml.github/workflows/pro-integration-tests.ymlpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/dummy/package.jsonpackages/react-on-rails-pro/package.jsonreact_on_rails_pro/spec/execjs-compatible-dummy/package.jsonCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to react_on_rails_pro/**/*.{js,ts,jsx,tsx,json,md,yml,yaml} : The `react_on_rails_pro/` directory has its own Prettier/ESLint configuration; both root and Pro directories are linted separately by CI
Applied to files:
packages/react-on-rails/package.json.npmrcCONTRIBUTING.md.claude/docs/testing-build-scripts.md.github/workflows/pro-test-package-and-gem.yml.github/workflows/lint-js-and-ruby.ymlpackage.json.github/workflows/pro-lint.ymlrakelib/dummy_apps.rakerakelib/lint.rake.github/workflows/pro-integration-tests.ymlpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/dummy/package.jsonspec/dummy/package.jsonpackages/react-on-rails-pro/package.jsonreact_on_rails_pro/spec/execjs-compatible-dummy/package.jsonCLAUDE.md
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 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.
Applied to files:
packages/react-on-rails/package.json.npmrcCONTRIBUTING.mdpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/dummy/package.jsonspec/dummy/package.jsonpackages/react-on-rails-pro/package.jsonreact_on_rails_pro/spec/execjs-compatible-dummy/package.jsonCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to package.json : After resolving merge conflicts affecting `package.json`, verify no hardcoded paths need updating
Applied to files:
packages/react-on-rails/package.json.claude/docs/testing-build-scripts.md.github/workflows/pro-test-package-and-gem.yml.github/workflows/lint-js-and-ruby.ymlpackage.json.github/workflows/pro-lint.yml.github/workflows/package-js-tests.yml.github/workflows/pro-integration-tests.ymlpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/dummy/package.jsonpackages/react-on-rails-pro/package.jsonCLAUDE.md
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 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.
Applied to files:
packages/react-on-rails/package.json.npmrcCONTRIBUTING.mdpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/dummy/package.jsonpackages/react-on-rails-pro/package.jsonreact_on_rails_pro/spec/execjs-compatible-dummy/package.json
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to **/*.{js,ts,jsx,tsx,json,md,yml,yaml} : Install Prettier with auto-format capability via git hooks for automatic formatting on changed files before each commit
Applied to files:
packages/react-on-rails/package.jsonCONTRIBUTING.md.claude/docs/testing-build-scripts.mdpackage.json.github/workflows/pro-lint.ymlrakelib/lint.rakepackages/react-on-rails-pro-node-renderer/package.jsonpackages/react-on-rails-pro/package.jsonCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to **/*.{js,ts,jsx,tsx,json,md,yml,yaml} : ALWAYS let Prettier and RuboCop handle ALL formatting - never manually format code
Applied to files:
packages/react-on-rails/package.jsonCONTRIBUTING.md.claude/docs/testing-build-scripts.md.github/workflows/lint-js-and-ruby.ymlpackage.json.github/workflows/pro-lint.ymlrakelib/lint.rakepackages/react-on-rails-pro-node-renderer/package.jsonCLAUDE.md
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Applied to files:
.npmrcCONTRIBUTING.mdpackages/react-on-rails-pro-node-renderer/package.jsonpackages/react-on-rails-pro/package.jsonreact_on_rails_pro/spec/execjs-compatible-dummy/package.json
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to /CHANGELOG.md : Do NOT add changelog entries for linting, formatting, refactoring, tests, or documentation-only fixes
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to /CHANGELOG_PRO.md : Do NOT add changelog entries for linting, formatting, refactoring, tests, or documentation-only fixes in the Pro changelog
Applied to files:
CONTRIBUTING.md.github/workflows/pro-lint.yml.github/workflows/pro-integration-tests.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for open-source features, bug fixes, breaking changes, deprecations, and performance improvements
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to lib/generators/react_on_rails/**/*.rb : Generators in `lib/generators/react_on_rails/` run in host app context during setup and can assume Rails app structure
Applied to files:
CONTRIBUTING.mdrakelib/dummy_apps.rakepackages/react-on-rails-pro-node-renderer/package.jsonpackages/react-on-rails-pro/package.json
📚 Learning: 2025-07-08T05:57:29.630Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1745
File: node_package/src/RSCRequestTracker.ts:8-14
Timestamp: 2025-07-08T05:57:29.630Z
Learning: The global `generateRSCPayload` function in React on Rails Pro (RORP) is provided by the framework during rendering requests, not implemented in application code. The `declare global` statements are used to document the expected interface that RORP will inject at runtime.
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-11-24T16:54:16.958Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.958Z
Learning: Use `yarn test:e2e` to run Playwright tests; Rails server auto-starts on port 5017
Applied to files:
CONTRIBUTING.mdrakelib/run_rspec.rake.github/workflows/pro-test-package-and-gem.ymlpackage.json.github/workflows/pro-lint.yml.github/workflows/pro-integration-tests.yml.github/workflows/playwright.ymlreact_on_rails_pro/spec/dummy/package.jsonpackages/react-on-rails-pro/package.jsonCLAUDE.md
📚 Learning: 2025-10-23T17:22:01.074Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to spec/dummy/TESTING_LOCALLY.md : Refer to `spec/dummy/TESTING_LOCALLY.md` for local testing tips and known issues when testing the dummy Rails app
Applied to files:
CONTRIBUTING.md.github/workflows/pro-test-package-and-gem.yml.github/workflows/pro-integration-tests.ymlCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to **/*.{js,ts,jsx,tsx} : Install ESLint with auto-fix capability via git hooks for automatic linting on changed files before each commit
Applied to files:
CONTRIBUTING.md.claude/docs/testing-build-scripts.mdpackage.jsonrakelib/lint.rakeCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to **/*.rb : ALWAYS run `bundle exec rubocop` and fix ALL violations before every commit/push
Applied to files:
CONTRIBUTING.mdrakelib/run_rspec.rake.claude/docs/testing-build-scripts.md.github/workflows/lint-js-and-ruby.yml.github/workflows/pro-lint.ymlrakelib/lint.rakeCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Run `bundle exec rubocop` to fix ALL violations before pushing (mandatory pre-push requirement)
Applied to files:
CONTRIBUTING.mdrakelib/run_rspec.rake.github/workflows/pro-lint.ymlrakelib/lint.rakeCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to **/*.rb : Install RuboCop with auto-fix capability via git hooks for automatic linting on changed files before each commit
Applied to files:
CONTRIBUTING.mdrakelib/lint.rakeCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to lib/react_on_rails/**/*.rb : When creating new Ruby files in `lib/react_on_rails/`, add corresponding RBS type signatures and include in Steepfile for type checking
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to **/*.rb : ALWAYS let RuboCop handle ALL formatting for Ruby files - never manually format code
Applied to files:
CONTRIBUTING.mdrakelib/lint.rake
📚 Learning: 2025-11-24T16:54:16.958Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.958Z
Learning: When resolving merge conflicts, follow exact sequence: resolve conflicts, verify file paths, auto-fix formatting with `rake autofix`, test critical scripts, then continue merge
Applied to files:
CONTRIBUTING.md.claude/docs/testing-build-scripts.mdrakelib/lint.rakeCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to spec/**/*.rb : Use `bin/ci-run-failed-specs` to run only failing RSpec examples from GitHub Actions output
Applied to files:
.github/workflows/lint-js-and-ruby.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Validate RBS type signatures with `bundle exec rake rbs:validate` before committing type-checked code
Applied to files:
.github/workflows/pro-lint.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Run `bundle exec rake rbs:steep` to type-check Ruby code with Steep against RBS signatures
Applied to files:
.github/workflows/pro-lint.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to lib/react_on_rails/engine.rb : In the Engine class, do not use `rake_tasks` block to load rake files from `lib/tasks/` as Rails::Engine loads them automatically, causing duplicate execution
Applied to files:
rakelib/dummy_apps.rake
📚 Learning: 2025-02-13T19:09:15.991Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/RSCWebpackLoader.ts:0-0
Timestamp: 2025-02-13T19:09:15.991Z
Learning: In React Server Components webpack loader, using `new Function('return import("react-server-dom-webpack/node-loader")')()` is necessary as a workaround to bypass TypeScript compilation issues with direct dynamic imports.
Applied to files:
packages/react-on-rails-pro/package.jsonCLAUDE.md
📚 Learning: 2024-12-12T13:07:09.929Z
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 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.
Applied to files:
packages/react-on-rails-pro/package.json
📚 Learning: 2025-02-13T16:50:47.848Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 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`.
Applied to files:
packages/react-on-rails-pro/package.json
🪛 LanguageTool
CONTRIBUTING.md
[uncategorized] ~22-~22: “React” is a proper noun and needs to be capitalized.
Context: ...sites Note for users: End users of react_on_rails can continue using their prefe...
(A_GOOGLE)
[uncategorized] ~22-~22: “React” is a proper noun and needs to be capitalized.
Context: ...low are for contributors working on the react_on_rails codebase itself. - [Yalc](htt...
(A_GOOGLE)
[style] ~310-~310: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...figs, then you need to restart Foreman. If you change the JS code for react-on-rai...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.18.1)
.claude/docs/testing-build-scripts.md
114-114: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ 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). (2)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: claude-review
🔇 Additional comments (47)
CONTRIBUTING.md (1)
1-883: Ensure file ends with newline character.Per the coding guidelines, all files must end with a newline before commit.
rakelib/run_rspec.rake (1)
113-116: LGTM! Rake task correctly updated for pnpm.The task description and command have been properly migrated from Yarn to pnpm, maintaining functional equivalence.
react_on_rails_pro/spec/execjs-compatible-dummy/package.json (2)
2-2: LGTM! Package manager field correctly updated.The packageManager field has been properly set to [email protected], consistent with the monorepo-wide migration.
42-43: LGTM! Scripts correctly migrated to pnpm with yalc integration.The preinstall and link-source scripts have been properly updated to use pnpm while maintaining the necessary yalc workflow for this test dummy app. The combination of pnpm for package management and yalc for local linking is appropriate for this integration test scenario.
packages/react-on-rails/package.json (1)
8-16: LGTM! Scripts correctly migrated to pnpm.All script entries have been properly updated to use pnpm commands. The changes maintain functional equivalence while migrating to the new package manager.
Note: The AI summary mentions a
packageManagerfield update to [email protected], but it's not visible in the provided diff. Ensure this field has been added to maintain consistency with other package.json files in the workspace..npmrc (1)
1-9: LGTM! pnpm configuration appropriately balances compatibility and strictness.The hoisting configuration is well-documented and necessary for compatibility with existing tooling (yalc, Jest). While
shamefully-hoist=trueis less strict than pnpm's default isolated node_modules, it's a pragmatic choice for this migration to maintain compatibility with the existing ecosystem.package.json (1)
69-82: LGTM! Root workspace scripts correctly migrated to pnpm.All script commands have been properly updated to use pnpm with appropriate flags:
-rfor recursive workspace operations--filterfor targeting specific packages- Maintained the postinstall hook for lefthook setup
The migration maintains functional equivalence while leveraging pnpm's workspace features.
CLAUDE.md (3)
74-96: LGTM! Essential commands comprehensively updated to pnpm.All development commands have been properly migrated from Yarn to pnpm, maintaining consistency throughout the documentation. The explicit guidance on line 96 to "Never run npm commands, only equivalent pnpm ones" correctly establishes pnpm as the standard package manager.
Note: The retrieved learning stating "Never use npm commands; only use equivalent Yarn Classic commands for package management" is now outdated by this migration and should be updated to reflect pnpm as the standard.
Based on learnings: The historical learning about Yarn Classic is superseded by this PR's migration to pnpm.
308-339: LGTM! Merge conflict resolution workflow updated for pnpm.The critical workflow section has been updated with pnpm commands, including verification steps for affected scripts. The testing guidance correctly references
pnpm run prepackandpnpm run yalc.publish.
502-541: LGTM! Playwright and general tooling commands migrated to pnpm.All Playwright E2E testing commands and general development tooling references have been consistently updated to use pnpm, including:
- Playwright installation:
pnpm playwright install --with-deps- Test execution:
pnpm test:e2evariants- Clear statement that pnpm is the only supported JS package manager
.claude/docs/testing-build-scripts.md (3)
45-68: LGTM! Testing instructions correctly updated for pnpm.The mandatory testing steps have been properly migrated to pnpm commands, including:
- Clean install testing with
pnpm install --frozen-lockfile- Build and package-specific script testing with pnpm
- Clear emphasis on CI-equivalent testing procedures
112-159: LGTM! Excellent documentation of pnpm workspace dependencies.The new section comprehensively explains:
- Correct workspace:* syntax for pnpm
- Why the previous "*" syntax (Yarn Classic) won't work
- Testing procedures for workspace changes
- Clear comparison with Yarn Classic behavior
This will prevent common migration issues and clarify the workspace protocol differences between package managers.
161-182: LGTM! Historical context aids understanding.The real-world examples document lessons learned from the migration, providing valuable context for:
- Path reference issues during restructuring
- Workspace protocol migration gotchas
- The importance of testing with
pnpm install --frozen-lockfilepackages/react-on-rails-pro/package.json (2)
7-17: LGTM! Scripts correctly migrated to pnpm.All script commands have been properly updated to use pnpm, maintaining functional equivalence with the previous Yarn-based commands.
58-58: LGTM! Workspace dependency correctly usesworkspace:*protocol.The migration from
"*"to"workspace:*"is the correct pattern for pnpm workspaces. This ensures pnpm properly resolves to the local workspace package rather than attempting to fetch from the registry.packages/react-on-rails-pro-node-renderer/package.json (4)
50-50: LGTM: Correct workspace protocol usage.The migration from
"*"to"workspace:*"properly leverages pnpm's native workspace linking, eliminating the need for manual yalc workflow for this internal dependency.
73-81: Scripts correctly migrated to pnpm.The build and type-check scripts properly use pnpm commands. Direct
tscinvocations work since TypeScript is a local devDependency, though usingpnpm exec tscwould make the dependency resolution more explicit.
82-83: Verify yalc workflow compatibility with pnpm.These yalc scripts are retained as part of the migration. Ensure the yalc publish/add workflow still functions correctly under pnpm, as noted in your test plan.
126-126: LGTM: Package manager correctly specified.The
packageManagerfield ensures Corepack uses pnpm 9.14.2, maintaining consistency across all environments.pnpm-workspace.yaml (1)
1-2: LGTM: Minimal workspace configuration is correct.The workspace pattern
packages/*appropriately includes all publishable packages. The spec/dummy test fixture is intentionally excluded from the workspace as it's a development/testing app, not a publishable package.rakelib/lint.rake (2)
20-20: LGTM: Lint commands migrated to pnpm.The eslint task correctly uses pnpm commands, maintaining the same version check and lint functionality.
30-32: LGTM: Autofix and stylelint tasks properly migrated.All linting and formatting commands correctly use pnpm equivalents, preserving the original functionality.
Also applies to: 40-40
rakelib/node_package.rake (1)
10-10: LGTM: Build task migrated to pnpm.The command correctly uses
pnpm yalc:publishto match the script name in package.json (line 82 of react-on-rails-pro-node-renderer/package.json)..github/workflows/lint-js-and-ruby.yml (3)
101-114: LGTM: pnpm setup and caching correctly configured.The workflow uses the official
pnpm/action-setup@v4action with proper store path detection and cache configuration keyed topnpm-lock.yaml. This follows pnpm's recommended GitHub Actions integration pattern.
131-132: LGTM: Dependency installation migrated to pnpm.All installation steps correctly use
pnpm install --frozen-lockfile(equivalent to yarn's--frozen-lockfile) andpnpm add -gfor global packages.Also applies to: 138-138
165-182: LGTM: Build and lint commands properly migrated.All build, lint, type-check, and packaging commands correctly use pnpm equivalents while maintaining the same functionality.
rakelib/dummy_apps.rake (2)
8-12: LGTM: Dummy app installation migrated to pnpm.The task correctly uses
pnpm installand maintains the yalc linking for local development. The yalc workflow will be validated as part of the test plan.
14-14: LGTM: Task dependency correctly updated.The dependency change from
:yarn_installto:pnpm_installmaintains the same task flow with the new package manager..github/workflows/playwright.yml (4)
56-71: LGTM: pnpm setup matches lint workflow pattern.The pnpm configuration follows the same proven pattern used in the lint workflow, with proper caching and store path detection.
74-84: LGTM: Dependencies installed with pnpm.All installation steps correctly use pnpm commands, including the global yalc installation and both root and dummy app dependencies.
87-87: LGTM: Correct use of pnpm exec for Playwright.Using
pnpm exec playwrightensures the command runs with the correct dependency resolution from the pnpm workspace.
97-101: LGTM: Test commands migrated to pnpm.Build and test execution commands properly use
pnpm runequivalents.spec/dummy/package.json (3)
70-71: LGTM: Installation scripts migrated to pnpm.The preinstall and link-source scripts correctly use pnpm commands. The yalc integration is intentionally retained for local development workflow validation.
72-86: LGTM: All test and build scripts use pnpm.Comprehensive migration of all scripts to pnpm equivalents, maintaining the same functionality for testing, linting, and building.
90-90: LGTM: Package manager version pinned.Consistent with other package.json files, using [email protected] across the repository.
.github/workflows/integration-tests.yml (2)
122-135: Clean pnpm setup with consistent caching across job duplication.The pnpm/action-setup and store caching blocks are replicated correctly in both jobs. Cache configuration properly references
pnpm-lock.yamland the store path discovery matches pnpm best practices. No issues detected.Also applies to: 211-224
148-151: Frozen-lockfile usage appropriately conditional on dependency levels.Restricting
--frozen-lockfileto latest/primary dependencies (Node 22, latest) while allowing flexibility for minimum dependencies is a sound pattern that balances reproducibility with test coverage. This enables validation of dependency constraints without blocking minimum-version testing.Also applies to: 156-157, 254-257, 262-263
.github/workflows/package-js-tests.yml (1)
101-114: LGTM — Straightforward pnpm migration with correct setup and cache configuration.The workflow properly migrates from Yarn to pnpm with standard setup, caching, and command patterns. Cache keys and store paths are correct.
Also applies to: 125-132
.github/workflows/examples.yml (2)
157-159: Verify thepnpm yalc:publishworkspace script pattern.The use of
pnpm yalc:publish(instead of direct yalc calls like in integration-tests.yml) suggests a workspace-aware script. Confirm that this script is properly defined in package.json and handles all necessary packages. If this needs to run recursively across packages, verify the command is correct or consider usingpnpm -r run yalc:publish.
119-132: LGTM — Consistent pnpm setup and cache configuration.The pnpm migration follows the established pattern across other workflows. Setup, caching, and installation commands are correct.
Also applies to: 152-155
.github/workflows/pro-lint.yml (1)
100-115: LGTM — Clean pnpm workspace migration with idiomatic filtering.The use of
pnpm --filter(line 169) for package-specific builds is idiomatic and correct. All setup, caching, and lint commands are properly converted. The always-frozen-lockfile approach is appropriate for a lint workflow.Also applies to: 147-162, 178-184
react_on_rails_pro/spec/dummy/package.json (1)
98-103: LGTM — Correct packageManager field and script conversions.The packageManager field correctly specifies
[email protected]as required. Script conversions properly chain pnpm operations with yalc setup. Preinstall/postinstall hooks follow pnpm conventions.Also applies to: 128-128
.github/workflows/pro-integration-tests.yml (3)
102-117: LGTM — Consistent pnpm setup across all three jobs.The pnpm setup blocks are properly replicated in each job with correct store path discovery, caching, and installation. Frozen-lockfile usage is consistent and appropriate for integration testing.
Also applies to: 203-218, 405-420
516-516: LGTM — Playwright integration correctly uses pnpm.Commands are syntactically correct for pnpm:
pnpm playwright install --with-depsandpnpm e2e-testproperly invoke Playwright functionality through the package manager.Also applies to: 519-519
351-356: Artifact naming correctly updated for pnpm.Reference to
pnpm-debug.logis correct and will capture debug logs from pnpm operations for troubleshooting on failure..github/workflows/pro-test-package-and-gem.yml (2)
102-117: LGTM — Consistent pnpm setup across Node-dependent jobs.The pnpm setup is properly configured in jobs that require Node. The third job (rspec-gem-specs) correctly omits pnpm setup as it's Ruby-only.
Also applies to: 208-223
249-256: LGTM — Correct use of pnpm workspace filtering for monorepo tests.The
pnpm --filter react-on-rails-pro-node-renderer cicommand (line 256) correctly uses pnpm's idiomatic filtering syntax for running Jest tests in a specific workspace package.
| "packageManager": "[email protected]", | ||
| "pnpm": { | ||
| "overrides": { | ||
| "sentry-testkit>body-parser": "npm:[email protected]", | ||
| "sentry-testkit>express": "npm:[email protected]" | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify pnpm version is current and stable.
The packageManager field declares [email protected], and overrides have been migrated to pnpm's format. Ensure this version is the intended stable release.
🌐 Web query:
What is the latest stable version of pnpm in the 9.x series?
💡 Result:
The latest stable pnpm in the 9.x series is 9.15.9. [1][2]
Sources:
- npm package page for pnpm (v9.15.9). [1]
- pnpm documentation (9.x installation page). [2]
Update pnpm to the latest stable 9.x release.
The latest stable pnpm in the 9.x series is 9.15.9, while package.json specifies 9.14.2. Update the packageManager field to [email protected].
🤖 Prompt for AI Agents
In package.json around lines 101 to 107, the packageManager field pins pnpm to
9.14.2; update it to the latest stable 9.x (9.15.9) by changing the value of
"packageManager" from "[email protected]" to "[email protected]" and keep the existing
pnpm.overrides block unchanged so only the manager version is bumped.
PR Review: Yarn Classic to pnpm MigrationThis is a well-executed migration with excellent workspace configuration and GitHub Actions updates. However, there are several critical issues that will cause CI failures and need to be fixed before merging. 🔴 Critical Issues (Will Break CI)1. package-scripts.yml line 18 - Uses
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.claude/agents/pr-testing-agent.md (1)
56-667: Add language identifiers to all fenced code blocks.This new file has 20+ markdown linting violations (MD040: missing language specifications on code blocks). Prettier should auto-fix these, but run formatting immediately:
rake autofixExamples of required fixes:
- Line 56:
```→(or blank code block, remove if unnecessary)- Line 70:
```→bash- Line 82:
```→markdown- And 17+ more throughout the file
Per coding guidelines, "ALWAYS let Prettier and RuboCop handle ALL formatting." Run
rake autofixto resolve all markdown linting violations before commit. If violations persist after autofix, manually verify each code block has a language identifier (bash, ruby, javascript, json, markdown, yaml, etc.).react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb (1)
249-265: Add nil guard tohandle_stream_cache_hitbefore using@async_barrier.The cache hit path bypasses the nil check in
consumer_stream_async. Whencached_stream_react_componentis called outsidestream_view_containing_react_componentsand a cache hit occurs,handle_stream_cache_hitwill crash at line 259 attempting to call.asyncon nil.The cache miss path is protected—it routes through
handle_stream_cache_miss→render_stream_component_with_props→stream_react_component→consumer_stream_async, which has the required nil check (line 304-306). The cache hit path needs the same protection.Recommendation: Add a nil check before line 259:
- Either in
handle_stream_cache_hitmatching the pattern at line 304-306- Or in
fetch_stream_react_componentbefore callinghandle_stream_cache_hit
🧹 Nitpick comments (6)
packages/react-on-rails-pro-node-renderer/tests/httpRequestUtils.ts (1)
142-142: Fix typo in error messages.The error message contains "eneded" which should be "ended".
Apply this diff to fix the typos:
- rejectChunksPromise('Request already eneded'); + rejectChunksPromise('Request already ended');- reject(new Error('Request already eneded')); + reject(new Error('Request already ended'));Also applies to: 162-162
packages/react-on-rails-pro-node-renderer/tests/testingNodeRendererConfigs.ts (1)
1-49: Well-structured test helper for parallel test isolation.The factory pattern ensures each test gets a unique
serverBundleCachePath, preventing race conditions. A few observations:
Line 25: The type cast
env.RENDERER_LOG_LEVEL as LevelWithSilentis unsafe—if an invalid value is provided, it will pass TypeScript checks but may cause runtime issues. Consider validating the value against allowed pino log levels.Line 17-19: The synchronous cleanup is acceptable for test setup, but consider wrapping in a try-catch to handle edge cases (e.g., permission errors) gracefully.
Consider adding validation for the log level:
+const validLogLevels = ['fatal', 'error', 'warn', 'info', 'debug', 'trace', 'silent'] as const; + +function getLogLevel(): LevelWithSilent { + const level = env.RENDERER_LOG_LEVEL; + if (level && validLogLevels.includes(level as LevelWithSilent)) { + return level as LevelWithSilent; + } + return 'info'; +} + export function createTestConfig(testName: string): { config: Partial<Config>; bundlePath: string } { // ... - logLevel: (env.RENDERER_LOG_LEVEL as LevelWithSilent | undefined) || 'info', + logLevel: getLogLevel(),packages/react-on-rails-pro-node-renderer/tests/vm.test.ts (1)
196-269: Appropriate adaptation for React 19 compatibility.The validation approach shift from exact
data-react-checksumvalues to content presence and length checks is reasonable given React 19's removal of checksums. The comments clearly explain the rationale.However, the validations are quite permissive—checking for
'<'andlength > 100would pass for many malformed outputs. Consider slightly stronger assertions:- // React 19 removed data-react-checksum, verify component rendered successfully - expect(welcomePageRenderingResult as string).toContain('<'); - expect((welcomePageRenderingResult as string).length).toBeGreaterThan(100); + // React 19 removed data-react-checksum, verify component rendered successfully + expect(welcomePageRenderingResult as string).toMatch(/<\w+[^>]*>/); // Contains HTML tag + expect((welcomePageRenderingResult as string).length).toBeGreaterThan(100);This would ensure actual HTML tags are present rather than just the
<character.react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb (2)
11-11: Document why instance variables are necessary here.The RuboCop disable is broad (entire file). Consider adding a comment explaining that instance variables are required to simulate the streaming context that
stream_view_containing_react_componentsnormally provides, and that these are intentionally shared between the around block and test examples.
724-747: Consider extracting the async setup pattern into a test helper.The pattern of wrapping in
Sync, initializing@async_barrierand@main_output_queue, then draining the queue appears multiple times (lines 366-379, 727-744, 785-789). This could be extracted into a reusable test helper method to reduce duplication.Example:
def with_streaming_context(&block) Sync do @async_barrier = Async::Barrier.new @main_output_queue = Async::Queue.new result = block.call @async_barrier.wait @main_output_queue.close # Drain queue while @main_output_queue.dequeue; end result end endreact_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb (1)
301-341: Consider thread safety forall_chunksarray.The
all_chunksarray (line 311) is accessed from within an async task (line 319) without explicit synchronization. While Ruby's GIL typically protects array operations, this could be fragile if execution moves to a truly parallel context in the future.Consider using
Async::Queuefor collecting chunks instead:def consumer_stream_async(on_complete:) require "async/variable" if @async_barrier.nil? raise ReactOnRails::Error, "You must call stream_view_containing_react_components to render the view containing the react component" end first_chunk_var = Async::Variable.new chunk_queue = Async::Queue.new if on_complete @async_barrier.async do stream = yield is_first = true stream.each_chunk do |chunk| chunk_queue&.enqueue(chunk) # Thread-safe enqueue if is_first first_chunk_var.value = chunk is_first = false else @main_output_queue.enqueue(chunk) end end first_chunk_var.value = nil if is_first if on_complete chunk_queue.close all_chunks = [] while (chunk = chunk_queue.dequeue) all_chunks << chunk end on_complete.call(all_chunks) end end first_chunk_var.wait first_chunk_var.value end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
.claude/agents/pr-testing-agent.md(1 hunks).claude/agents/pr-testing-guide.md(1 hunks).claude/docs/analysis/CI_FAILURES_2024-11-21.md(1 hunks).claude/docs/analysis/INDEX.md(2 hunks).github/workflows/examples.yml(2 hunks).github/workflows/integration-tests.yml(5 hunks).github/workflows/lint-js-and-ruby.yml(2 hunks).github/workflows/package-js-tests.yml(1 hunks).github/workflows/playwright.yml(2 hunks).github/workflows/pro-integration-tests.yml(13 hunks).github/workflows/pro-lint.yml(4 hunks).github/workflows/pro-test-package-and-gem.yml(5 hunks)CLAUDE.md(9 hunks)packages/react-on-rails-pro-node-renderer/tests/concurrentHtmlStreaming.test.ts(1 hunks)packages/react-on-rails-pro-node-renderer/tests/htmlStreaming.test.js(1 hunks)packages/react-on-rails-pro-node-renderer/tests/httpRequestUtils.ts(1 hunks)packages/react-on-rails-pro-node-renderer/tests/serverRenderRSCReactComponent.test.js(1 hunks)packages/react-on-rails-pro-node-renderer/tests/testingNodeRendererConfigs.js(0 hunks)packages/react-on-rails-pro-node-renderer/tests/testingNodeRendererConfigs.ts(1 hunks)packages/react-on-rails-pro-node-renderer/tests/vm.test.ts(11 hunks)packages/react-on-rails-pro-node-renderer/tests/worker.test.ts(1 hunks)react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb(4 hunks)react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb(1 hunks)react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb(7 hunks)react_on_rails_pro/spec/react_on_rails_pro/stream_spec.rb(2 hunks)
💤 Files with no reviewable changes (1)
- packages/react-on-rails-pro-node-renderer/tests/testingNodeRendererConfigs.js
✅ Files skipped from review due to trivial changes (3)
- .claude/docs/analysis/CI_FAILURES_2024-11-21.md
- packages/react-on-rails-pro-node-renderer/tests/worker.test.ts
- packages/react-on-rails-pro-node-renderer/tests/serverRenderRSCReactComponent.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/integration-tests.yml
🧰 Additional context used
📓 Path-based instructions (4)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS ensure files end with a newline character before every commit/push
Files:
packages/react-on-rails-pro-node-renderer/tests/testingNodeRendererConfigs.tspackages/react-on-rails-pro-node-renderer/tests/vm.test.tspackages/react-on-rails-pro-node-renderer/tests/httpRequestUtils.tsreact_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/stream_spec.rbpackages/react-on-rails-pro-node-renderer/tests/concurrentHtmlStreaming.test.tspackages/react-on-rails-pro-node-renderer/tests/htmlStreaming.test.jsCLAUDE.mdreact_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rbreact_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
**/*.{js,ts,jsx,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,ts,jsx,tsx,json,md,yml,yaml}: ALWAYS let Prettier and RuboCop handle ALL formatting - never manually format code
Install Prettier with auto-format capability via git hooks for automatic formatting on changed files before each commit
Files:
packages/react-on-rails-pro-node-renderer/tests/testingNodeRendererConfigs.tspackages/react-on-rails-pro-node-renderer/tests/vm.test.tspackages/react-on-rails-pro-node-renderer/tests/httpRequestUtils.tspackages/react-on-rails-pro-node-renderer/tests/concurrentHtmlStreaming.test.tspackages/react-on-rails-pro-node-renderer/tests/htmlStreaming.test.jsCLAUDE.md
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Install ESLint with auto-fix capability via git hooks for automatic linting on changed files before each commit
Files:
packages/react-on-rails-pro-node-renderer/tests/testingNodeRendererConfigs.tspackages/react-on-rails-pro-node-renderer/tests/vm.test.tspackages/react-on-rails-pro-node-renderer/tests/httpRequestUtils.tspackages/react-on-rails-pro-node-renderer/tests/concurrentHtmlStreaming.test.tspackages/react-on-rails-pro-node-renderer/tests/htmlStreaming.test.js
**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rb: ALWAYS runbundle exec rubocopand fix ALL violations before every commit/push
ALWAYS let RuboCop handle ALL formatting for Ruby files - never manually format code
Install RuboCop with auto-fix capability via git hooks for automatic linting on changed files before each commit
Files:
react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/stream_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rbreact_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
🧠 Learnings (48)
📓 Common learnings
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to package-scripts.yml : After resolving merge conflicts affecting `package-scripts.yml`, verify no hardcoded paths need updating and test with `yarn run prepack`
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to package-scripts.yml : After resolving merge conflicts affecting `package-scripts.yml`, verify no hardcoded paths need updating and test with `yarn run prepack`
Applied to files:
.github/workflows/lint-js-and-ruby.yml.github/workflows/pro-test-package-and-gem.yml.claude/agents/pr-testing-agent.md.github/workflows/examples.ymlpackages/react-on-rails-pro-node-renderer/tests/vm.test.tspackages/react-on-rails-pro-node-renderer/tests/httpRequestUtils.ts.claude/agents/pr-testing-guide.mdpackages/react-on-rails-pro-node-renderer/tests/concurrentHtmlStreaming.test.ts.github/workflows/pro-lint.ymlCLAUDE.md.github/workflows/package-js-tests.yml.github/workflows/pro-integration-tests.yml.github/workflows/playwright.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to **/*webpack* : After resolving merge conflicts affecting webpack configs, verify hardcoded paths and test affected scripts
Applied to files:
.github/workflows/lint-js-and-ruby.ymlpackages/react-on-rails-pro-node-renderer/tests/testingNodeRendererConfigs.ts.github/workflows/pro-test-package-and-gem.ymlpackages/react-on-rails-pro-node-renderer/tests/vm.test.tspackages/react-on-rails-pro-node-renderer/tests/httpRequestUtils.tspackages/react-on-rails-pro-node-renderer/tests/concurrentHtmlStreaming.test.tsCLAUDE.md.github/workflows/pro-integration-tests.yml.github/workflows/playwright.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to package.json : After resolving merge conflicts affecting `package.json`, verify no hardcoded paths need updating
Applied to files:
.github/workflows/lint-js-and-ruby.yml.github/workflows/examples.yml.github/workflows/pro-lint.ymlCLAUDE.md.github/workflows/package-js-tests.yml.github/workflows/pro-integration-tests.yml
📚 Learning: 2025-11-24T16:54:16.958Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.958Z
Learning: Run `yarn playwright install --with-deps` in spec/dummy to install Playwright browsers before running E2E tests
Applied to files:
.github/workflows/lint-js-and-ruby.yml.github/workflows/pro-test-package-and-gem.yml.github/workflows/examples.yml.github/workflows/pro-lint.ymlCLAUDE.md.github/workflows/pro-integration-tests.yml.github/workflows/playwright.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Use `bin/ci-switch-config` to switch between CI configurations (Latest: Ruby 3.4, Node 22, Shakapacker 9.3.0, React 19 vs Minimum: Ruby 3.2, Node 20, Shakapacker 8.2.0, React 18)
Applied to files:
.github/workflows/lint-js-and-ruby.yml.github/workflows/pro-test-package-and-gem.yml.github/workflows/examples.yml.github/workflows/pro-lint.ymlCLAUDE.md.github/workflows/pro-integration-tests.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to react_on_rails_pro/**/*.{js,ts,jsx,tsx,json,md,yml,yaml} : The `react_on_rails_pro/` directory has its own Prettier/ESLint configuration; both root and Pro directories are linted separately by CI
Applied to files:
.github/workflows/lint-js-and-ruby.yml.github/workflows/pro-test-package-and-gem.yml.github/workflows/pro-lint.ymlCLAUDE.md.github/workflows/pro-integration-tests.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to packages/react-on-rails/src/**/*.{ts,tsx,js,jsx} : TypeScript in `packages/react-on-rails/src/` compiles to JavaScript in `packages/react-on-rails/lib/` during build process
Applied to files:
.github/workflows/lint-js-and-ruby.yml.github/workflows/pro-test-package-and-gem.yml.github/workflows/examples.ymlpackages/react-on-rails-pro-node-renderer/tests/vm.test.tspackages/react-on-rails-pro-node-renderer/tests/httpRequestUtils.tspackages/react-on-rails-pro-node-renderer/tests/concurrentHtmlStreaming.test.ts.github/workflows/pro-lint.ymlCLAUDE.md.github/workflows/pro-integration-tests.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to react_on_rails_pro/sig/**/*.rbs : Validate Pro package RBS signatures with `cd react_on_rails_pro && bundle exec rake rbs:validate`
Applied to files:
.github/workflows/lint-js-and-ruby.yml.github/workflows/pro-test-package-and-gem.yml.github/workflows/examples.ymlreact_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/stream_spec.rb.github/workflows/pro-lint.ymlCLAUDE.mdreact_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb.github/workflows/pro-integration-tests.yml
📚 Learning: 2025-11-24T16:54:16.958Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.958Z
Learning: Exclude `/coverage`, `/tmp`, `/gen-examples`, `/packages/react-on-rails/lib`, `/node_modules`, `/spec/dummy/node_modules`, `/spec/dummy/tmp`, `/spec/dummy/app/assets/webpack`, `/spec/dummy/log`, `/spec/dummy/e2e/playwright-report`, `/spec/dummy/test-results` directories in IDE configuration to prevent slowdowns
Applied to files:
.github/workflows/lint-js-and-ruby.ymlpackages/react-on-rails-pro-node-renderer/tests/httpRequestUtils.ts.github/workflows/pro-lint.ymlCLAUDE.md.github/workflows/pro-integration-tests.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to **/*.rb : ALWAYS run `bundle exec rubocop` and fix ALL violations before every commit/push
Applied to files:
.github/workflows/lint-js-and-ruby.yml.github/workflows/pro-lint.ymlCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.958Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.958Z
Learning: Use `yalc` for local development when testing React on Rails with external applications
Applied to files:
.github/workflows/lint-js-and-ruby.yml.github/workflows/pro-test-package-and-gem.yml.github/workflows/examples.yml.github/workflows/pro-lint.ymlCLAUDE.md.github/workflows/pro-integration-tests.yml.github/workflows/playwright.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to **/*.{js,ts,jsx,tsx,json,md,yml,yaml} : ALWAYS let Prettier and RuboCop handle ALL formatting - never manually format code
Applied to files:
.github/workflows/lint-js-and-ruby.yml.github/workflows/pro-lint.ymlCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to spec/dummy/e2e/playwright/**/*.spec.js : Write Playwright E2E tests in `spec/dummy/e2e/playwright/e2e/` directory using the `cypress-on-rails` gem helpers for Rails integration
Applied to files:
.github/workflows/lint-js-and-ruby.yml.github/workflows/pro-test-package-and-gem.ymlpackages/react-on-rails-pro-node-renderer/tests/httpRequestUtils.tsreact_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb.github/workflows/pro-lint.ymlCLAUDE.md.github/workflows/pro-integration-tests.yml.github/workflows/playwright.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to spec/dummy/** : The dummy app in `spec/dummy/` is a full Rails app for integration testing of the React on Rails gem
Applied to files:
.github/workflows/lint-js-and-ruby.yml.github/workflows/pro-test-package-and-gem.ymlreact_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb.github/workflows/pro-lint.ymlCLAUDE.md.github/workflows/pro-integration-tests.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to /CHANGELOG_PRO.md : Update `/CHANGELOG_PRO.md` for Pro-only features, bug fixes, and changes specific to the react_on_rails_pro package
Applied to files:
.github/workflows/pro-test-package-and-gem.ymlpackages/react-on-rails-pro-node-renderer/tests/vm.test.tsreact_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/stream_spec.rb.github/workflows/pro-lint.ymlCLAUDE.mdreact_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rbreact_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb.github/workflows/pro-integration-tests.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to spec/dummy/TESTING_LOCALLY.md : Refer to `spec/dummy/TESTING_LOCALLY.md` for local testing tips and known issues when testing the dummy Rails app
Applied to files:
.github/workflows/pro-test-package-and-gem.yml.claude/agents/pr-testing-guide.mdCLAUDE.md.github/workflows/pro-integration-tests.yml
📚 Learning: 2025-01-23T18:20:45.824Z
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 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.
Applied to files:
.github/workflows/pro-test-package-and-gem.ymlCLAUDE.md.github/workflows/pro-integration-tests.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to spec/dummy/e2e/playwright/app_commands/**/*.rb : Create custom Rails helper commands for Playwright tests in `spec/dummy/e2e/playwright/app_commands/` directory
Applied to files:
.github/workflows/pro-test-package-and-gem.ymlreact_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb.github/workflows/pro-lint.ymlCLAUDE.md.github/workflows/pro-integration-tests.yml.github/workflows/playwright.yml
📚 Learning: 2025-11-24T16:54:16.958Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.958Z
Learning: Use `yarn test:e2e` to run Playwright tests; Rails server auto-starts on port 5017
Applied to files:
.github/workflows/pro-test-package-and-gem.ymlpackages/react-on-rails-pro-node-renderer/tests/concurrentHtmlStreaming.test.ts.github/workflows/pro-lint.ymlCLAUDE.md.github/workflows/pro-integration-tests.yml.github/workflows/playwright.yml
📚 Learning: 2025-11-24T16:54:16.958Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.958Z
Learning: Never use `npm` commands; only use equivalent Yarn Classic commands for package management
Applied to files:
.github/workflows/pro-test-package-and-gem.yml.github/workflows/examples.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Document testing activities in commit messages for complex fixes and note in PR descriptions which fixes were tested locally vs. hypothetical
Applied to files:
.claude/agents/pr-testing-agent.md.claude/agents/pr-testing-guide.mdCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: NEVER claim a test is 'fixed' without running it locally first; use 'SHOULD fix' for untested proposed fixes and 'Analysis suggests' for investigation without fixes
Applied to files:
.claude/agents/pr-testing-agent.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Prefer local testing over CI iteration; do not push 'hopeful' fixes and wait for CI feedback
Applied to files:
.claude/agents/pr-testing-agent.mdCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.958Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.958Z
Learning: When resolving merge conflicts, follow exact sequence: resolve conflicts, verify file paths, auto-fix formatting with `rake autofix`, test critical scripts, then continue merge
Applied to files:
.claude/agents/pr-testing-agent.mdCLAUDE.md
📚 Learning: 2024-12-12T13:07:09.929Z
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 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.
Applied to files:
packages/react-on-rails-pro-node-renderer/tests/vm.test.ts
📚 Learning: 2025-02-13T19:09:15.991Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/RSCWebpackLoader.ts:0-0
Timestamp: 2025-02-13T19:09:15.991Z
Learning: In React Server Components webpack loader, using `new Function('return import("react-server-dom-webpack/node-loader")')()` is necessary as a workaround to bypass TypeScript compilation issues with direct dynamic imports.
Applied to files:
packages/react-on-rails-pro-node-renderer/tests/vm.test.tspackages/react-on-rails-pro-node-renderer/tests/concurrentHtmlStreaming.test.tsCLAUDE.md
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 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.
Applied to files:
packages/react-on-rails-pro-node-renderer/tests/httpRequestUtils.tspackages/react-on-rails-pro-node-renderer/tests/concurrentHtmlStreaming.test.tsreact_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 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.
Applied to files:
packages/react-on-rails-pro-node-renderer/tests/httpRequestUtils.tsCLAUDE.md
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: theforestvn88
Repo: shakacode/react_on_rails PR: 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.
Applied to files:
react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/stream_spec.rb
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 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:
react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rbreact_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
📚 Learning: 2025-04-09T13:02:16.009Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1696
File: node_package/src/transformRSCNodeStream.ts:3-31
Timestamp: 2025-04-09T13:02:16.009Z
Learning: In the React Server Components (RSC) stream processing implementation, all chunks end with a newline character (`\n`), so the `lastIncompleteChunk` variable is expected to be empty by the time the stream ends, making a flush handler unnecessary in the Transform stream.
Applied to files:
react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
📚 Learning: 2025-04-09T12:56:10.756Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 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:
react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rbreact_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 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.
Applied to files:
react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
📚 Learning: 2025-02-13T14:29:49.267Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 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.
Applied to files:
react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Applied to files:
packages/react-on-rails-pro-node-renderer/tests/concurrentHtmlStreaming.test.tspackages/react-on-rails-pro-node-renderer/tests/htmlStreaming.test.jsreact_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
📚 Learning: 2025-07-08T05:57:29.630Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1745
File: node_package/src/RSCRequestTracker.ts:8-14
Timestamp: 2025-07-08T05:57:29.630Z
Learning: The global `generateRSCPayload` function in React on Rails Pro (RORP) is provided by the framework during rendering requests, not implemented in application code. The `declare global` statements are used to document the expected interface that RORP will inject at runtime.
Applied to files:
packages/react-on-rails-pro-node-renderer/tests/concurrentHtmlStreaming.test.tsreact_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to /CHANGELOG_PRO.md : Do NOT add changelog entries for linting, formatting, refactoring, tests, or documentation-only fixes in the Pro changelog
Applied to files:
.github/workflows/pro-lint.yml.github/workflows/pro-integration-tests.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Validate RBS type signatures with `bundle exec rake rbs:validate` before committing type-checked code
Applied to files:
.github/workflows/pro-lint.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Run `bundle exec rake rbs:steep` to type-check Ruby code with Steep against RBS signatures
Applied to files:
.github/workflows/pro-lint.yml
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Run `bundle exec rubocop` to fix ALL violations before pushing (mandatory pre-push requirement)
Applied to files:
.github/workflows/pro-lint.ymlCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to **/*.{js,ts,jsx,tsx,json,md,yml,yaml} : Install Prettier with auto-format capability via git hooks for automatic formatting on changed files before each commit
Applied to files:
.github/workflows/pro-lint.ymlCLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to **/*.{js,ts,jsx,tsx} : Install ESLint with auto-fix capability via git hooks for automatic linting on changed files before each commit
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to **/*.rb : Install RuboCop with auto-fix capability via git hooks for automatic linting on changed files before each commit
Applied to files:
CLAUDE.md
📚 Learning: 2025-09-15T21:24:48.207Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicit `data-force-load="true"` usage and the ability to hydrate components during the page loading state (`document.readyState === 'loading'`). Both capabilities require a Pro license, so the condition `!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')` correctly gates both scenarios.
Applied to files:
react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rbreact_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
📚 Learning: 2025-11-24T16:54:16.957Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:54:16.957Z
Learning: Applies to lib/react_on_rails/**/*.rb : When creating new Ruby files in `lib/react_on_rails/`, add corresponding RBS type signatures and include in Steepfile for type checking
Applied to files:
react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
📚 Learning: 2025-02-13T16:50:47.848Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 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`.
Applied to files:
react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 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:
react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
🧬 Code graph analysis (7)
packages/react-on-rails-pro-node-renderer/tests/testingNodeRendererConfigs.ts (2)
packages/react-on-rails-pro-node-renderer/tests/htmlStreaming.test.js (1)
createTestConfig(7-7)packages/react-on-rails-pro-node-renderer/src/shared/configBuilder.ts (1)
Config(25-89)
packages/react-on-rails-pro-node-renderer/tests/vm.test.ts (2)
packages/react-on-rails-pro-node-renderer/tests/helper.ts (1)
readRenderingRequest(137-145)packages/react-on-rails-pro-node-renderer/src/worker/vm.ts (1)
runInVM(110-179)
react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb (3)
react_on_rails_pro/spec/react_on_rails_pro/stream_spec.rb (1)
include(8-35)spec/dummy/spec/helpers/react_on_rails_helper_spec.rb (1)
include(6-9)packages/react-on-rails-pro/tests/AsyncQueue.ts (1)
dequeue(27-54)
react_on_rails_pro/spec/react_on_rails_pro/stream_spec.rb (1)
packages/react-on-rails-pro/tests/AsyncQueue.ts (2)
dequeue(27-54)enqueue(10-20)
packages/react-on-rails-pro-node-renderer/tests/concurrentHtmlStreaming.test.ts (2)
packages/react-on-rails-pro-node-renderer/tests/htmlStreaming.test.js (3)
createTestConfig(7-7)app(8-8)app(24-24)packages/react-on-rails-pro-node-renderer/tests/testingNodeRendererConfigs.ts (1)
createTestConfig(13-49)
packages/react-on-rails-pro-node-renderer/tests/htmlStreaming.test.js (1)
packages/react-on-rails-pro-node-renderer/tests/testingNodeRendererConfigs.ts (1)
createTestConfig(13-49)
react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb (2)
react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb (2)
render_to_string(17-17)response(18-18)react_on_rails_pro/spec/react_on_rails_pro/stream_spec.rb (1)
render_to_string(18-34)
🪛 LanguageTool
.claude/agents/pr-testing-guide.md
[style] ~32-~32: Try using a synonym here to strengthen your wording.
Context: ...h pr create`) - Responding to PR review comments about testing - CI failures occur and n...
(COMMENT_REMARK)
[grammar] ~494-~494: Ensure spelling is correct
Context: ...tion tests: rake run_rspec:dummy ### Environment Limitations **Mark any testing blocked...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
.claude/docs/analysis/INDEX.md
229-229: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.claude/agents/pr-testing-guide.md
56-56: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
70-70: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
82-82: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
92-92: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
101-101: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
110-110: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
119-119: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
130-130: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
160-160: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
191-191: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
204-204: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
229-229: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
264-264: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
288-288: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
295-295: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
302-302: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
309-309: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
555-555: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
599-599: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
628-628: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
667-667: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (7)
- GitHub Check: examples (3.4, latest)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: pro-lint-js-and-ruby
- GitHub Check: claude-review
🔇 Additional comments (25)
.github/workflows/examples.yml (1)
119-157: LGTM! Pnpm migration is correct.The workflow properly:
- Sets up pnpm/action-setup@v4 with store caching
- Uses
pnpm-lock.yamlfor deterministic builds- Installs with
--frozen-lockfile(good for CI reproducibility)- Replaces yarn commands with pnpm equivalents
- Uses workspace script invocation (
pnpm yalc:publish)Ensure
pnpm-lock.yamlexists and is committed to the repository (cache key on line 128 depends on it).CLAUDE.md (2)
58-100: Excellent comprehensive pnpm migration in CLAUDE.md.The updates correctly:
- Replace all yarn/npm commands with pnpm equivalents
- Maintain command syntax and semantics
- Preserve testing guidance while updating tooling references
- Add explicit guidance ("Only use pnpm as the JS package manager")
506-507: Clear policy on package manager enforcement.The explicit guidance to use only pnpm (never npm or yarn) is strong and clear. This prevents users from accidentally running incompatible package manager commands in a pnpm monorepo.
.github/workflows/lint-js-and-ruby.yml (1)
101-180: LGTM! Comprehensive pnpm migration in linting workflow.The workflow correctly:
- Sets up pnpm with action-setup, store path detection, and caching
- Replaces all yarn commands with pnpm equivalents
- Maintains linting tool invocations (eslint, stylelint, rubocop)
- Properly handles monorepo package operations (pnpm pack, attw, publint)
- Includes frozen-lockfile installs for reproducibility
Verify that all pnpm scripts referenced in this workflow (eslint, lint:scss, type-check, attw, publint, knip) are defined in the respective package.json files (root, packages/react-on-rails, spec/dummy).
.github/workflows/playwright.yml (1)
56-99: LGTM! Correct pnpm migration in Playwright workflow.The workflow properly:
- Sets up pnpm caching and store management
- Installs Playwright browsers with system dependencies via
pnpm exec- Uses monorepo install with frozen-lockfile for reproducibility
- Invokes E2E tests through package.json scripts
.github/workflows/pro-lint.yml (1)
100-182: LGTM! Proper pnpm migration including monorepo filtering.The workflow correctly:
- Sets up pnpm with consistent cache strategy
- Handles multiple apps (pro package, dummy, execjs-compatible-dummy)
- Uses
pnpm --filterfor workspace-targeted builds (line 167)- Invokes build scripts via nps through pnpm (lines 176, 179, 182)
- Maintains Pro-specific RBS and linting validation
.github/workflows/package-js-tests.yml (2)
101-112: pnpm setup and caching configuration looks correct.The pnpm action setup, store path detection, and cache configuration using
pnpm-lock.yamlfollow standard pnpm CI patterns. No issues here.
125-126: Verify pnpm conditional flag syntax and note yalc usage.Line 125 uses a bash ternary to conditionally add
--frozen-lockfile. This should expand correctly but ensure it's been tested. Additionally, while the PR objective is to eliminate yalc through native workspace linking, line 126 still installs it globally. Confirm this is intentional for the current migration phase..github/workflows/pro-test-package-and-gem.yml (2)
102-115: Consistent pnpm setup across both job blocks.Both the
build-dummy-app-webpack-test-bundlesandpackage-js-testsjobs have identical, correct pnpm setup patterns with proper store-path caching and frozen-lockfile flags.Also applies to: 206-219
252-252: Verify the filtered package and ci script exist.The command
pnpm --filter react-on-rails-pro-node-renderer ciuses correct pnpm workspace filtering syntax. Ensure that thereact-on-rails-pro-node-rendererpackage exists in the workspace and has aciscript defined in its package.json..github/workflows/pro-integration-tests.yml (3)
102-115: Consistent and correct pnpm setup across all three job blocks.All three integration test jobs maintain identical, properly configured pnpm setup patterns with correct store-path and cache handling. Good consistency across the workflow.
Also applies to: 201-214, 401-414
347-352: Good addition: pnpm debug log capture.Capturing
pnpm-debug.logon workflow failures is helpful for troubleshooting. The pathreact_on_rails_pro/spec/dummy/pnpm-debug.logis correct for logs generated by pnpm in the dummy app directory.
510-510: Verify Playwright and e2e-test script integration with pnpm.The commands
pnpm playwright install --with-depsandpnpm e2e-testreplace yarn equivalents. Confirm that theplaywrightpackage is properly accessible via pnpm and that thee2e-testscript exists in the dummy app's package.json.Also applies to: 513-513
packages/react-on-rails-pro-node-renderer/tests/vm.test.ts (2)
273-314: Assertion counts correctly updated.The assertion counts are accurate:
- Line 196: 5 components × 2 assertions = 10 ✓
- Line 273: 3 components × 2 assertions = 6 ✓
The validation pattern is consistent with the earlier tests.
317-371: Slightly stronger validation in BionicWorkshop tests.These tests use
toContain('<div')instead oftoContain('<'), which is marginally more specific. For consistency across all render tests, consider using the same pattern everywhere.react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb (2)
63-82: Sound async streaming architecture with proper cleanup.The barrier-based coordination is well-structured:
writing_taskconsumes chunks concurrently while component tasks produce themensureblock guarantees queue closure and writer completion- Error handling stops the barrier and re-raises appropriately
One observation: After
@async_barrier.stopon line 75, the barrier won't accept new tasks, but already-running tasks continue until completion. This is likely the desired behavior, but consider whether you want@async_barrier.stop(graceful) vs potentially more aggressive cleanup if an error occurs mid-stream.
51-57: Stream closing behavior is properly handled.Verification confirms that error paths correctly close the response stream. The
ApplicationController#rescue_fromhandler (line 45 ofreact_on_rails_pro/spec/dummy/app/controllers/application_controller.rb) writes diagnostic/error details to the stream and then closes it, exactly as the comment describes. The design is intentional and working correctly: the stream is kept open during error handling to allow error details to be sent, and the application layer handles closing it after writing diagnostic information.packages/react-on-rails-pro-node-renderer/tests/htmlStreaming.test.js (1)
3-7: Clean migration to factory-based test configuration.The switch to
createTestConfigaligns with the parallel test isolation strategy. ThebundlePathis correctly ignored since this test file doesn't need it directly—it's used internally within the config'sserverBundleCachePath.react_on_rails_pro/spec/react_on_rails_pro/stream_spec.rb (2)
18-34: Test mock correctly mirrors production async flow.The updated
render_to_stringmock properly simulates the barrier-based streaming:
- Creates async tasks via
@async_barrier.async- Drains component queues into
@main_output_queue- Uses
nilas the termination signal (matchingqueue.closebehavior)This aligns well with the production implementation in
stream.rb.
5-5: Let me examine the full file to verify whether this import is used:Let me check the full file content to understand the context:
Let me view the complete file to verify the context:
#!/bin/bash # View the full stream_spec.rb file cat react_on_rails_pro/spec/react_on_rails_pro/stream_spec.rbpackages/react-on-rails-pro-node-renderer/tests/concurrentHtmlStreaming.test.ts (1)
8-12: LGTM! Test config factory pattern improves test isolation.The migration to
createTestConfigfactory ensures each test gets an isolated bundle path, preventing race conditions during parallel test execution.react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb (2)
366-379: LGTM! Async context setup correctly simulates streaming environment.The
aroundblock properly initializes the async primitives (@async_barrierand@main_output_queue) within aSyncblock, matching the behavior thatstream_view_containing_react_componentswould provide in production.
392-425: Async streaming test correctly validates implementation.Verification confirms the test at lines 392-425 properly validates the async streaming flow:
- First chunk returned synchronously (line 401)
- Barrier wait ensures async task completion (line 404)
- Remaining chunks consumed from queue (lines 409-411)
- Expected chunk count of 2 remaining chunks verified (line 418)
Implementation in
consumer_stream_asynccorrectly aligns with these expectations: first chunk stored in Async::Variable and returned synchronously, while remaining chunks are enqueued to@main_output_queue.react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb (2)
131-137: LGTM! Clean extraction of streaming callback.The extraction of
on_completeand delegation toconsumer_stream_asyncmaintains the same public API while enabling the new async streaming architecture.
332-332: LGTM! Proper handling of empty stream edge case.Setting
first_chunk_var.value = nilwhen the stream has no chunks prevents thewaitcall from blocking indefinitely.
| ## Contact & Updates | ||
|
|
||
| These documents were generated on: 2025-11-19 | ||
| These documents were generated on: 2025-11-22 |
There was a problem hiding this comment.
Add language identifier or remove blank code block.
Line 229 has a fenced code block without a language specification. Either specify the language (e.g., bash, python) or remove the block.
-```
+```bash🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
229-229: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In .claude/docs/analysis/INDEX.md around line 229 there is an empty fenced code
block with no language specified; update the block by either adding an
appropriate language identifier (for example ```bash) and placing the intended
content inside, or remove the entire fenced block if no code/content is needed
so the file no longer contains an empty/unnamed code fence.
PR Review: Migration from Yarn Classic to pnpmThank you for this comprehensive migration PR! I've reviewed the changes and have several critical findings that need attention before this can be merged. 🚨 CRITICAL ISSUES1. Incomplete Migration - Package Manager Still Set to YarnThe root "packageManager": "[email protected]+sha512..."According to the PR description, this should be changed to Impact: This contradicts the entire purpose of the migration and will cause confusion for contributors. Files to update:
2. Missing pnpm Configuration FilesThe PR description mentions adding Expected files: # pnpm-workspace.yaml
packages:
- 'packages/*'# .npmrc
shamefully-hoist=true
strict-peer-dependencies=false3. GitHub Actions Still Configured for YarnAll workflow files are still using:
Files needing updates:
Required changes: # Change from:
cache: yarn
cache-dependency-path: '**/yarn.lock'
# To:
cache: pnpm
cache-dependency-path: '**/pnpm-lock.yaml'# Add pnpm setup before node setup:
- uses: pnpm/action-setup@v4
with:
version: 9.14.2# Change all commands:
yarn install --frozen-lockfile → pnpm install --frozen-lockfile
yarn run build → pnpm run build
yarn global add yalc → pnpm add -g yalc4. Rake Tasks Still Using Yarn# react_on_rails/rakelib/node_package.rake:10
sh "yarn run build && yarn yalc publish"This should be: sh "pnpm run build && pnpm yalc publish"Files to update:
5. Missing Workspace Protocol UpdatesThe PR description mentions converting workspace dependencies from Packages to check:
|
justin808
force-pushed
the
jg/yarn-to-pnpm
branch
2 times, most recently
from
355f31d to
491348a
Compare
PR Review: Yarn Classic → pnpm MigrationThank you for this comprehensive migration! This is a significant infrastructure change that will benefit the project. Here's my review: ✅ Strengths
|
justin808
force-pushed
the
jg/yarn-to-pnpm
branch
from
491348a to
288bc57
Compare
Code Review: Yarn to pnpm MigrationI've reviewed PR #2121 and the migration looks solid overall. Here's my detailed feedback: ✅ Strengths1. Proper Workspace Configuration
2. Correct Workspace Protocol UsageAll package.json files properly use
This is the correct pnpm workspace syntax (not Yarn's 3. Comprehensive CI Updates
4. Consistent Rake Task UpdatesAll rake files properly updated:
|
justin808
force-pushed
the
jg/yarn-to-pnpm
branch
from
288bc57 to
30198d6
Compare
Code Review: Yarn Classic to pnpm MigrationI've conducted a thorough review of PR #2121 migrating from Yarn Classic to pnpm. This is a significant infrastructure change that appears to be well-executed. Here's my analysis: ✅ Strengths1. Comprehensive Migration Approach
2. GitHub Actions Workflows Well-Updated
3. Good Documentation Updates
4. Proper Cleanup
|
justin808
force-pushed
the
jg/yarn-to-pnpm
branch
from
30198d6 to
88b8b09
Compare
Code Review: Yarn to pnpm MigrationI've reviewed this PR for the migration from Yarn Classic to pnpm. Overall, this is a well-structured migration with comprehensive updates across the codebase. Here's my detailed feedback: ✅ Strengths1. Comprehensive Workspace Configuration
2. Correct Workspace Protocol Migration
3. Thorough CI/CD Updates
4. Documentation Excellence
5. Rake Task Updates
|
justin808
force-pushed
the
jg/yarn-to-pnpm
branch
from
88b8b09 to
89f8e99
Compare
Comprehensive Code Review: Yarn → pnpm Migration🎯 Overall AssessmentThis is a well-executed migration from Yarn Classic to pnpm with comprehensive updates across the monorepo. The changes are methodical and appear complete. However, there is one important concern about scope creep. ✅ Strengths1. Comprehensive Scope
2. Proper pnpm Configuration
3. Good Testing Practices
|
| Category | Rating |
|---|---|
| Code Quality | ⭐⭐⭐⭐ |
| Test Coverage | ⭐⭐⭐ (waiting for CI) |
| Documentation | ⭐⭐⭐⭐ |
| Security | ⭐⭐⭐⭐⭐ |
| Performance | ⭐⭐⭐⭐⭐ |
| Overall | ⭐⭐⭐⭐ |
Great work! Main concern is mixing streaming refactor with package manager migration.
Reviewed per CLAUDE.md: code quality, bugs, performance, security, test coverage
Code Review: Yarn Classic to pnpm MigrationThis is a well-executed package manager migration with excellent attention to detail. Strengths1. Excellent Migration Approach
2. CI Integration Quality
3. Monorepo Configuration
4. Build Script Safety
Critical Concerns1. Bundled Unrelated ChangesThis PR includes 3 unrelated commits in addition to the pnpm migration:
The streaming refactor (#2111) changes react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb - this is a complex Pro feature change with behavior modifications. Recommendation: Consider splitting streaming changes into a separate PR OR clearly document why they're bundled. This makes rollback and bisecting easier if issues arise. 2. Missing --ignore-workspace Flag ExplanationIn integration-tests.yml, dummy apps use pnpm install --ignore-workspace. Why is this needed?
3. pnpm/action-setup Version InconsistencySome workflows specify version: 9 explicitly, others rely on packageManager field auto-detection. Recommend standardizing the approach. Testing Verification NeededPer CLAUDE.md, verify these LOCALLY before merging:
Minor Issues
Security ReviewNo security concerns detected:
Performance ConsiderationsExpected benefits:
Monitor CI build times for first 5-10 runs to establish new baseline. Final RecommendationsBefore Merging:
After Merging: SummaryOverall Assessment: Approve with minor recommendations This is a high-quality migration that is technically sound, well-documented, and follows monorepo best practices. Once CI completes successfully and bundled changes are addressed, this is ready to merge. Great work on a complex infrastructure change! |
justin808
force-pushed
the
jg/yarn-to-pnpm
branch
from
89f8e99 to
78cc390
Compare
Code Review: Migrate from Yarn Classic to pnpmThis is a comprehensive migration PR that modernizes the package management infrastructure. Overall, the changes are well-structured and follow best practices for pnpm monorepo management. Below is my detailed review: ✅ StrengthsPackage Manager Configuration
GitHub Actions Updates
Documentation
|
Code Review: Yarn Classic → pnpm MigrationOverall AssessmentThis is a well-executed package manager migration from Yarn Classic to pnpm. The changes are comprehensive and follow the project's testing guidelines. The PR properly updates all relevant files, workflows, and documentation. ✅ Strengths1. Comprehensive Migration
2. CI/CD Updates
3. Documentation
4. Cleanup
|
There was a problem hiding this comment.
Can we move all linting changes to a separate PR?
Migrate the react-on-rails monorepo from Yarn Classic (v1.22.22) to pnpm for improved monorepo management, faster installs, and native workspace linking that eliminates the need for yalc in internal development. Key changes: - Add pnpm-workspace.yaml and .npmrc for pnpm configuration - Update packageManager field to [email protected] in all package.json files - Convert workspace dependencies from "*" to "workspace:*" protocol - Move pnpm.overrides from workspace package to root - Replace yarn.lock with pnpm-lock.yaml - Update all GitHub Actions workflows with pnpm setup and commands - Update rake tasks to use pnpm commands - Update CONTRIBUTING.md and CLAUDE.md documentation Note for users: End users of react_on_rails can continue using their preferred package manager (npm, yarn, pnpm, or bun). The generators automatically detect your package manager. These changes only affect contributors working on the react_on_rails codebase itself. Closes #2120 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Remove explicit version: 9 parameter from pnpm/action-setup@v4 steps. The action reads the version from packageManager field in package.json, and specifying both causes a conflict error. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Fix pnpm filter command syntax (add 'run' for script execution) - Fix test:js script in dummy app to use jest directly - Update knip config: add ignored deps (nps, attw, publint, jest) - Remove non-existent src/index.ts entry from knip config - Clean up stale ignoreDependencies entries per knip hints - Fix prettier-format script to use pnpm exec instead of pnpm run 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Restore @babel/runtime and mini-css-extract-plugin to ignoreDependencies - Remove jest from ignoreDependencies (now properly detected) - Ignore disableHttp2 export (test-only utility) - Fix ESLint no-base-to-string error in LazyApolloGraphQL.tsx - Suppress pre-existing typescript-eslint errors in LazyApolloGraphQL.tsx - Fix eslint-lint script to use pnpm exec instead of pnpm run 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
### Summary #2121 missed some changes in scripts and docs. ### Pull Request checklist - [ ] ~Add/update test to cover these changes~ - [x] Update documentation - [ ] ~Update CHANGELOG file~ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Migrated documentation and developer workflows from Yarn to PNPM across install, build, test, lint, publish, workspace, and CI guidance. * Updated examples, checklists, verification steps, and lockfile references to reflect PNPM or package-manager-agnostic instructions. * Removed a legacy optional preinstall orchestration step and aligned scripts and setup guidance with the PNPM workflow. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Alexey Romanov <[email protected]> Co-authored-by: Claude Opus 4.5 <[email protected]>
1 participant
Summary
Migrate the react-on-rails monorepo from Yarn Classic (v1.22.22) to pnpm for improved monorepo management, faster installs, and native workspace linking.
Key Changes
Package Manager Configuration
pnpm-workspace.yamland.npmrcfor pnpm configurationpackageManagerfield to[email protected]in all package.json files"*"to"workspace:*"protocolpnpm.overridesfrom workspace package to rootLock Files
yarn.lockfilespnpm-lock.yamlGitHub Actions Workflows
yarn.locktopnpm-lock.yamlRake Tasks
rakelib/lint.rake,rakelib/node_package.rake,rakelib/dummy_apps.rake,rakelib/run_rspec.rakeDocumentation
CONTRIBUTING.mdandCLAUDE.mdNote for Users
End users of react_on_rails can continue using their preferred package manager (npm, yarn, pnpm, or bun). The generators automatically detect your package manager. These changes only affect contributors working on the react_on_rails codebase itself.
Test Plan
pnpm installcompletes successfullypnpm run buildproduces correct artifactsCloses #2120
🤖 Generated with Claude Code
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.