-
-
Notifications
You must be signed in to change notification settings - Fork 638
Switch Pro packages to public distribution #1901
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughChanged publishing from scoped/private GitHub Packages to public registries: the pro node renderer package was renamed to Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Release task
participant NPM as npmjs.org
participant Ruby as RubyGems.org
rect rgb(220,245,255)
Note over Dev,NPM: Node renderer publish (PUBLIC)
Dev->>NPM: npm publish react-on-rails-pro-node-renderer
NPM-->>Dev: OTP / success
end
rect rgb(245,245,220)
Note over Dev,Ruby: Ruby gems publish (PUBLIC)
Dev->>Ruby: gem push react_on_rails-*.gem / react_on_rails_pro-*.gem
Ruby-->>Dev: OTP / success
end
Note over Dev: Post-release summary references npmjs.org and RubyGems.org only
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 - PR #1901: Switch Pro packages to public distributionOverviewThis PR transitions React on Rails Pro packages from GitHub Packages (private) to public registries (npmjs.org + RubyGems.org), maintaining runtime JWT enforcement for license validation. This is a strategic change that simplifies customer onboarding while keeping the security model intact. ✅ Strengths1. Clean, Focused Changes
2. Well-Documented Approach
3. Consistent with Established Patterns
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
rakelib/release.rake(4 hunks)react_on_rails_pro/package.json(0 hunks)
💤 Files with no reviewable changes (1)
- react_on_rails_pro/package.json
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
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.
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.
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.
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.
📚 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:
rakelib/release.rake
📚 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:
rakelib/release.rake
📚 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:
rakelib/release.rake
📚 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:
rakelib/release.rake
📚 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:
rakelib/release.rake
📚 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:
rakelib/release.rake
📚 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:
rakelib/release.rake
⏰ 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). (10)
- GitHub Check: build
- GitHub Check: lint-js-and-ruby
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: build-dummy-app-webpack-test-bundles (3.4, 22)
- GitHub Check: claude-review
- GitHub Check: build-dummy-app-webpack-test-bundles
| puts "\nPublishing #{node_renderer_name}@#{actual_npm_version}..." | ||
| puts "Carefully add your OTP for NPM when prompted." unless use_verdaccio | ||
| sh_in_dir(pro_gem_root, | ||
| "yarn publish --new-version #{actual_npm_version} --no-git-tag-version #{npm_publish_args}") | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add --access public when publishing the scoped renderer package
Scoped npm packages default to private. On the first public release, yarn publish without --access public fails with the npm error “You must provide --access public to publish scoped packages.” That blocks the new distribution flow this PR introduces. Please append the flag (while skipping it for Verdaccio runs) so the publish step succeeds.
- sh_in_dir(pro_gem_root,
- "yarn publish --new-version #{actual_npm_version} --no-git-tag-version #{npm_publish_args}")
+ publish_command = [
+ "yarn publish",
+ "--new-version #{actual_npm_version}",
+ "--no-git-tag-version"
+ ]
+ publish_command << npm_publish_args unless npm_publish_args.to_s.empty?
+ publish_command << "--access public" unless use_verdaccio
+ sh_in_dir(pro_gem_root, publish_command.join(" "))🤖 Prompt for AI Agents
In rakelib/release.rake around lines 217 to 221, the yarn publish command needs
the --access public flag for scoped packages; update the sh_in_dir call so that
when use_verdaccio is false you append "--access public" to the yarn publish
arguments (while preserving existing npm_publish_args and --no-git-tag-version),
and do not add the flag when use_verdaccio is true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this is resolved since we removed the package scope now. can you check and approve the pr for merge?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ihabadham You're absolutely correct! Since unscoped packages are always public on npm, the --access public flag is not needed for react-on-rails-pro-node-renderer.
I can see from the current code that the package has been changed from the scoped @shakacode-tools/react-on-rails-pro-node-renderer to the unscoped react-on-rails-pro-node-renderer, which completely resolves the concern I raised.
The PR looks good overall:
- ✅ Unified versioning approach is solid
- ✅ Publishing flow correctly handles both npm and RubyGems
- ✅ Verdaccio testing support is maintained
- ✅ Git operations are properly sequenced
- ✅ Package scope removed, making public publishing straightforward
Approving the PR for merge! 🎉
🧠 Learnings used
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.
Code Review - PR #1901: Switch Pro packages to public distributionSummaryThis PR transitions the Pro packages from private GitHub Packages distribution to public npm/RubyGems distribution, simplifying customer installation while maintaining runtime JWT license validation. ✅ Code Quality & Best PracticesStrengths:
Observations:
🐛 Potential IssuesMinor:
🔒 Security ConsiderationsStrong points:
Recommendations:
⚡ Performance ConsiderationsPositive impacts:
Neutral:
🧪 Test CoverageCurrent state:
Testing plan review:
Recommendations:
📝 Documentation & MessagingUpdates needed:
Suggestions:
🎯 Breaking Changes AssessmentFor customers:
For maintainers:
🚀 Release ReadinessBefore merging:
After merging (first release):
📊 Overall AssessmentRating: ✅ Approve with minor fixes This is a well-structured PR that achieves its stated goal of simplifying distribution. The code changes are clean and focused. The main blocker is the failing test, which should be resolved before merge. The strategic decision to move to public distribution with runtime license enforcement is sound and significantly improves customer experience. Estimated risk: Low (assuming runtime validation is well-tested) 🤖 Review generated by Claude Code |
Code Review: Switch Pro packages to public distributionOverviewThis PR implements a significant architectural change, moving React on Rails Pro packages from private GitHub Packages to public distribution (npmjs.org + RubyGems.org). I've reviewed the changes against code quality, security, performance, and best practices. ✅ Strengths1. Clean Implementation
2. Security Model Intact
3. Good Code Quality
|
PR Review: Switch Pro packages to public distributionI've reviewed PR #1901 comprehensively. Here's my detailed feedback: ✅ Code Quality & Best PracticesStrengths:
Areas of Concern:
🐛 Potential Issues
🔒 Security ConsiderationsPositive:
Recommendations:
⚡ Performance ConsiderationsNo performance concerns - this is purely a distribution change. The actual package contents and runtime behavior are unchanged. 🧪 Test CoverageConcerns:
📝 RecommendationsCritical (Must fix before merge):
Important (Should fix):
Nice to have:
📊 Overall AssessmentThis is a well-executed change that accomplishes its goal cleanly. The code changes are solid, but the documentation is significantly out of date and must be updated before this can be merged. The installation docs still instruct users to set up GitHub PATs and authenticate with GitHub Packages, which will confuse users after the switch. Recommendation: Request changes - Update documentation before merging. Review generated by Claude Code - Let me know if you'd like me to help update the documentation files! |
Code Review: Switch Pro packages to public distributionI've reviewed PR #1901 and have the following feedback: ✅ Strengths
🔍 Issues & ConcernsCritical Issues
Medium Priority Issues
🔐 Security Considerations✅ Runtime enforcement preserved: The PR correctly maintains JWT validation at runtime 📋 RecommendationsBefore merging:
Testing checklist additions:
📝 Minor Suggestions
Overall AssessmentThis is a well-intentioned and strategically sound change that will significantly improve the user experience. However, it's not ready to merge due to the critical documentation gap. The installation docs must be updated before this can safely go out, as it will cause immediate confusion and support burden. Recommended path forward:
Would you like me to help with updating the installation documentation or creating a migration guide? |
Following PR #1901's switch from GitHub Packages to public distribution, update all documentation to reflect that React on Rails Pro packages are now publicly available on npmjs.org and RubyGems.org. Changes: - Remove all GitHub Personal Access Token requirements - Remove .npmrc and gem credentials configuration steps - Simplify installation to standard gem/npm commands - Update releasing docs to show all 5 packages as public - Remove "private repository" language throughout - Update troubleshooting to remove token-related issues Files updated: - react_on_rails_pro/docs/installation.md (major simplification) - docs/contributor-info/releasing.md - react_on_rails_pro/docs/contributors-info/releasing.md - react_on_rails_pro/docs/contributors-info/onboarding-customers.md - react_on_rails_pro/docs/troubleshooting.md - react_on_rails_pro/docs/ruby-api.md - react_on_rails_pro/docs/code-splitting-loadable-components.md - react_on_rails_pro/docs/node-renderer/basics.md - react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
react_on_rails_pro/docs/installation.md (1)
56-68: Clarify initializer requirement vs. environment variable setup.Line 58 states "You don't need to create an initializer if you are satisfied with the defaults," but the license configuration (lines 39-54) appears to require setting the license token. Clarify whether this statement assumes the license token is always configured via environment variable, and update the text to make this explicit.
Example revision for line 58:
"You don't need to create an initializer if you've configured the license via environment variable and are satisfied with other defaults."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
CONTRIBUTING.md(1 hunks)docs/contributor-info/releasing.md(1 hunks)react_on_rails_pro/CONTRIBUTING.md(1 hunks)react_on_rails_pro/docs/code-splitting-loadable-components.md(1 hunks)react_on_rails_pro/docs/contributors-info/releasing.md(1 hunks)react_on_rails_pro/docs/installation.md(1 hunks)react_on_rails_pro/docs/node-renderer/basics.md(1 hunks)react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.md(7 hunks)react_on_rails_pro/docs/node-renderer/js-configuration.md(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- react_on_rails_pro/docs/code-splitting-loadable-components.md
- docs/contributor-info/releasing.md
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
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.
📚 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:
CONTRIBUTING.mdreact_on_rails_pro/docs/node-renderer/basics.mdreact_on_rails_pro/docs/node-renderer/js-configuration.mdreact_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.mdreact_on_rails_pro/CONTRIBUTING.mdreact_on_rails_pro/docs/contributors-info/releasing.mdreact_on_rails_pro/docs/installation.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:
CONTRIBUTING.mdreact_on_rails_pro/docs/node-renderer/basics.mdreact_on_rails_pro/docs/node-renderer/js-configuration.mdreact_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.mdreact_on_rails_pro/CONTRIBUTING.mdreact_on_rails_pro/docs/contributors-info/releasing.mdreact_on_rails_pro/docs/installation.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:
CONTRIBUTING.mdreact_on_rails_pro/docs/node-renderer/basics.mdreact_on_rails_pro/docs/node-renderer/js-configuration.mdreact_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.mdreact_on_rails_pro/CONTRIBUTING.mdreact_on_rails_pro/docs/contributors-info/releasing.mdreact_on_rails_pro/docs/installation.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:
CONTRIBUTING.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:
CONTRIBUTING.mdreact_on_rails_pro/docs/node-renderer/js-configuration.md
📚 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:
react_on_rails_pro/docs/node-renderer/js-configuration.mdreact_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.mdreact_on_rails_pro/CONTRIBUTING.mdreact_on_rails_pro/docs/contributors-info/releasing.mdreact_on_rails_pro/docs/installation.md
📚 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:
react_on_rails_pro/docs/node-renderer/js-configuration.md
📚 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/docs/node-renderer/js-configuration.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:
react_on_rails_pro/docs/node-renderer/js-configuration.mdreact_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.md
📚 Learning: 2025-06-09T07:58:02.646Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1696
File: node_package/src/RSCPayloadGenerator.ts:124-168
Timestamp: 2025-06-09T07:58:02.646Z
Learning: In React Server Components (RSC) implementations, explicit error handling in RSC payload generation streams (like in RSCPayloadGenerator.ts) is not needed because errors will propagate to client components that fetch the RSC payloads (such as RSCRoute components). React's built-in error handling mechanisms and ErrorBoundary components are sufficient for handling these errors at the component level, which allows for better user experience with appropriate error UI fallbacks.
Applied to files:
react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.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:
react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.mdreact_on_rails_pro/CONTRIBUTING.mdreact_on_rails_pro/docs/installation.md
📚 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/CONTRIBUTING.mdreact_on_rails_pro/docs/contributors-info/releasing.mdreact_on_rails_pro/docs/installation.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/docs/installation.md
⏰ 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)
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: build
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: lint-js-and-ruby
- GitHub Check: build-dummy-app-webpack-test-bundles (3.4, 22, latest)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: claude-review
- GitHub Check: markdown-link-check
🔇 Additional comments (15)
CONTRIBUTING.md (1)
131-131: LGTM.The yalc workflow documentation now correctly references the public package names (
react-on-rails-pro-node-renderer), consistent with the broader PR changes.react_on_rails_pro/CONTRIBUTING.md (2)
295-301: LGTM.Prerequisites section correctly updated to reflect public registry authentication (npmjs.org and RubyGems.org), removing private GitHub Packages references. Clear and actionable guidance for contributors.
303-317: LGTM.Release command section provides clear, concrete examples (dry-run, Verdaccio test, full release) with proper reference to root-level documentation. Aligns with the testing plan outlined in PR objectives.
react_on_rails_pro/docs/node-renderer/js-configuration.md (1)
57-57: LGTM.Import path updated to reflect the unscoped public package name for consistency with public npm publishing.
react_on_rails_pro/docs/contributors-info/releasing.md (1)
35-40: LGTM.Package list updated to reflect new public distribution:
react-on-rails-pro-node-renderer(NPM) is now listed separately, and Pro gem is correctly listed as RubyGem (removing GitHub Packages reference). Accurate and complete.react_on_rails_pro/docs/node-renderer/basics.md (2)
36-45: LGTM.Installation instructions correctly updated to use public package name. Documentation flow is clear and follows standard npm conventions.
45-45: Module import syntax inconsistency across documentation.Line 45 uses default import:
import reactOnRailsProNodeRenderer from 'react-on-rails-pro-node-renderer', but other documentation (e.g.,js-configuration.md:57) shows destructured import:import { reactOnRailsProNodeRenderer } from '...'Verify which is correct and standardize across all docs to avoid user confusion.
react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.md (1)
17-17: LGTM.All import paths comprehensively updated to the unscoped public package name across multiple integration examples (Sentry, Honeybadger, BugSnag, Fastify API). Changes are consistent and thorough.
Also applies to: 20-20, 43-43, 52-52, 62-62, 79-79, 120-120, 145-145
react_on_rails_pro/docs/installation.md (7)
3-11: LGTM.Introduction clearly establishes the public distribution model and license token requirement. Contact information for purchasing is helpful for users.
13-37: LGTM.Ruby gem installation section is clear and comprehensive, providing both standard gem declaration and direct install options with version guidance. Follows established Ruby patterns.
39-54: LGTM.License configuration section provides both environment variable (recommended) and initializer approaches, with helpful production guidance. Clear and actionable for users.
70-96: LGTM.Node package installation section is clear and comprehensive, showing both npm and yarn options with package.json example. Consistent use of public package name.
98-141: Module syntax inconsistency with other documentation.This section uses CommonJS require/destructuring syntax (line 104:
const { reactOnRailsProNodeRenderer } = require(...)), butdocs/node-renderer/basics.md:45uses ES6 default import syntax (import reactOnRailsProNodeRenderer from ...). Both sections describe setting up a custom configuration file for the same package.Verify the actual package export style and standardize the examples across all documentation to avoid user confusion.
143-175: LGTM.Node renderer startup and Rails configuration sections are well-structured. package.json script, startup command, and Rails initializer example are clear and correctly reference the configuration options. The password field correctly maps to the example configuration above.
177-195: LGTM.Configuration options summary is helpful and provides clear bullet-point reference to key settings. Proper links to detailed documentation files.
581605d to
32ef93d
Compare
Code Review - Switch Pro Packages to Public DistributionI've completed a thorough review of PR #1901. This is a significant architectural change that transitions React on Rails Pro packages from private GitHub Packages distribution to public npmjs.org and RubyGems.org distribution. Overall, the implementation is well-executed and clean, but I have some recommendations and observations. ✅ Strengths1. Clean Code Changes
2. Comprehensive Documentation Updates
3. Good Separation of Concerns
🔍 Code Quality & Best Practicesrakelib/release.rake (Lines 210-220)Good:
Observation: # Note: Using 'yarn publish' here instead of 'yarn workspace' because
# the node-renderer package.json is in react_on_rails_pro/ root directory
sh_in_dir(pro_gem_root,
"yarn publish --new-version #{actual_npm_version} --no-git-tag-version #{npm_publish_args}")Documentation Consistency (installation.md)Good:
Minor suggestions:
|
|
@claude review this PR |
|
Claude finished @justin808's task —— View job Code Review: Switch Pro packages to public distributionI've completed a comprehensive review of PR #1901. This PR transitions React on Rails Pro packages from private GitHub Packages to public npmjs.org and RubyGems.org distribution, which is a significant architectural change that will greatly simplify the customer experience. Overall AssessmentStatus: ✅ Approve This is a well-executed PR that achieves its stated goals cleanly and safely. All CI checks are passing, documentation has been thoroughly updated, and the security model remains intact through runtime JWT validation. ✅ Strengths1. Clean Code Changes
2. Comprehensive Documentation Updates
3. Sound Architecture
4. Testing & CI
📋 Code Quality Reviewrakelib/release.rake (Lines 29-35, 196-241)Excellent improvements:
Minor observation: react_on_rails_pro/package.json (Lines 1-159)Key changes:
Note: I see the previous CodeRabbit review mentioned the repository URL still pointing to Documentation (installation.md, CONTRIBUTING.md, etc.)Excellent updates:
One observation from CodeRabbit:
This is actually fine - one shows CommonJS, the other shows ES modules. Both are valid. 🔒 Security AnalysisRuntime Security (Unchanged - Correct)✅ JWT validation at Rails startup Distribution Security (New Model)✅ Public distribution doesn't expose secrets (packages secured by runtime license checks) Recommendation: Ensure 2FA is enabled on npmjs.org and RubyGems.org publishing accounts (mentioned in release script prompts). 🎯 Breaking Changes & MigrationFor Existing Customers:This is a breaking change for customers currently using GitHub Packages. They will need to:
PR Description addresses this: "Existing customers using GitHub Packages will need to update their This is appropriate since the customer base is small and can be communicated with directly. 🧪 Testing & Release ProcessTesting Plan (from PR description):
Recommendation: Execute the remaining testing checklist items in a staging environment before the actual release. Release Script Robustness:✅ Dry-run mode available 📝 Minor Suggestions (Non-blocking)
🚀 Release Readiness Checklist
Before actual release:
💯 SummaryThis PR represents a significant strategic improvement that will:
The implementation is clean, thorough, and well-documented. All previous review feedback has been addressed. The only remaining items are runtime testing during the actual release process. Recommendation: Approve and merge (after updating CHANGELOG per project guidelines) Great work on this strategic improvement! 🎉 Review Checklist (per CLAUDE.md)
|
53ed09c to
e68fe4f
Compare
Code Review: Switch Pro Packages to Public DistributionOverviewThis PR successfully implements the transition from GitHub Packages (private) to public distribution (npmjs.org + RubyGems.org) for React on Rails Pro packages. The changes are well-structured and consistent across the codebase. ✅ Strengths1. Excellent Consistency
2. Clean Release Script Refactoring (rakelib/release.rake:207-240)
3. Comprehensive Documentation Updates
4. Security Considerations
🔍 Issues & Recommendations1. Critical: package.json Repository Inconsistency
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
react_on_rails_pro/docs/installation.md (2)
13-57: Consider adding migration guidance for existing users.The installation documentation has been updated to reflect public distribution, but the PR objectives mention a breaking change: "existing customers using GitHub Packages must update their .npmrc." Consider adding a migration section or callout box for existing users who need to transition from GitHub Packages authentication to the new public registry setup.
Example addition after the Prerequisites section:
## Migrating from GitHub Packages If you previously installed React on Rails Pro from GitHub Packages: 1. Remove GitHub Packages authentication from your `.npmrc`: ```bash # Remove lines containing @shakacode-tools or npm.pkg.github.com
Update your Gemfile to remove any
sourceblocks for GitHub PackagesReinstall packages:
bundle install yarn install # or npm installConfigure your license token as described below
--- `39-56`: **Strengthen security guidance for license token.** While the security warning at line 56 is good, consider making it more prominent since the license token is critical for both gem and node renderer authentication. A warning box or banner at the start of the License Configuration section would be more visible. Consider restructuring like this: ```markdown ## License Configuration ⚠️ **Security Warning**: Your license token is a sensitive credential that authenticates both the Ruby gem and Node renderer. Never commit it to version control. Always use environment variables or secure secret management systems (like Rails credentials, Heroku config vars, AWS Secrets Manager, etc.). Set your license token as an environment variable: ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
react_on_rails_pro/spec/dummy/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (15)
CONTRIBUTING.md(1 hunks)docs/contributor-info/releasing.md(1 hunks)rakelib/release.rake(5 hunks)react_on_rails_pro/CONTRIBUTING.md(1 hunks)react_on_rails_pro/docs/code-splitting-loadable-components.md(1 hunks)react_on_rails_pro/docs/contributors-info/releasing.md(1 hunks)react_on_rails_pro/docs/installation.md(1 hunks)react_on_rails_pro/docs/node-renderer/basics.md(1 hunks)react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.md(7 hunks)react_on_rails_pro/docs/node-renderer/js-configuration.md(1 hunks)react_on_rails_pro/package-scripts.yml(1 hunks)react_on_rails_pro/package.json(2 hunks)react_on_rails_pro/packages/node-renderer/src/integrations/api.ts(1 hunks)react_on_rails_pro/spec/dummy/client/node-renderer.js(2 hunks)react_on_rails_pro/spec/dummy/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- react_on_rails_pro/spec/dummy/client/node-renderer.js
- react_on_rails_pro/package.json
- react_on_rails_pro/docs/node-renderer/js-configuration.md
- CONTRIBUTING.md
- react_on_rails_pro/docs/node-renderer/basics.md
- react_on_rails_pro/spec/dummy/package.json
- react_on_rails_pro/packages/node-renderer/src/integrations/api.ts
- docs/contributor-info/releasing.md
🧰 Additional context used
🧠 Learnings (9)
📚 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:
react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.mdreact_on_rails_pro/docs/installation.mdreact_on_rails_pro/docs/contributors-info/releasing.mdreact_on_rails_pro/docs/code-splitting-loadable-components.mdreact_on_rails_pro/package-scripts.ymlreact_on_rails_pro/CONTRIBUTING.mdrakelib/release.rake
📚 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:
react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.mdreact_on_rails_pro/docs/code-splitting-loadable-components.mdreact_on_rails_pro/package-scripts.yml
📚 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:
react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.mdreact_on_rails_pro/docs/installation.mdreact_on_rails_pro/docs/contributors-info/releasing.mdreact_on_rails_pro/docs/code-splitting-loadable-components.mdreact_on_rails_pro/package-scripts.ymlreact_on_rails_pro/CONTRIBUTING.mdrakelib/release.rake
📚 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:
react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.mdreact_on_rails_pro/docs/installation.mdreact_on_rails_pro/docs/contributors-info/releasing.mdreact_on_rails_pro/docs/code-splitting-loadable-components.mdreact_on_rails_pro/package-scripts.ymlreact_on_rails_pro/CONTRIBUTING.mdrakelib/release.rake
📚 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:
react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.mdreact_on_rails_pro/docs/installation.mdreact_on_rails_pro/docs/code-splitting-loadable-components.mdreact_on_rails_pro/package-scripts.ymlreact_on_rails_pro/CONTRIBUTING.mdrakelib/release.rake
📚 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:
react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.mdreact_on_rails_pro/docs/installation.mdreact_on_rails_pro/docs/contributors-info/releasing.mdreact_on_rails_pro/docs/code-splitting-loadable-components.mdreact_on_rails_pro/package-scripts.ymlreact_on_rails_pro/CONTRIBUTING.md
📚 Learning: 2025-06-09T07:58:02.646Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1696
File: node_package/src/RSCPayloadGenerator.ts:124-168
Timestamp: 2025-06-09T07:58:02.646Z
Learning: In React Server Components (RSC) implementations, explicit error handling in RSC payload generation streams (like in RSCPayloadGenerator.ts) is not needed because errors will propagate to client components that fetch the RSC payloads (such as RSCRoute components). React's built-in error handling mechanisms and ErrorBoundary components are sufficient for handling these errors at the component level, which allows for better user experience with appropriate error UI fallbacks.
Applied to files:
react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.md
📚 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/docs/installation.mdreact_on_rails_pro/docs/contributors-info/releasing.mdreact_on_rails_pro/package-scripts.ymlreact_on_rails_pro/CONTRIBUTING.mdrakelib/release.rake
📚 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/docs/installation.mdreact_on_rails_pro/docs/code-splitting-loadable-components.md
⏰ 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). (5)
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: lint-js-and-ruby
- GitHub Check: markdown-link-check
- GitHub Check: claude-review
🔇 Additional comments (12)
react_on_rails_pro/package-scripts.yml (1)
50-50: Error message correctly updated to reflect new package name.The prepack script error message now accurately reflects the unscoped public package name
react-on-rails-pro-node-renderer, consistent with the package name change inpackage.jsonas part of the switch to public distribution.react_on_rails_pro/docs/code-splitting-loadable-components.md (1)
248-248: LGTM - Package import updated correctly.The require path has been updated to use the new unscoped public package name, consistent with the PR's migration to public npm distribution.
react_on_rails_pro/CONTRIBUTING.md (1)
350-356: LGTM - Authentication documentation updated for public registries.The documentation correctly reflects the new public publishing strategy, removing GitHub Packages references and clarifying the authentication requirements for npmjs.org and RubyGems.org.
rakelib/release.rake (4)
32-32: LGTM - Node renderer added to public package list.The node renderer package is now correctly listed under PUBLIC packages, reflecting the migration from GitHub Packages to npmjs.org.
210-220: LGTM - Node renderer publishing updated correctly.The publishing flow correctly handles the unscoped package name. Since
react-on-rails-pro-node-rendereris unscoped (no@prefix), the--access publicflag is not needed—unscoped npm packages are public by default.Note: The previous review comment about adding
--access publicis no longer applicable after the package was renamed from@shakacode-tools/react-on-rails-pro-node-renderer(scoped) toreact-on-rails-pro-node-renderer(unscoped).
234-240: LGTM - Pro gem publishing migrated to RubyGems.org.The Pro gem publishing has been correctly updated to target RubyGems.org instead of GitHub Packages, using the standard
gem releasecommand without private registry flags.
278-285: LGTM - Success messaging reflects unified public distribution.The completion messages correctly show all packages published to public registries (npmjs.org and RubyGems.org), including the unscoped node renderer package.
Also applies to: 305-305
react_on_rails_pro/docs/contributors-info/releasing.md (1)
38-40: LGTM - Package names updated correctly.The documentation correctly lists the unscoped package names for the node renderer and Pro gem, consistent with the public distribution strategy.
react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.md (1)
17-17: LGTM - All integration import paths updated consistently.All require/import statements have been correctly updated from the scoped package (
@shakacode-tools/react-on-rails-pro-node-renderer) to the unscoped public package (react-on-rails-pro-node-renderer), maintaining consistency across all integration examples (Sentry, Honeybadger, Bugsnag, custom integrations, and Fastify hooks).Also applies to: 20-20, 43-43, 52-52, 62-62, 79-79, 120-120, 145-145
react_on_rails_pro/docs/installation.md (3)
1-12: LGTM - Clear version format documentation.The addition of the Version Format section with explicit examples of how pre-release versions differ between gems (dots) and NPM packages (dashes) is helpful and prevents potential confusion during installation.
72-154: LGTM - Comprehensive node renderer setup documentation.The node renderer installation and setup documentation is thorough and provides clear examples for both npm and yarn workflows. The JavaScript configuration example is well-commented and covers all essential options including worker configuration, password setup, and environment-specific adjustments.
155-189: LGTM - Clear Rails configuration for node renderer.The Rails configuration section provides clear guidance on connecting to the node renderer with appropriate cross-references to the password setup in the JavaScript configuration.
- Fix bugs/homepage URLs in package.json to point to monorepo - Add prominent link to upgrading guide in installation.md - Update version examples to 16.2 for consistency with release Addresses Claude AI review feedback on PR #1901 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
react_on_rails_pro/docs/installation.md (2)
43-48: Add language identifier to code block.The bash code block is missing a language identifier, which affects syntax highlighting and accessibility.
Apply this diff:
-``` +```bash export REACT_ON_RAILS_PRO_LICENSE="your-license-token-here"--- `183-193`: **Add language identifier to Configuration Options heading.** The "Configuration Options" section heading at line 183 introduces important configuration guidance. However, there's a minor formatting issue at line 185 where a code block is missing a language identifier. If there's a fenced code block around line 185, ensure it has a language identifier like: ````diff -``` +```ruby # configuration example</blockquote></details> <details> <summary>react_on_rails_pro/docs/updating.md (2)</summary><blockquote> `43-48`: **Add language identifier to .npmrc code block.** The .npmrc configuration example is missing a language identifier. While `.npmrc` files use a specific format, using `ini` or `properties` as the language identifier would improve rendering. Apply this diff: ````diff -``` +```ini always-auth=true //npm.pkg.github.com/:_authToken=YOUR_TOKEN @shakacode-tools:registry=https://npm.pkg.github.com ```
185-189: Add language identifier to expected output block.The expected console output is missing a language identifier. Using
textorconsolewould improve rendering.Apply this diff:
-``` +```text React on Rails Pro license validated successfully ```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
rakelib/release.rake(5 hunks)react_on_rails_pro/docs/installation.md(1 hunks)react_on_rails_pro/docs/updating.md(1 hunks)react_on_rails_pro/package.json(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- react_on_rails_pro/package.json
🧰 Additional context used
🧠 Learnings (7)
📚 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:
rakelib/release.rakereact_on_rails_pro/docs/installation.mdreact_on_rails_pro/docs/updating.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:
rakelib/release.rakereact_on_rails_pro/docs/installation.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:
rakelib/release.rakereact_on_rails_pro/docs/installation.mdreact_on_rails_pro/docs/updating.md
📚 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:
rakelib/release.rakereact_on_rails_pro/docs/installation.mdreact_on_rails_pro/docs/updating.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:
rakelib/release.rakereact_on_rails_pro/docs/installation.mdreact_on_rails_pro/docs/updating.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/docs/installation.mdreact_on_rails_pro/docs/updating.md
📚 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:
react_on_rails_pro/docs/installation.md
🪛 markdownlint-cli2 (0.18.1)
react_on_rails_pro/docs/installation.md
43-43: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
185-185: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
react_on_rails_pro/docs/updating.md
43-43: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
185-185: 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). (4)
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: lint-js-and-ruby
- GitHub Check: markdown-link-check
🔇 Additional comments (6)
rakelib/release.rake (2)
218-222: Past review concern resolved by unscoped package name.The past review comment flagged the need for
--access publicwhen publishing scoped packages. However, the package is now unscoped (react-on-rails-pro-node-rendererwithout@scope/prefix), which defaults to public access on npmjs.org. No additional flag is needed.
236-242: LGTM! Pro gem publishing correctly updated to RubyGems.org.The Pro gem publishing has been correctly updated to target RubyGems.org (public) instead of GitHub Packages. The removal of
--key github --hostarguments and updated messaging align with the public distribution strategy.react_on_rails_pro/docs/installation.md (1)
59-59: Excellent security guidance!The security warning about never committing license tokens is critical and well-placed. This helps prevent common security mistakes.
react_on_rails_pro/docs/updating.md (3)
1-28: Excellent migration guide introduction!The guide clearly explains who it's for, what's changing, and the benefits of the migration. The comparison between old (scoped) and new (unscoped) package names is particularly helpful.
158-160: Good security practice emphasized.The security warning and guidance about environment variables/secret management systems is essential for production deployments. The specific examples (Rails credentials, Heroku config vars, AWS Secrets Manager) are helpful.
197-216: Comprehensive troubleshooting section.The troubleshooting section covers the most common migration issues with clear solutions. This will significantly reduce support burden during the transition.
Remove GitHub Packages configuration to publish Pro packages publicly on npmjs.org and RubyGems.org. Changes: - Remove publishConfig from react_on_rails_pro/package.json (node-renderer) - Update release script to publish Pro gem to RubyGems.org (remove --key github --host) - Update all messaging from PRIVATE/GitHub Packages to PUBLIC/npmjs.org/RubyGems.org - Update script documentation to reflect all packages are now public - Update success message for unified public distribution This follows Bob's pattern from react-on-rails-pro NPM package (already configured for public). Runtime enforcement via JWT license validation remains unchanged.
Change package name from @shakacode-tools/react-on-rails-pro-node-renderer to react-on-rails-pro-node-renderer (unscoped). Rationale: - @shakacode-tools scope was only needed for GitHub Packages (requires scoping) - Now publishing to public npmjs.org where scoping is optional - Matches naming pattern of react-on-rails and react-on-rails-pro (both unscoped) - Unscoped packages are public by default (no --access public flag needed) - @shakacode-tools npm org doesn't exist on npmjs.org Migration impact: Small number of Pro customers will need to update their node-renderer.js imports when upgrading (Justin will handle directly). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Remove all references to GitHub Packages and private distribution. Update package name from @shakacode-tools/react-on-rails-pro-node-renderer to react-on-rails-pro-node-renderer throughout documentation. Changes: - Completely rewrite installation.md to remove GitHub PAT authentication - Simplify to standard gem install and npm install commands - Add JWT license token configuration section - Update all code examples with unscoped package name - Remove GitHub Packages authentication instructions - Update contributor documentation to reflect all packages are public - Update CONTRIBUTING.md files to remove private package sections Documentation files updated: - react_on_rails_pro/docs/installation.md (major rewrite) - react_on_rails_pro/docs/node-renderer/basics.md - react_on_rails_pro/docs/node-renderer/js-configuration.md - react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.md - react_on_rails_pro/docs/code-splitting-loadable-components.md - docs/contributor-info/releasing.md - react_on_rails_pro/docs/contributors-info/releasing.md - CONTRIBUTING.md - react_on_rails_pro/CONTRIBUTING.md 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fix ESLint errors from CI by updating all remaining references from @shakacode-tools/react-on-rails-pro-node-renderer to react-on-rails-pro-node-renderer. Files updated: - react_on_rails_pro/spec/dummy/client/node-renderer.js (3 requires) - react_on_rails_pro/packages/node-renderer/src/integrations/api.ts (JSDoc example) - react_on_rails_pro/spec/dummy/package.json (dependency + yalc link) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Update error message from @shakacode-tools/react-on-rails-pro-node-renderer to react-on-rails-pro-node-renderer in package-scripts.yml 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Add comprehensive migration guide in react_on_rails_pro/docs/updating.md - Shows users their current GitHub Packages setup - Provides step-by-step migration to public distribution - Includes verification and troubleshooting sections - Add explanatory comment in release.rake for node-renderer publish command - Clarifies why plain 'yarn publish' is used instead of 'yarn workspace' Addresses code review feedback from Claude AI review 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Fix bugs/homepage URLs in package.json to point to monorepo - Add prominent link to upgrading guide in installation.md - Update version examples to 16.2 for consistency with release Addresses Claude AI review feedback on PR #1901 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
cd14aa5 to
65e8d37
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
react_on_rails_pro/docs/installation.md (1)
185-187: Add a language hint to the fenced block for lint compliance.
Markdownlint flags this block (MD040). Label it as plain text to silence the warning.-``` +```text React on Rails Pro license validated successfully -``` +```react_on_rails_pro/docs/updating.md (2)
43-47: Label the.npmrcsnippet with a language.
MD040 warns when fenced blocks lack a language. Usinginikeeps markdownlint happy.-``` +```ini always-auth=true //npm.pkg.github.com/:_authToken=YOUR_TOKEN @shakacode-tools:registry=https://npm.pkg.github.com -``` +```
185-187: Add a language to the success-message fence.
Same MD040 warning as above—marking this as text avoids future lint failures.-``` +```text React on Rails Pro license validated successfully -``` +```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
react_on_rails_pro/spec/dummy/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (16)
CONTRIBUTING.md(1 hunks)docs/contributor-info/releasing.md(1 hunks)rakelib/release.rake(5 hunks)react_on_rails_pro/CONTRIBUTING.md(1 hunks)react_on_rails_pro/docs/code-splitting-loadable-components.md(1 hunks)react_on_rails_pro/docs/contributors-info/releasing.md(1 hunks)react_on_rails_pro/docs/installation.md(3 hunks)react_on_rails_pro/docs/node-renderer/basics.md(1 hunks)react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.md(7 hunks)react_on_rails_pro/docs/node-renderer/js-configuration.md(1 hunks)react_on_rails_pro/docs/updating.md(1 hunks)react_on_rails_pro/package-scripts.yml(1 hunks)react_on_rails_pro/package.json(3 hunks)react_on_rails_pro/packages/node-renderer/src/integrations/api.ts(1 hunks)react_on_rails_pro/spec/dummy/client/node-renderer.js(2 hunks)react_on_rails_pro/spec/dummy/package.json(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- react_on_rails_pro/CONTRIBUTING.md
- react_on_rails_pro/packages/node-renderer/src/integrations/api.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- react_on_rails_pro/spec/dummy/client/node-renderer.js
- CONTRIBUTING.md
- react_on_rails_pro/docs/node-renderer/basics.md
- react_on_rails_pro/package-scripts.yml
- react_on_rails_pro/docs/node-renderer/js-configuration.md
- react_on_rails_pro/package.json
- react_on_rails_pro/spec/dummy/package.json
- react_on_rails_pro/docs/code-splitting-loadable-components.md
- docs/contributor-info/releasing.md
- react_on_rails_pro/docs/contributors-info/releasing.md
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
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.
📚 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:
react_on_rails_pro/docs/updating.mdreact_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.mdreact_on_rails_pro/docs/installation.mdrakelib/release.rake
📚 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:
react_on_rails_pro/docs/updating.mdreact_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.mdreact_on_rails_pro/docs/installation.mdrakelib/release.rake
📚 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:
react_on_rails_pro/docs/updating.mdreact_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.mdreact_on_rails_pro/docs/installation.mdrakelib/release.rake
📚 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/docs/updating.mdreact_on_rails_pro/docs/installation.mdrakelib/release.rake
📚 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:
react_on_rails_pro/docs/updating.mdreact_on_rails_pro/docs/installation.mdrakelib/release.rake
📚 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/docs/updating.mdreact_on_rails_pro/docs/installation.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:
react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.md
📚 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:
react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.mdreact_on_rails_pro/docs/installation.md
📚 Learning: 2025-06-09T07:58:02.646Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1696
File: node_package/src/RSCPayloadGenerator.ts:124-168
Timestamp: 2025-06-09T07:58:02.646Z
Learning: In React Server Components (RSC) implementations, explicit error handling in RSC payload generation streams (like in RSCPayloadGenerator.ts) is not needed because errors will propagate to client components that fetch the RSC payloads (such as RSCRoute components). React's built-in error handling mechanisms and ErrorBoundary components are sufficient for handling these errors at the component level, which allows for better user experience with appropriate error UI fallbacks.
Applied to files:
react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.md
🪛 markdownlint-cli2 (0.18.1)
react_on_rails_pro/docs/updating.md
43-43: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
185-185: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
react_on_rails_pro/docs/installation.md
43-43: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
185-185: 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). (5)
- GitHub Check: lint-js-and-ruby
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: markdown-link-check
- GitHub Check: claude-review
🔇 Additional comments (2)
react_on_rails_pro/docs/node-renderer/error-reporting-and-tracing.md (1)
17-146: Import path updates look correct.
The switch to the unscopedreact-on-rails-pro-node-renderernamespace keeps all integration examples in sync with the new package distribution.rakelib/release.rake (1)
210-223: Good call highlighting the non-workspace publish path.
Keeping the plainyarn publishinvocation (with the OTP reminder) avoids workspace assumptions around the node renderer package location after moving to the unscoped public package.
|
@claude review this PR. |
|
Claude finished @justin808's task —— View job Code Review: Switch Pro packages to public distributionI've completed a comprehensive review of PR #1901. This is a well-executed strategic change that transitions React on Rails Pro packages from private GitHub Packages distribution to public npmjs.org and RubyGems.org distribution. ✅ Overall AssessmentStatus: Approved This PR achieves its goals cleanly and comprehensively. All 10 CI checks are passing, the code changes are minimal and focused, documentation has been thoroughly updated, and the security model remains intact through runtime JWT validation. 🎯 Strengths1. Clean, Surgical Code Changes
2. Package Naming Strategy
3. Comprehensive Documentation
4. Consistency Across Files
5. Sound Architecture
📋 Code Quality Analysisrakelib/release.rake (lines 210-242)✅ Excellent improvements:
Note on previous review feedback: The earlier CodeRabbit review mentioned needing react_on_rails_pro/package.json✅ Key changes handled correctly:
Documentation Updates✅ Installation.md - Excellent rewrite:
✅ Updating.md - Outstanding migration guide:
🔒 Security AnalysisRuntime Security (Unchanged - Correct)✅ JWT validation at Rails startup Distribution Security (New Model)✅ Public distribution doesn't expose secrets (packages secured by runtime license checks) Assessment: The security model is sound. Public distribution of packages that require runtime license validation is an industry-standard pattern. 🎯 Breaking Changes & MigrationFor Existing Customers:This is a breaking change requiring:
Mitigation: The new 🧪 Testing & Release ReadinessCI Status:✅ All 10 workflows passing Testing Plan (from PR description):The PR includes a comprehensive testing plan with dry-run, Verdaccio testing, and verification steps. This is appropriate for a change of this magnitude. 💡 Observations & Minor Suggestions1. Markdown Linting (Non-blocking)Two files have fenced code blocks without language identifiers:
These don't block merge but could be fixed for consistency. 2. CHANGELOG Update (Recommended)Per the project's #### [16.2.0]
##### Changed
- **BREAKING**: Pro packages now distributed via public registries (npmjs.org, RubyGems.org) instead of GitHub Packages.
Customers no longer need GitHub PAT tokens - only JWT license token required.
See [Upgrading Guide](./react_on_rails_pro/docs/updating.md) for migration steps.
[PR 1901](https://github.com/shakacode/react_on_rails/pull/1901) by [ihabadham](https://github.com/ihabadham)You can use the 3. Release Validation (Pre-merge)Before merging, consider testing the actual release flow with Verdaccio as outlined in the PR description's testing plan. 📊 Files ReviewedRelease automation: Total: 16 files thoroughly reviewed (17th file is yarn.lock - auto-generated) 🚀 Impact & BenefitsFor Customers:
For ShakaCode:
For Development:
🎓 Comparison with Industry StandardsThis approach matches the commercial model used by:
This is a proven, battle-tested pattern for commercial open-source software. ✅ Final RecommendationApprove and Merge This is a strategic improvement that:
Minor items to address (non-blocking):
Risk Assessment: Low
Great work on this strategic transition! The implementation is professional, thorough, and well-documented. 🚀 |
Summary
This PR implements the switch from GitHub Packages to public distribution for React on Rails Pro packages and gem.
Changes
1. Node Renderer NPM Package (
react_on_rails_pro/package.json)publishConfigsection to publish to npmjs.org instead of GitHub Packages2. Release Script (
rakelib/release.rake)--key github --hostarguments to publish to RubyGems.org3. Documentation Updates
Distribution Strategy
Before:
After:
gem installandnpm installBreaking Change
.npmrcconfiguration after this release. Justin will communicate migration steps directly to customers.Security
Runtime enforcement remains completely unchanged:
Testing Plan
rake release[16.2.0,true]rake release[16.2.0-test.1,false,verdaccio]🤖 Generated with Claude Code
This change is
Summary by CodeRabbit
Chores
Documentation