fix: dependencies version and SSR lottie rendering #983
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@PedroRosalba is attempting to deploy a commit to the LFG Labs Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe pull request involves modifications across multiple UI components and the package configuration. The primary changes include dynamically importing the Changes
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (7)
components/UI/screens/successScreen.tsx (1)
4-8: LGTM! Dynamic import implementation looks correct.The dynamic import of Lottie with
ssr: falsewill prevent hydration mismatches by ensuring the animation only renders client-side.Consider adding type definitions for Lottie props to improve type safety:
import type { LottieComponentProps } from 'lottie-react';components/UI/isSendingTx.tsx (1)
Line range hint
16-16: Consider using className for consistent styling.For consistency with other components, consider using className instead of inline style for width.
- <Lottie style={{ width: "500px" }} animationData={sendingLottie} loop /> + <Lottie className="w-[500px]" animationData={sendingLottie} loop />components/UI/txConfirmationModal.tsx (1)
Line range hint
35-44: Add security attributes to external link.When opening external links, it's recommended to add security-related attributes.
window.open( `https://${ process.env.NEXT_PUBLIC_IS_TESTNET === "true" ? "sepolia." : "" - }starkscan.co/tx/${txHash}` + }starkscan.co/tx/${txHash}`, + '_blank', + 'noopener,noreferrer' )components/UI/modalMessage.tsx (1)
Line range hint
12-12: Improve type definition for lottie prop.The current
unknowntype for the lottie prop could be more specific.- lottie?: unknown; // Lottie doesn't have a special type + lottie?: Record<string, unknown>; // Type for JSON animation datacomponents/UI/confirmationTx.tsx (1)
5-7: Consider handling loading state.Since dynamic imports can introduce a brief loading delay, consider adding a loading placeholder or skeleton UI while the Lottie component is being loaded.
import dynamic from "next/dynamic"; -const Lottie = dynamic(() => import("lottie-react"), { ssr: false }); +const Lottie = dynamic(() => import("lottie-react"), { + ssr: false, + loading: () => <div className="w-48 h-48 animate-pulse bg-gray-200 rounded-full" /> +});components/UI/registerConfirmationModal.tsx (1)
Line range hint
33-43: Extract duplicated explorer URL logic.The explorer URL construction logic is duplicated across components. Consider extracting it into a utility function for better maintainability.
// utils/explorer.ts export const getExplorerUrl = (txHash: string): string => { const network = process.env.NEXT_PUBLIC_IS_TESTNET === "true" ? "sepolia." : ""; return `https://${network}starkscan.co/tx/${txHash}`; };Then update the usage:
-window.open( - `https://${ - process.env.NEXT_PUBLIC_IS_TESTNET === "true" - ? "sepolia." - : "" - }starkscan.co/tx/${txHash}` -) +window.open(getExplorerUrl(txHash))package.json (1)
Line range hint
29-29: Consider pinning the lottie-react version.Given that this PR addresses SSR-related issues with Lottie animations, consider pinning the
lottie-reactversion to prevent potential future SSR issues from minor version updates. Replace^2.4.0with2.4.0.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
components/UI/confirmationTx.tsx(1 hunks)components/UI/isSendingTx.tsx(1 hunks)components/UI/modalMessage.tsx(1 hunks)components/UI/registerConfirmationModal.tsx(1 hunks)components/UI/screens/successScreen.tsx(1 hunks)components/UI/txConfirmationModal.tsx(1 hunks)package.json(1 hunks)
🔇 Additional comments (7)
components/UI/isSendingTx.tsx (1)
6-8: LGTM! Dynamic import implementation is consistent.The dynamic import matches the pattern used across other components.
components/UI/txConfirmationModal.tsx (1)
5-8: LGTM! Dynamic import implementation is consistent.The dynamic import matches the pattern used across other components.
components/UI/modalMessage.tsx (2)
5-8: LGTM! Dynamic import implementation is consistent.The dynamic import matches the pattern used across other components.
Line range hint
1-1: Verify dependency version changes.The PR objectives mention fixing dependency versions, but the changes are not visible in the provided files. Please ensure that the dependency version updates are included in the PR.
✅ Verification successful
Dependency version changes confirmed
The PR includes the dependency updates as mentioned in the objectives, specifically updating
get-starknet-corefrom ^3.3.4 to ^4.0.0.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for package.json changes git diff origin/main...HEAD -- package.jsonLength of output: 86
Script:
#!/bin/bash # First, check if package.json exists and its location fd package.json # Then, check its contents cat $(fd package.json) 2>/dev/null || echo "No package.json found" # Try different git diff syntax git diff HEAD~1 -- $(fd package.json) 2>/dev/null || echo "No git diff available"Length of output: 2820
components/UI/confirmationTx.tsx (1)
5-7: LGTM! Correct implementation of dynamic import.The dynamic import of Lottie with SSR disabled is the right approach to fix the SSR rendering issues.
components/UI/registerConfirmationModal.tsx (1)
5-7: LGTM! Consistent implementation across components.The dynamic import implementation matches the pattern used in other components, maintaining consistency in the codebase.
package.json (1)
Line range hint
13-15: Verify compatibility between Starknet packages.The upgrade of
get-starknet-coreto version 4 requires verification of compatibility with other Starknet-related packages:
starknet(^6.11.0)starknetid.js(^4.0.1)starknetkit(^2.6.2)@starknet-react/core(^3.6.2)Let's check the peer dependencies and version requirements:
Also applies to: 34-36
✅ Verification successful
All Starknet packages are compatible
The upgrade to get-starknet-core v4 is compatible with all related packages:
- starknetkit@2.6.2 already uses get-starknet-core ^4.0.4
- @starknet-react/core@3.6.2 requires get-starknet-core ^4.0.0
- starknet@6.11.0 satisfies all peer dependency requirements
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Fetch package.json files from npm to check peer dependencies echo "Checking peer dependencies for @starknet-io/get-starknet-core@4.0.4:" curl -s https://registry.npmjs.org/@starknet-io/get-starknet-core/4.0.4 | jq '.peerDependencies' echo -e "\nChecking peer dependencies for @starknet-react/core@3.6.2:" curl -s https://registry.npmjs.org/@starknet-react/core/3.6.2 | jq '.peerDependencies'Length of output: 549
Script:
#!/bin/bash # Check starknetkit dependencies echo "Checking dependencies for starknetkit@2.6.2:" curl -s https://registry.npmjs.org/starknetkit/2.6.2 | jq '.dependencies, .peerDependencies'Length of output: 637
| @@ -33,7 +33,7 @@ | |||
| "dotenv": "^16.3.1", | |||
| "eslint-config-next": "^13.4.7", | |||
| "ethers": "^6.12.1", | |||
| "get-starknet-core": "^3.3.4", | |||
| "get-starknet-core": "^4.0.0", | |||
There was a problem hiding this comment.
💡 Codebase verification
Migrate to @starknet-io/get-starknet-core and remove the unscoped package
The codebase currently uses both packages:
@starknet-io/get-starknet-core(4.0.4) - The official scoped packageget-starknet-core(^4.0.0) - The legacy unscoped package
To avoid potential conflicts and reduce bundle size:
- Update
hooks/isDeployed.tsxto use@starknet-io/get-starknet-core - Remove the unscoped
get-starknet-corefrom package.json - Run package manager to update dependencies
🔗 Analysis chain
Remove duplicate Starknet core dependency.
There are two similar packages in the dependencies:
get-starknet-coreat version^4.0.0@starknet-io/get-starknet-coreat version4.0.4
This duplication could lead to increased bundle size and potential version conflicts.
Let's verify which package is actually being used in the codebase:
Consider removing the duplicate and standardizing on @starknet-io/get-starknet-core as it appears to be the official scoped package.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for imports of both packages to determine which one is actively used
echo "Searching for get-starknet-core imports:"
rg "from ['\"]get-starknet-core['\"]" -A 2
echo -e "\nSearching for @starknet-io/get-starknet-core imports:"
rg "from ['\"]@starknet-io/get-starknet-core['\"]" -A 2
Length of output: 696
Script:
#!/bin/bash
# Check the content of both files to understand the usage context
echo "Content of hooks/isDeployed.tsx:"
cat hooks/isDeployed.tsx
echo -e "\nContent of utils/connectorWrapper.ts:"
cat utils/connectorWrapper.ts
# Search for any other potential imports or uses
echo -e "\nAny other references to these packages:"
rg -i "starknet.*core" --type ts --type tsx
Length of output: 45132
Script:
#!/bin/bash
# Check package.json for any other related dependencies
echo "Checking package.json for related dependencies:"
cat package.json
# Search for any other imports or uses of these packages
echo -e "\nSearching for any other imports or uses:"
rg "from ['\"](@starknet-io/|)get-starknet-core['\"]" --type ts --type jsx --type tsx
Length of output: 2668
Script:
#!/bin/bash
# Search with a more flexible pattern and include file content
echo "Searching for get-starknet-core imports and usage:"
rg -B 2 -A 2 "get-starknet-core" --type ts --type tsx
# Check if there are any other files importing from these packages
echo -e "\nSearching for any Starknet-related imports:"
rg "from ['\"].*starknet.*['\"]" --type ts --type tsx
Length of output: 383
Script:
#!/bin/bash
# Search without file type restrictions
echo "Searching for get-starknet-core imports and usage:"
rg -B 3 -A 3 "get-starknet-core"
# Check for any file extensions first
echo -e "\nListing relevant files:"
find . -type f -name "*.ts" -o -name "*.tsx" | grep -v "node_modules"
Length of output: 16491
Fixed the dependencies version and the Lottie calls that were being made w SSR which were preventing the app to run properly
Summary by CodeRabbit
Performance Optimization
Lottiecomponent imports to use dynamic loading with server-side rendering disabled across multiple UI componentsDependency Update
get-starknet-corepackage to version 4.0.0