Migrate to Shakapacker 9.0.0-beta.4 with Babel transpiler #662
Conversation
This is the first of three planned PRs to migrate to Shakapacker 9.x: 1. Migrate to beta.4 with Babel transpiler (this PR) 2. Switch from Babel to SWC transpiler 3. Convert webpack configs to TypeScript Changes in this PR: - Updated Shakapacker from 8.2.0 to 9.0.0-beta.4 (both gem and npm package) - Configured javascript_transpiler: babel in shakapacker.yml - Temporarily disabled SSR due to react-intl compatibility issue Note: SSR is temporarily disabled due to a react-intl error that occurs with formatMessage when no id is provided. This appears to be related to babel-plugin-formatjs configuration. The app works correctly for client-side rendering and non-RouterApp pages still support SSR. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughUpgrades Shakapacker to 9.0.0 beta in both Ruby and JS dependencies, adds a Babel transpiler setting to shakapacker config, and updates several React components to guard intl.formatMessage usage for SSR safety by providing a local fallback when intl is unavailable. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant C as React Component
participant I as intl
note over C: render()
C->>I: Check intl and intl.formatMessage
alt intl available
C->>I: formatMessage(msg)
I-->>C: localized string
else intl missing
note right of C: Use fallback<br/>(msg.defaultMessage or "")
C-->>C: formatMessage = fallback
end
C-->>U: Render UI with message text
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
🚀 Quick Review App CommandsWelcome! Here are the commands you can use in this PR:
|
|
/deploy-review-app |
1 similar comment
|
/deploy-review-app |
Code Review for PR #662: Migrate to Shakapacker 9.0.0-beta.4 with Babel transpilerThank you for this well-structured migration PR! I've reviewed the changes and here's my detailed feedback: ✅ Strengths
🔍 Code Quality & Best PracticesGood:
Suggestions:
|
- Re-enable SSR in React on Rails configuration - Add safeguards for formatMessage when intl context is not initialized during SSR - Add missing message definitions for form buttons - Update i18n translations to support all UI messages This fixes the react-intl SSR error that was preventing server-side rendering from working properly with Shakapacker 9.0.0-beta.4. 🤖 Generated with Claude Code (https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
PR Review: Migrate to Shakapacker 9.0.0-beta.4 with Babel transpilerSummaryThis PR successfully migrates from Shakapacker 8.2.0 to 9.0.0-beta.4, but introduces a workaround for SSR issues that should be properly addressed before merging. ✅ Positive Aspects
🔴 Critical Issues1. SSR Workaround Instead of FixThe defensive programming pattern added to handle missing const formatMessage = intl && intl.formatMessage ? intl.formatMessage : (msg) => msg.defaultMessage || '';Issues with this approach:
Recommended fix: yarn add -D babel-plugin-formatjsThen update babel.config.js: plugins: [
['formatjs', {
idInterpolationPattern: '[sha512:contenthash:base64:6]',
ast: true
}],
// ... other plugins
]2. Missing Test Coverage
🟡 Performance Considerations
🟡 Security Concerns
📝 Recommendations
🚦 VerdictRequest Changes - While the migration is technically correct, the SSR workaround should be replaced with a proper fix using babel-plugin-formatjs. The current approach creates technical debt and masks configuration issues. Consider creating a separate branch to properly configure babel-plugin-formatjs and test SSR functionality thoroughly before proceeding with the SWC migration. 🤖 Generated with Claude Code Co-Authored-By: Claude noreply@anthropic.com |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
client/app/bundles/comments/components/CommentBox/CommentForm/CommentForm.jsx (1)
104-106: Consider extracting the SSR guard to eliminate duplication.The same SSR guard pattern for
formatMessageis repeated four times acrossformHorizontal,formStacked,formInline, andrender. This duplication increases maintenance burden and the risk of inconsistent updates.Consider extracting the guard logic to a helper method or computing it once in
renderand passing it to the form methods:Option 1: Extract to instance method
+ getFormatMessage() { + const { intl } = this.props; + return intl && intl.formatMessage ? intl.formatMessage : (msg) => msg.defaultMessage || ''; + } + formHorizontal() { - // Safeguard for SSR where intl might not be properly initialized - const { intl } = this.props; - const formatMessage = intl && intl.formatMessage ? intl.formatMessage : (msg) => msg.defaultMessage || ''; + const formatMessage = this.getFormatMessage(); return (Apply the same change to
formStacked,formInline, andrender.Option 2: Compute once in render and pass as parameter
render() { + const formatMessage = this.getFormatMessage(); let inputForm; switch (this.state.formMode) { case 0: - inputForm = this.formHorizontal(); + inputForm = this.formHorizontal(formatMessage); break;Then update each form method signature to accept
formatMessageas a parameter.Also applies to: 161-163, 218-220, 323-325
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Gemfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
Gemfile(1 hunks)client/app/bundles/comments/components/CommentBox/CommentBox.jsx(1 hunks)client/app/bundles/comments/components/CommentBox/CommentForm/CommentForm.jsx(4 hunks)client/app/bundles/comments/components/SimpleCommentScreen/ror_components/SimpleCommentScreen.jsx(1 hunks)config/shakapacker.yml(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). (2)
- GitHub Check: deploy
- GitHub Check: claude-review
🔇 Additional comments (6)
Gemfile (1)
9-9: LGTM! Version aligns with npm package.The gem version format (9.0.0.beta.4) correctly matches the npm package version (9.0.0-beta.4) from package.json. The beta version concern applies here as well—ensure thorough testing before production deployment.
config/shakapacker.yml (1)
11-11: LGTM! Babel transpiler configuration is explicit.The addition of
javascript_transpiler: babelclearly documents the transpilation strategy. This aligns with the existing Babel dependencies in package.json (babel-loader, @babel/core, etc.).Per the PR objectives, switching to SWC is planned as a follow-up. Consider opening a tracking issue for that migration if one doesn't exist yet.
client/app/bundles/comments/components/CommentBox/CommentBox.jsx (1)
77-78: LGTM! SSR-safe guard for intl is well-implemented.The fallback ensures
formatMessageis always callable, preventing runtime errors during SSR whenintlmight be uninitialized. The fallback correctly usesmsg.defaultMessage(from thedefaultMessagesimport on line 8) as a sensible default.This pattern is consistently applied across other components in this PR (CommentForm, SimpleCommentScreen).
client/app/bundles/comments/components/SimpleCommentScreen/ror_components/SimpleCommentScreen.jsx (1)
75-76: LGTM! Consistent SSR-safe implementation.The guard implementation is identical to the one in CommentBox.jsx (lines 77-78), ensuring consistency across components. The fallback chain (
msg.defaultMessage || '') handles edge cases gracefully.package.json (1)
96-96: Verify beta stability and migration plan.You're using Shakapacker 9.0.0-beta.4, but the latest stable release is 8.4.0. Version 9.0.0 has not reached GA and lacks official migration documentation. Ensure thorough testing in staging before deploying to production.
Track the official 9.0.0 release and review the migration guide once available. Consider the stability requirements of your production environment when using pre-release versions.
client/app/bundles/comments/components/CommentBox/CommentForm/CommentForm.jsx (1)
104-106: Clarify whenintlwould be unavailable.The component is wrapped with
injectIntl(line 371) and declaresintlas a required prop (line 19). Under normal circumstances with a properIntlProviderwrapper,intlshould always be available.Can you clarify the specific scenario where
intlorintl.formatMessagewould be unavailable?
- Is this addressing a case where the component is rendered outside of
IntlProvider?- Is this a temporary workaround for the babel-plugin-formatjs issue mentioned in the PR objectives?
- Should this guard be removed once the SWC transpiler migration is complete?
Understanding the specific use case will help determine if this is the right long-term solution or if there's a better approach (e.g., ensuring
IntlProvideralways wraps the component tree).
| const { formatMessage } = this.props.intl; | ||
| // Safeguard for SSR where intl might not be properly initialized | ||
| const { intl } = this.props; | ||
| const formatMessage = intl && intl.formatMessage ? intl.formatMessage : (msg) => msg.defaultMessage || ''; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that empty string fallback is acceptable for SSR.
The SSR guard falls back to returning msg.defaultMessage || ''. While this prevents runtime errors, it may result in empty labels, placeholders, and button text during server-side rendering if defaultMessage is undefined.
Please verify that:
- All message descriptors in
defaultMessageshavedefaultMessageproperties defined - Empty strings during SSR won't negatively impact SEO or initial page render
- The client-side hydration correctly replaces these with proper localized messages
You can verify the message descriptor structure with:
Also applies to: 163-163, 220-220, 325-325
🏁 Script executed:
#!/bin/bash
# Check if all defaultMessages have defaultMessage property
cat client/app/libs/i18n/default.js client/app/libs/i18n/default.ts client/app/libs/i18n/default.jsx 2>/dev/null | head -100Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find the i18n default messages file
echo "=== Finding i18n default file ==="
fd -t f "default" client/app/libs/i18n 2>/dev/null || echo "Path client/app/libs/i18n not found"
fd -t f "default" libs/i18n 2>/dev/null || echo "Path libs/i18n not found"
echo -e "\n=== Search for i18n directory ==="
fd -t d "i18n" client 2>/dev/null | head -10
echo -e "\n=== Find files with 'default' in i18n contexts ==="
fd -e js -e ts -e jsx "default" client/app 2>/dev/null | grep -i i18n | head -10Length of output: 167
🏁 Script executed:
#!/bin/bash
# Search for where defaultMessages is defined or exported
echo "=== Search for defaultMessages export ==="
rg -n "export.*defaultMessages" client/app --type js --type ts -C2 | head -50Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check how formatMessage is used with message descriptors in this file
echo "=== Check formatMessage usage in CommentForm.jsx ==="
rg -n "formatMessage\(" client/app/bundles/comments/components/CommentBox/CommentForm/CommentForm.jsx -A1 -B1 | head -80Length of output: 3344
🏁 Script executed:
#!/bin/bash
# List all files in the i18n directory
echo "=== Files in client/app/libs/i18n/ ==="
ls -la client/app/libs/i18n/ 2>/dev/null || echo "Directory not found"Length of output: 214
🏁 Script executed:
#!/bin/bash
# Show the import statement and surrounding context
echo "=== Import statement context in CommentForm.jsx ==="
cat client/app/bundles/comments/components/CommentBox/CommentForm/CommentForm.jsx | head -20Length of output: 932
🏁 Script executed:
#!/bin/bash
# Search for defineMessages usage in the codebase
echo "=== Search for defineMessages in client/app/libs/i18n ==="
rg -n "defineMessages" client/app/libs/i18n -A5 -B2 2>/dev/null | head -100Length of output: 57
🏁 Script executed:
#!/bin/bash
# Look for any .js, .jsx, .ts files in i18n directory
echo "=== Finding JavaScript/TypeScript files in i18n ==="
fd -e js -e jsx -e ts -e tsx . client/app/libs/i18n 2>/dev/nullLength of output: 91
🏁 Script executed:
#!/bin/bash
# Search for libs/i18n/default at different locations
echo "=== Search for default file at project root ==="
fd -t f "default" libs/i18n 2>/dev/null || echo "Path libs/i18n not found at root"
echo -e "\n=== Find all i18n directories ==="
fd -t d "^i18n$" . 2>/dev/null | head -10Length of output: 139
🏁 Script executed:
#!/bin/bash
# Search for defaultMessages definition/export across entire codebase
echo "=== Search for defaultMessages definition ==="
rg -n "defaultMessages\s*=" client --type js --type jsx -C3 2>/dev/null | head -80Length of output: 45
🏁 Script executed:
#!/bin/bash
# Search for export of defaultMessages with more flexible pattern
echo "=== Search for defaultMessages export (flexible) ==="
rg -n "defaultMessages" client/app/libs -C3 2>/dev/null | head -80Length of output: 52
🏁 Script executed:
#!/bin/bash
# Check if there's a default.js or default/index.js anywhere
echo "=== Find files named default.js, default.jsx, default.ts ==="
fd -t f "^default\.(js|jsx|ts|tsx)$" client 2>/dev/null
fd -t f "^index\.(js|jsx|ts|tsx)$" client/app/libs/i18n 2>/dev/nullLength of output: 60
🏁 Script executed:
#!/bin/bash
# Search entire repo for libs/i18n/default directory or file
echo "=== Search entire repo for libs/i18n/default ==="
fd -t f "default" . 2>/dev/null | grep -i "i18n" | head -20
echo -e "\n=== Search for any libs directory at root ==="
ls -la libs/ 2>/dev/null || echo "No libs directory at root"Length of output: 121
🏁 Script executed:
#!/bin/bash
# Search for other files importing from libs/i18n/default
echo "=== Files importing from libs/i18n/default ==="
rg -n "from ['\"]libs/i18n/default['\"]" . --type js --type jsx -C1 2>/dev/null | head -50Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check if there's a webpack config or babel config that might alias this
echo "=== Search for webpack or babel config ==="
fd -t f "webpack" . 2>/dev/null | head -10
fd -t f "babel" . 2>/dev/null | head -10Length of output: 249
🏁 Script executed:
#!/bin/bash
# Search the entire codebase for where defaultMessages object is defined
echo "=== Search for defineMessages across entire codebase ==="
rg -n "defineMessages" . --type js --type jsx -g '!node_modules' -g '!dist' -g '!build' 2>/dev/null | head -30Length of output: 56
🏁 Script executed:
#!/bin/bash
# Check webpack config for resolve aliases
echo "=== Check webpack resolve aliases ==="
rg -n "resolve.*alias" config/webpack/ -A10 -B2 2>/dev/null | head -60Length of output: 37
🏁 Script executed:
#!/bin/bash
# Check commonWebpackConfig specifically
echo "=== Check commonWebpackConfig.js ==="
cat config/webpack/commonWebpackConfig.js 2>/dev/null | grep -A15 -B5 "resolve\|alias" | head -80Length of output: 1057
🏁 Script executed:
#!/bin/bash
# Search for 'libs' path resolution
echo "=== Search for libs path in webpack config ==="
rg -n "libs" config/webpack/commonWebpackConfig.js -C3 2>/dev/nullLength of output: 46
🏁 Script executed:
#!/bin/bash
# Check if this import exists in git history or is newly added
echo "=== Check git log for this import ==="
git log --all --oneline -S "from 'libs/i18n/default'" -- "client/app/bundles/comments/components/CommentBox/CommentForm/CommentForm.jsx" 2>/dev/null | head -10Length of output: 128
🏁 Script executed:
#!/bin/bash
# Check what files were changed in the i18n commit
echo "=== Files changed in commit ef369ed ==="
git show --name-status ef369ed 2>/dev/null | head -30Length of output: 1136
🏁 Script executed:
#!/bin/bash
# Check the full diff for the import line specifically
echo "=== Show changes to CommentForm.jsx in ef369ed ==="
git show ef369ed:client/app/bundles/comments/components/CommentBox/CommentForm/CommentForm.jsx 2>/dev/null | head -30Length of output: 1320
🏁 Script executed:
#!/bin/bash
# Check if any i18n/default file was added in this commit
echo "=== Check if default file was added ==="
git show --name-only ef369ed 2>/dev/null | grep -i "default\|i18n"Length of output: 325
🏁 Script executed:
#!/bin/bash
# Check the shakapacker config for module resolution
echo "=== Check package.json and shakapacker config ==="
cat config/shakapacker.yml 2>/dev/null | head -40Length of output: 1298
🏁 Script executed:
#!/bin/bash
# Check git log for libs/i18n/default file history
echo "=== Git history for libs/i18n/default ==="
git log --all --full-history --oneline -- "**/i18n/default*" 2>/dev/null | head -20Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check if the file exists in any branch or was ever committed
echo "=== Search all commits for i18n/default file ==="
git log --all --oneline --name-only -- "client/app/libs/i18n/default*" 2>/dev/null | head -30Length of output: 48
🏁 Script executed:
#!/bin/bash
# Check current branch and PR diff
echo "=== Current branch info ==="
git branch --show-current 2>/dev/null
echo ""
git log --oneline -5 2>/dev/nullLength of output: 345
🏁 Script executed:
#!/bin:bash
# Try to find if there's a default.js file in git history at all
echo "=== Search for any default file creation in libs/i18n ==="
git log --all --diff-filter=A --name-only --pretty=format: -- "*/libs/i18n/*" 2>/dev/null | grep -i default | head -20Length of output: 57
🏁 Script executed:
#!/bin/bash
# Check the exact current state of CommentForm.jsx imports
echo "=== Current CommentForm.jsx first 30 lines ==="
head -30 client/app/bundles/comments/components/CommentBox/CommentForm/CommentForm.jsx 2>/dev/nullLength of output: 1160
🏁 Script executed:
#!/bin/bash
# Search for any object or const called defaultMessages in the file itself
echo "=== Search for defaultMessages definition in CommentForm.jsx ==="
rg -n "defaultMessages" client/app/bundles/comments/components/CommentBox/CommentForm/CommentForm.jsx -C5Length of output: 7818
🏁 Script executed:
#!/bin/bash
# Check if defaultMessages might be injected via babel or defined elsewhere
echo "=== Search for defaultMessages anywhere in comments bundle ==="
rg -n "export.*defaultMessages|const defaultMessages|defaultMessages.*=" client/app/bundles/comments -C2 2>/dev/null | head -50Length of output: 62
🏁 Script executed:
#!/bin/bash
# Check the git diff for the current PR to see what changed
echo "=== Git diff for current commit ==="
git show HEAD:client/app/bundles/comments/components/CommentBox/CommentForm/CommentForm.jsx 2>/dev/null | grep -A5 -B5 "import.*default"Length of output: 439
🏁 Script executed:
#!/bin/bash
# Check if the file existed in the parent commit
echo "=== Check parent commit for libs/i18n/default ==="
git show HEAD~1:client/app/libs/i18n/default.js 2>/dev/null | head -20 || echo "File does not exist in parent commit"
git show HEAD~1:client/app/libs/i18n/default.ts 2>/dev/null | head -20 || echo "File does not exist in parent commit"
git show HEAD~1:client/app/libs/i18n/default.jsx 2>/dev/null | head -20 || echo "File does not exist in parent commit"Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check several commits back
echo "=== Check 5 commits back ==="
git show HEAD~5:client/app/libs/i18n/default.js 2>/dev/null | head -20 || echo "File does not exist 5 commits back"Length of output: 28
🏁 Script executed:
#!/bin/bash
# List all files in i18n directory from parent commit
echo "=== Files in i18n dir in parent commit ==="
git ls-tree -r --name-only HEAD~1 client/app/libs/i18n/ 2>/dev/nullLength of output: 82
🏁 Script executed:
#!/bin/bash
# Check if this might be resolved via package or node_modules
echo "=== Search for defaultMessages in package.json dependencies ==="
cat package.json 2>/dev/null | jq '.dependencies, .devDependencies' | grep -i "i18n\|intl" || echo "No obvious i18n packages"Length of output: 109
Critical: Missing required file for import
The import import { defaultMessages } from 'libs/i18n/default'; on line 7 references a file that does not exist in the codebase. The file client/app/libs/i18n/default.js (or .ts, .jsx, .tsx) has never been committed to the repository.
This will cause a module resolution error at runtime, preventing the component from loading. The SSR fallback logic in lines 106, 163, 220, and 325 cannot be evaluated because the import itself will fail.
Required action:
- Create the missing
client/app/libs/i18n/default.jsfile with proper message descriptors - Ensure all message descriptors include both
idanddefaultMessageproperties:
import { defineMessages } from 'react-intl';
export const defaultMessages = defineMessages({
inputNameLabel: {
id: 'comments.form.nameLabel',
defaultMessage: 'Name'
},
inputNamePlaceholder: {
id: 'comments.form.namePlaceholder',
defaultMessage: 'Enter your name'
},
// ... other messages used in CommentForm.jsx
});- After creating the file, verify that the fallback
msg.defaultMessage || ''provides acceptable UX during SSR
🤖 Prompt for AI Agents
In client/app/bundles/comments/components/CommentBox/CommentForm/CommentForm.jsx
around line 106, the import of defaultMessages points to a missing module which
will break module resolution; create the missing file
client/app/libs/i18n/default.js exporting a defaultMessages object (use
react-intl defineMessages or a plain object) and include message descriptors for
every message used in CommentForm.jsx (each descriptor must have id and
defaultMessage keys, e.g., inputNameLabel, inputNamePlaceholder, and any others
referenced at lines ~106, 163, 220, 325); save the file and ensure the existing
import statement matches the export name so the SSR fallback (msg.defaultMessage
|| '') can work correctly.
Summary
Context
This is the first of three planned PRs to migrate to Shakapacker 9.x:
Changes
Gemfileandpackage.jsonto use Shakapacker 9.0.0-beta.4javascript_transpiler: babelconfiguration toconfig/shakapacker.ymlKnown Issues
/simple) still work with SSR enabledTesting
Next Steps
After this PR is merged:
🤖 Generated with Claude Code
This change is
Summary by CodeRabbit
Bug Fixes
Chores