Upgrade Medusa to 2.13.1 and stabilize workspace#77
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
WarpBuilds runners (warp-ubuntu-2404-x64-2x) are not configured on this repo, causing all jobs to queue indefinitely. Switch to standard GitHub-hosted ubuntu-latest runners and replace WarpBuilds-specific actions (WarpBuilds/cache, WarpBuilds/setup-node) with their official equivalents (actions/cache@v4, actions/setup-node@v4). Also fix: remove dangling `needs.detect-changes.outputs.e2e` condition on the typecheck step (detect-changes job doesn't exist in this workflow) which was silently skipping typecheck on every run.
WalkthroughThe PR updates the project's build infrastructure from WarpBuilds to GitHub Actions, upgrades Yarn from 4.5.0 to 4.9.1, and modernizes the Medusa framework ecosystem from 2.7.0 to 2.13.1 across all package.json files. It removes Mikro-ORM dependencies and adds type safety guards to storefront components. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/storefront/app/hooks/useSiteDetails.ts`:
- Line 7: Replace the unsafe empty-object cast in useSiteDetails by returning
data?.siteDetails directly: remove the fallback "({} as SiteDetailsRootData)" in
the return of the useSiteDetails hook so the function returns data?.siteDetails
(possibly undefined) instead of a lie-typed empty object; update the return
statement in the useSiteDetails function to reference data?.siteDetails and let
callers handle the optional value.
In `@package.json`:
- Around line 82-83: The root package.json currently forces "react" and
"react-dom" to ^19.2.3 via the top-level resolutions which overrides workspace
packages (including apps/medusa which declares "react": "18.3.1"); update the
root package.json resolutions to either remove the "react" and "react-dom"
entries or change them to match apps/medusa's declared version (18.3.1) or, if
you intend to move medusa to React 19, update apps/medusa/package.json to
"react" and "react-dom": "^19.2.3" and run yarn install to regenerate yarn.lock
so the installed versions match the declarations and avoid runtime mismatch.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
.yarn/releases/yarn-4.5.0.cjsis excluded by!**/.yarn/**.yarn/releases/yarn-4.9.1.cjsis excluded by!**/.yarn/**yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
.github/workflows/pr-checks.yaml.yarnrc.ymlapps/medusa/package.jsonapps/storefront/app/components/layout/footer/Footer.tsxapps/storefront/app/hooks/useSiteDetails.tsapps/storefront/libs/util/products.tsapps/storefront/libs/util/server/data/cart.server.tsapps/storefront/libs/util/server/data/product-reviews.server.tsapps/storefront/libs/util/server/page.server.tsapps/storefront/package.jsonpackage.json
| const data = useRootLoaderData(); | ||
|
|
||
| return data.siteDetails || {}; | ||
| return data?.siteDetails ?? ({} as SiteDetailsRootData); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, locate and examine the useSiteDetails hook definition
find . -name "useSiteDetails.ts" -o -name "useSiteDetails.tsx"Repository: lambda-curry/medusa2-starter
Length of output: 117
🏁 Script executed:
#!/bin/bash
# Search for SiteDetailsRootData type definition
rg -nP "type SiteDetailsRootData|interface SiteDetailsRootData" --type=ts --type=tsx -A 10Repository: lambda-curry/medusa2-starter
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Find all useSiteDetails() calls to understand consumer patterns
rg -nP "useSiteDetails\s*\(" --type=ts --type=tsx -B 2 -A 3Repository: lambda-curry/medusa2-starter
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Check what properties are accessed from useSiteDetails result
rg -nP "useSiteDetails\(\)[?.]|const\s+\w+\s*=\s*useSiteDetails\(\)" --type=ts --type=tsx -A 5Repository: lambda-curry/medusa2-starter
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Read the useSiteDetails.ts file to understand the hook and its return type
cat -n ./apps/storefront/app/hooks/useSiteDetails.tsRepository: lambda-curry/medusa2-starter
Length of output: 378
🏁 Script executed:
#!/bin/bash
# Find SiteDetailsRootData type definition (ts files only)
rg -nP "type SiteDetailsRootData|interface SiteDetailsRootData" --type=ts -A 10Repository: lambda-curry/medusa2-starter
Length of output: 711
🏁 Script executed:
#!/bin/bash
# Find all useSiteDetails calls using simple pattern (without tsx file type)
rg "useSiteDetails" --type=ts -B 2 -A 3Repository: lambda-curry/medusa2-starter
Length of output: 5497
🏁 Script executed:
#!/bin/bash
# Find property accesses on useSiteDetails result
rg "useSiteDetails\(\)" --type=ts -A 5Repository: lambda-curry/medusa2-starter
Length of output: 2372
🏁 Script executed:
#!/bin/bash
# Find useRootLoaderData hook definition
cat -n ./apps/storefront/app/hooks/useRootLoaderData.tsRepository: lambda-curry/medusa2-starter
Length of output: 152
🏁 Script executed:
#!/bin/bash
# Search for root loader or where data.siteDetails is set
rg "siteDetails" --type=ts -B 2 -A 2Repository: lambda-curry/medusa2-starter
Length of output: 2275
Remove unsafe empty-object cast and return data?.siteDetails directly.
The cast ({} as SiteDetailsRootData) suppresses type checks. An empty object doesn't provide the required fields (store, settings, headerNavigationItems, footerNavigationItems), yet three of four consumers destructure these fields without null guards:
HeaderSideNav.tsx: destructuresheaderNavigationItemsdirectlyHeader.tsx: destructuresheaderNavigationItemsdirectlyFooter.tsx: destructuresfooterNavigationItemsandsettingsdirectlyLogoStoreName.tsx: includes defensiveif (!store || !settings) return null;
If the fallback is triggered, these consumers will attempt to use undefined values, causing runtime crashes. The root loader already guarantees siteDetails is populated, so the fallback is unnecessary. Return data?.siteDetails to fail fast if absent and let callers handle optional chaining where needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/storefront/app/hooks/useSiteDetails.ts` at line 7, Replace the unsafe
empty-object cast in useSiteDetails by returning data?.siteDetails directly:
remove the fallback "({} as SiteDetailsRootData)" in the return of the
useSiteDetails hook so the function returns data?.siteDetails (possibly
undefined) instead of a lie-typed empty object; update the return statement in
the useSiteDetails function to reference data?.siteDetails and let callers
handle the optional value.
| "react": "^19.2.3", | ||
| "react-dom": "^19.2.3" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Declared React versions in manifests ==="
for file in package.json apps/medusa/package.json apps/storefront/package.json; do
if [ -f "$file" ]; then
name=$(jq -r '.name' "$file" 2>/dev/null || echo "unknown")
react=$(jq -r '.dependencies.react // "n/a"' "$file" 2>/dev/null || echo "error")
react_dom=$(jq -r '.dependencies["react-dom"] // "n/a"' "$file" 2>/dev/null || echo "error")
echo "$file ($name): react=$react, react-dom=$react_dom"
fi
done
echo
echo "=== Root resolutions for React ==="
if [ -f package.json ]; then
jq -r '.resolutions | keys[] as $k | "\($k)=\(.[$k])"' package.json 2>/dev/null | grep -E "react"
fi
echo
echo "=== Resolved React entries in yarn.lock (first 20 matches) ==="
if [ -f yarn.lock ]; then
rg -n '^"(react|react-dom)@' yarn.lock -A 1 | head -40
fiRepository: lambda-curry/medusa2-starter
Length of output: 543
Root React resolutions override workspace package declarations, forcing React 19.2.3+ on apps/medusa which explicitly requires React 18.3.1.
The root-level resolutions field forces React and react-dom to ^19.2.3 globally, overriding the declared 18.3.1 versions in apps/medusa/package.json. The yarn.lock confirms only React 19.2.3+ is installed; the declared React 18.3.1 is never resolved. This creates a major version incompatibility between declared intent and actual runtime, risking hidden compatibility issues and unexpected behavior.
Either align resolutions with workspace requirements, remove the global override, or upgrade apps/medusa to React 19 if compatible.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 82 - 83, The root package.json currently forces
"react" and "react-dom" to ^19.2.3 via the top-level resolutions which overrides
workspace packages (including apps/medusa which declares "react": "18.3.1");
update the root package.json resolutions to either remove the "react" and
"react-dom" entries or change them to match apps/medusa's declared version
(18.3.1) or, if you intend to move medusa to React 19, update
apps/medusa/package.json to "react" and "react-dom": "^19.2.3" and run yarn
install to regenerate yarn.lock so the installed versions match the declarations
and avoid runtime mismatch.
Summary
Verification
Notes
Summary by CodeRabbit
Bug Fixes
Chores