Fix RSC test infrastructure: Convert jest.setup.js to ESM and re-enable tests #2124
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Warning Rate limit exceeded@justin808 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 17 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
WalkthroughThe PR converts Changes
Sequence DiagramsequenceDiagram
participant npm as npm (test:rsc)
participant versionCheck as React Version Check
participant jest as Jest Runner
npm->>versionCheck: Read React version
alt React >= 19
versionCheck->>jest: Run RSC tests (tests/*.rsc.test.*)
jest-->>npm: Test results
else React < 19
versionCheck-->>npm: Exit with success (skip message)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/react-on-rails-pro/package.json (1)
12-12: Consider using a separate script file for version checking.The inline
require()call works becausenode -eevaluates code in a CommonJS context by default, but this is not immediately obvious given"type": "module"on Line 5. For better clarity and maintainability, consider moving this logic to a separate.cjsfile.Example implementation:
Create
scripts/check-react-version.cjs:const v = require('react/package.json').version; if (parseInt(v) < 19) { console.log('RSC tests skipped (requires React 19+, found ' + v + ')'); process.exit(0); } process.exit(1);Then update the script:
-"test:rsc": "node -e \"const v = require('react/package.json').version; if (parseInt(v) < 19) { console.log('RSC tests skipped (requires React 19+, found ' + v + ')'); process.exit(0); } process.exit(1);\" || NODE_CONDITIONS=react-server jest tests/*.rsc.test.*", +"test:rsc": "node scripts/check-react-version.cjs || NODE_CONDITIONS=react-server jest tests/*.rsc.test.*",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/react-on-rails-pro/package.json(1 hunks)packages/react-on-rails-pro/tests/jest.setup.js(1 hunks)script/convert(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
**/*: CRITICAL: Ensure all files end with a newline character
When making changes to directory structure or file paths, run comprehensive grep searches to find all affected files before committing:grep -r 'old/path' . --exclude-dir=node_modules --exclude-dir=.git
Files:
packages/react-on-rails-pro/package.jsonpackages/react-on-rails-pro/tests/jest.setup.jsscript/convert
**/*.{js,jsx,ts,tsx,json,yaml,yml,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
NEVER manually format code - let Prettier and RuboCop handle ALL formatting automatically via
rake autofix
Files:
packages/react-on-rails-pro/package.jsonpackages/react-on-rails-pro/tests/jest.setup.js
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Run ESLint linting via
yarn run lintorrake lint:eslintbefore committing JavaScript/TypeScript code
Files:
packages/react-on-rails-pro/tests/jest.setup.js
🧠 Learnings (23)
📓 Common learnings
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to react_on_rails_pro/**/*.{js,jsx,ts,tsx,json,yaml,yml,css,scss} : The react_on_rails_pro directory has its own Prettier and ESLint configuration - CI lints both directories separately
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to packages/react-on-rails/src/**/*.{ts,tsx} : TypeScript files in the npm package must be compiled to JavaScript in `packages/react-on-rails/lib/` via `yarn run build`
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.
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.
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to packages/react-on-rails/src/**/*.{ts,tsx} : TypeScript files in the npm package must be compiled to JavaScript in `packages/react-on-rails/lib/` via `yarn run build`
Applied to files:
packages/react-on-rails-pro/package.jsonpackages/react-on-rails-pro/tests/jest.setup.jsscript/convert
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to react_on_rails_pro/**/*.{js,jsx,ts,tsx,json,yaml,yml,css,scss} : The react_on_rails_pro directory has its own Prettier and ESLint configuration - CI lints both directories separately
Applied to files:
packages/react-on-rails-pro/package.jsonscript/convert
📚 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:
packages/react-on-rails-pro/package.jsonscript/convert
📚 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:
packages/react-on-rails-pro/package.json
📚 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.jsonscript/convert
📚 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/package.jsonscript/convert
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to spec/react_on_rails/**/*.rb : Ruby unit tests should be located in `spec/react_on_rails/` and tested via `rake run_rspec:gem`
Applied to files:
packages/react-on-rails-pro/package.jsonscript/convert
📚 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/package.jsonpackages/react-on-rails-pro/tests/jest.setup.jsscript/convert
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: CRITICAL: For infrastructure/config changes affecting directory structure, build configs, or CI workflows, perform comprehensive local testing before pushing including: grep for affected files, test build pipeline, run relevant specs, and run full lint check
Applied to files:
packages/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:
packages/react-on-rails-pro/package.jsonscript/convert
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Run ESLint linting via `yarn run lint` or `rake lint:eslint` before committing JavaScript/TypeScript code
Applied to files:
packages/react-on-rails-pro/package.json
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to **/*.ts : Run `yarn run type-check` to validate TypeScript type safety before committing
Applied to files:
packages/react-on-rails-pro/package.json
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to **/*.{js,jsx,ts,tsx,json,yaml,yml,css,scss} : NEVER manually format code - let Prettier and RuboCop handle ALL formatting automatically via `rake autofix`
Applied to files:
packages/react-on-rails-pro/package.jsonscript/convert
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to package.json : Never run `npm` commands - only use equivalent Yarn Classic commands
Applied to files:
packages/react-on-rails-pro/package.json
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Use `yalc` for local development when testing React on Rails with external applications - this allows local package linking without publishing to npm
Applied to files:
packages/react-on-rails-pro/package.json
📚 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/tests/jest.setup.jsscript/convert
📚 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:
packages/react-on-rails-pro/tests/jest.setup.js
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to spec/dummy/e2e/playwright/**/*.spec.js : Playwright E2E tests should use the `cypress-on-rails` gem helpers for Rails integration, including `app()`, `appEval()`, and `appFactories()` from `support/on-rails.js`
Applied to files:
packages/react-on-rails-pro/tests/jest.setup.js
📚 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/tests/jest.setup.js
📚 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:
script/convert
📚 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:
script/convert
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to lib/react_on_rails/**/*.rb : When creating new Ruby files in `lib/react_on_rails/`, create corresponding RBS signature file in `sig/react_on_rails/` and add to Steepfile for type checking
Applied to files:
script/convert
⏰ 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). (8)
- GitHub Check: build-dummy-app-webpack-test-bundles (3.4, 22, latest)
- GitHub Check: examples (3.4, latest)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: build
- GitHub Check: pro-lint-js-and-ruby
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: claude-review
🔇 Additional comments (2)
packages/react-on-rails-pro/tests/jest.setup.js (1)
1-4: LGTM! Clean ESM conversion.The conversion from CommonJS
require()to ESM imports is correct and complete. All necessary modules are imported at the top level to support the test environment polyfills.script/convert (1)
56-57: LGTM! Documentation accurately reflects the new behavior.The comments clearly explain that the
test:rscscript now handles React version detection automatically, which aligns with the implementation inpackages/react-on-rails-pro/package.json.
Code Review✅ Overall AssessmentThis PR successfully addresses issue #2122 by converting the Jest setup file from CommonJS to ESM and re-enabling RSC tests. The changes are well-scoped and appropriate. 📝 Code Quality & Best PracticesStrengths:
Minor Observations:
🐛 Potential Issues1. CommonJS in ESM Context (Line 12 in package.json) The test:rsc script uses require() inside a node -e command. Since package.json has "type": "module", this might cause issues in some contexts. Why this might work anyway: The node -e command runs in a separate Node.js process without the --input-type=module flag, so it defaults to CommonJS mode and require() should work. However, this is fragile. Suggested improvements for robustness: Option 1 - Use a separate script file for better maintainability ⚡ Performance ConsiderationsNo performance concerns. The changes are minimal and appropriate. 🔒 Security ConcernsNo security issues identified. 🧪 Test CoverageConcerns:
From PR description:
Recommendation:
📋 Additional Suggestions
🎯 VerdictApprove with Minor Concerns The core changes (ESM conversion of jest.setup.js) are solid and correctly implement the requirements. The main concerns are:
Recommended Action Items:
Great work on cleaning up the test infrastructure! 🚀 |
…le tests - Convert jest.setup.js from CommonJS require() to ESM imports for compatibility with ESM mode - Re-enable RSC tests with automatic React version detection: - Tests run normally on React 19+ where RSC features are available - Tests skip gracefully on React 18 with informative message - Remove unnecessary script/convert override since package.json now handles version checking automatically 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Address code review feedback: - Move inline version detection to scripts/check-react-version.cjs for better clarity and maintainability - Use .cjs extension since the parent package is ESM and we need require() - Add root Gemfile.lock to .gitignore (it's a delegate to react_on_rails/Gemfile) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
justin808
force-pushed
the
jg/fix-rsc-tests-esm
branch
from
158ac6c to
a13cba9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2122
jest.setup.jsfrom CommonJSrequire()to ESM imports for compatibility with ESM modescript/convertoverride sincepackage.jsonnow handles version checking automaticallyChanges
packages/react-on-rails-pro/tests/jest.setup.jsMoved
require()statements to top-level ESM imports:import { TextEncoder, TextDecoder } from 'util'import { Readable } from 'stream'import { ReadableStream, ReadableStreamDefaultReader } from 'stream/web'import { jest } from '@jest/globals'packages/react-on-rails-pro/package.jsonRe-enabled
test:rscscript with React version detection:RSC tests skipped (requires React 19+, found 18.0.0)script/convertRemoved the override that disabled RSC tests since the version check is now built into
package.json.Test plan
yarn testinpackages/react-on-rails-pro- passes with React 18 (RSC tests skipped)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.