Skip to content

Conversation

@justin808
Copy link
Member

@justin808 justin808 commented Sep 21, 2025

This change is Reviewable

Summary by CodeRabbit

  • Refactor

    • Removed a deprecated UI component from both client and server bundles. The associated view is no longer available. Other pages and features remain unaffected, with a slightly smaller bundle size.
  • Chores

    • Updated a JSON parsing dependency to a newer minor version for improved compatibility and maintenance. No behavior changes expected for end-users.

@coderabbitai
Copy link

coderabbitai bot commented Sep 21, 2025

Walkthrough

RescriptShow was removed from client and server ReactOnRails registrations by commenting out imports and omitting it from register calls. Other registrations remain intact. Additionally, package.json updates the dependency @glennsl/rescript-json-combinators from ^1.2.1 to ^1.4.0.

Changes

Cohort / File(s) Summary of Changes
ReactOnRails registration cleanup
client/app/packs/client-bundle.js, client/app/packs/server-bundle.js
Commented out RescriptShow imports and removed RescriptShow from ReactOnRails.register payloads. No other components altered.
Dependency update
package.json
Bumped dependency @glennsl/rescript-json-combinators from ^1.2.1 to ^1.4.0.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant RailsApp
  participant ReactOnRails
  participant RescriptShow as (Removed) RescriptShow

  Note over ReactOnRails: Registry updated (RescriptShow not registered)
  User->>RailsApp: Request page
  RailsApp->>ReactOnRails: Resolve component "RescriptShow"
  ReactOnRails-->>RailsApp: Component not found
  RailsApp-->>User: Render without RescriptShow (or fallback)
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hop through bundles, light and slow,
Snipping a show that’s set to go.
Registry trimmed, the garden clean,
Dependencies fresh, a brighter green.
Thump-thump! I merge with tidy cheer—
Less carrot code, more crisp this year. 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "update to 16 rc2" is short and indicates a version bump that aligns with the PR's stated objective (branch name and description referencing a v16 RC update), so it is related to the changeset. It is concise and useful for a quick scan, but slightly ambiguous because it does not name which package or component is being updated (e.g., ReactOnRails). Overall, it communicates the primary intent but could be clearer.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update-react-on-rails-16.0.1-rc.0-try-2

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b57a728 and 855f99f.

⛔ Files ignored due to path filters (2)
  • Gemfile.lock is excluded by !**/*.lock
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (3)
  • client/app/packs/client-bundle.js (2 hunks)
  • client/app/packs/server-bundle.js (1 hunks)
  • package.json (1 hunks)
⏰ 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). (1)
  • GitHub Check: test (22.x, 3.3.4)
🔇 Additional comments (3)
package.json (1)

41-41: yarn.lock already pins @glennsl/rescript-json-combinators → 1.4.0 — verify CI and code usage

yarn.lock contains "@glennsl/rescript-json-combinators@^1.4.0" resolved to version 1.4.0; ripgrep did not return any in-repo matches ("No files were searched"). Ensure a full repo search for usages and that CI runs yarn install and a clean ReScript compile.

client/app/packs/server-bundle.js (1)

11-11: Remove commented-out RescriptShow import/registration and verify no remaining references.
Delete the commented import and the commented Footer entry in client/app/packs/server-bundle.js (≈ lines 11 and 19). Confirm there are no Rails views or JS referencing "RescriptShow" before removal:

rg -nP "react_component\\s*\\(\\s*['\"]RescriptShow['\"]" -S --hidden -uu --no-ignore-vcs || true
rg -n "\bRescriptShow\b" -S --hidden -uu --no-ignore-vcs || true
fd -HI "ReScriptShow.bs.js" || true
client/app/packs/client-bundle.js (1)

13-13: Remove commented RescriptShow import/registration from client bundle (mirror server)

  • Remove the commented import + commented registration in client/app/packs/client-bundle.js and mirror the same cleanup in client/app/packs/server-bundle.js.
    Apply cleanup:
-// import RescriptShow from '../bundles/comments/rescript/ReScriptShow.bs.js';
@@
   Footer,
-  // RescriptShow,
  • Confirm: app/views/pages/rescript.html.erb still calls react_component "RescriptShow" — ensure server-side prerendering/registration is provided elsewhere before removing server-side comments.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@justin808 justin808 force-pushed the update-react-on-rails-16.0.1-rc.0-try-2 branch from cdbfe8d to aa7a7b6 Compare September 21, 2025 02:35
justin808 and others added 2 commits September 20, 2025 16:38
This reverts the upgrade to React on Rails 16.0.1.rc.2 due to breaking changes:

1. Server bundle path resolution regression - React on Rails 16 looks for bundles in
   /public/webpack/test/ but Shakapacker 8.0.0 outputs to /public/packs/
2. ReScript compatibility issues - rescript-react-on-rails only supports React on Rails v10

The ReScript components have been temporarily disabled by commenting out imports
and registrations in both client-bundle.js and server-bundle.js.

This establishes a stable baseline for systematic upgrades:
- Next: Upgrade Shakapacker to 8.4 (separate PR)
- Then: Upgrade React on Rails to 16.x with proper compatibility

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@justin808 justin808 closed this Sep 21, 2025
@justin808 justin808 deleted the update-react-on-rails-16.0.1-rc.0-try-2 branch September 21, 2025 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants