Skip to content
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

fix landing build and errors #2668

Merged
merged 6 commits into from
Jan 26, 2025
Merged

fix landing build and errors #2668

merged 6 commits into from
Jan 26, 2025

Conversation

aymericdelab
Copy link
Collaborator

@aymericdelab aymericdelab commented Jan 25, 2025

Summary by CodeRabbit

Release Notes

  • Refactoring

    • Updated GraphQL schema references from s0 to s1 across multiple components and hooks
    • Streamlined type handling and data structure management
    • Removed unused Storybook story files
    • Updated TypeScript configuration for improved type safety and module resolution
  • Component Updates

    • Simplified resource and token handling in various components
    • Enhanced error handling and null safety in data processing
    • Removed deprecated or unused props and functions
    • Updated SVG import and processing configuration
  • Performance

    • Optimized type assertions and data access patterns
    • Removed unnecessary code and commented-out sections
  • Maintenance

    • Cleaned up import statements
    • Removed deprecated utility functions
    • Updated routing and configuration files

Copy link

vercel bot commented Jan 25, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
eternum ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 26, 2025 8:15am
eternum-docs ❌ Failed (Inspect) Jan 26, 2025 8:15am
eternum-landing ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 26, 2025 8:15am

Copy link
Contributor

mentatbot bot commented Jan 25, 2025

Hi @aymericdelab! You need to be added as a user to interact with me. Please ask @ponderingdemocritus to add you on the settings page.

You are receiving this comment because I am set to review all PRs. That is configurable here.

Copy link
Contributor

coderabbitai bot commented Jan 25, 2025

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 eslint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

client/apps/landing/src/hooks/query/eternum-statistics.tsx

Oops! Something went wrong! :(

ESLint: 9.18.0

ESLint couldn't find an eslint.config.(js|mjs|cjs) file.

From ESLint v9.0.0, the default configuration file is now eslint.config.js.
If you are using a .eslintrc.* file, please follow the migration guide
to update your configuration file to the new format:

https://eslint.org/docs/latest/use/configure/migration-guide

If you still have problems after following the migration guide, please stop by
https://eslint.org/chat/help to chat with the team.

Walkthrough

This pull request involves a comprehensive update to the CodeRabbit landing application, focusing on migrating from an s0 to an s1 data model across multiple components and GraphQL queries. The changes span type definitions, data structure handling, and component logic, with a particular emphasis on enhancing type safety, error handling, and data integrity. Key modifications include updating GraphQL query references, refining type assertions, and streamlining component interactions.

Changes

File/Directory Change Summary
.storybook/ Removed Storybook configuration files and stories
schema.graphql Renamed all model methods from s0 to s1 prefix
src/components/ Updated type handling, removed unused components, enhanced error checking
src/hooks/ Modified GraphQL query references from s0 to s1 models
src/routes/ Improved data filtering and type safety
TypeScript Configs Comprehensive updates to compilation and module resolution settings

Suggested Reviewers

  • Bob0005

Poem

🐰 A Rabbit's Code Refactoring Tale

From s0 to s1, we hop and we sail
Types refined, with precision we scale
GraphQL queries dance a new ballet
Components shimmer in a cleaner way
CodeRabbit's magic makes errors pale! 🔧✨


📜 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 1dc109a and 57be4e0.

📒 Files selected for processing (3)
  • client/apps/landing/src/components/providers/starknet-provider.tsx (0 hunks)
  • client/apps/landing/src/hooks/query/eternum-statistics.tsx (1 hunks)
  • client/apps/landing/src/routes/index.lazy.tsx (7 hunks)
💤 Files with no reviewable changes (1)
  • client/apps/landing/src/components/providers/starknet-provider.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • client/apps/landing/src/hooks/query/eternum-statistics.tsx
  • client/apps/landing/src/routes/index.lazy.tsx

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@aymericdelab aymericdelab changed the title remove commment fix landing build and errors Jan 25, 2025
Copy link

Failed to generate code suggestions for PR

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🔭 Outside diff range comments (2)
client/apps/landing/src/components/ui/elements/resource-cost.tsx (1)

Line range hint 52-52: Fix potential runtime issues with dynamic className construction.

The template literal for className using textSize prop could cause runtime issues if textSize values change. Consider using a mapping object for text sizes.

-        <div onClick={onClick} className={clsx(`relative text-${textSize} font-bold`, props.color)}>
+        const TEXT_SIZE_CLASSES = {
+          xxs: 'text-xxs',
+          xs: 'text-xs',
+          sm: 'text-sm',
+          md: 'text-md',
+          lg: 'text-lg'
+        } as const;
+        <div onClick={onClick} className={clsx('relative font-bold', TEXT_SIZE_CLASSES[textSize], props.color)}>

Apply the same pattern to line 60:

-        <div className={`text-${textSize} leading-[10px] self-start relative mt-1 font-normal`}>{trait}</div>
+        <div className={clsx(TEXT_SIZE_CLASSES[textSize], 'leading-[10px] self-start relative mt-1 font-normal')}>{trait}</div>

Also applies to: 60-60

client/apps/landing/src/components/ui/utils/utils.ts (1)

Line range hint 82-86: Fix O(n²) time complexity in reduce.

The spread operator in the accumulator causes quadratic time complexity as it creates a new object and copies all properties in each iteration.

Use direct property assignment instead:

-        (acc: Record<string, number>, r) => ({
-          ...acc,
-          [r.resourceId]: (acc[r.resourceId] || 0) + r.amount / RESOURCE_PRECISION,
-        }),
+        (acc: Record<string, number>, r) => {
+          acc[r.resourceId] = (acc[r.resourceId] || 0) + r.amount / RESOURCE_PRECISION;
+          return acc;
+        },
🧹 Nitpick comments (15)
client/apps/landing/src/components/ui/elements/resource-icon.tsx (2)

Line range hint 11-11: Remove redundant comment.

The comment for tooltipText is redundant as TypeScript already provides type information through the Props interface.

-  tooltipText?: string; // Added custom tooltip text as optional
+  tooltipText?: string;

73-73: Extract repeated string sanitization logic.

The resource name sanitization logic is duplicated multiple times. Consider extracting it into a utility function.

+const sanitizeResourceName = (resource: string): string => 
+  resource.replace(" ", "").replace("'", "");

 export const ResourceIcon = ({ withTooltip = true, tooltipText, ...props }: Props) => {
+  const sanitizedResource = sanitizeResourceName(props.resource);
   const Icon = (
     <div className={`flex paper relative group rounded-xl justify-center ${props.className}`}>
       <div className={`relative ${clsx(STYLES.size[props.size], props.className)} `}>
-        {Components[props.resource.replace(" ", "").replace("'", "")]?.component}
+        {Components[sanitizedResource]?.component}
       </div>

       {props.label && (
         <span className="self-center ml-4 text-center">
-          {Components[props.resource.replace(" ", "").replace("'", "")]?.name}
+          {Components[sanitizedResource]?.name}
         </span>
       )}
       {withTooltip && (
         <div className="absolute -top-2 flex-col items-center hidden -translate-y-full left-1/2 -translate-x-1/2 bg-brown rounded-lg w-max group-hover:flex">
           <span className="relative z-10 p-2 text-xs leading-none  whitespace-no-wrap rounded shadow-lg bg-gray-1000">
-            {tooltipText || Components[props.resource.replace(" ", "").replace("'", "")]?.name}
+            {tooltipText || Components[sanitizedResource]?.name}
           </span>

Also applies to: 89-89

client/apps/landing/src/components/modules/realm-card.tsx (2)

21-24: Consider using null or undefined instead of empty strings for fallback values.

While the type guard pattern is good, using empty strings as fallback values might mask errors in the UI. Consider using null or undefined to make it more explicit when the tokenMetadata is invalid.

  const { tokenId, contractAddress, metadata } =
    realm.node?.tokenMetadata.__typename === "ERC721__Token"
      ? realm.node.tokenMetadata
-      : { tokenId: "", contractAddress: "", metadata: "" };
+      : { tokenId: null, contractAddress: null, metadata: null };

Line range hint 92-92: Remove commented out code.

There are two instances of commented out code that should be removed to maintain code cleanliness:

  1. {/*description*/} in CardDescription
  2. {/* {Number(tokenId)} */} in CardContent
-        <CardDescription>{/*description*/}</CardDescription>
+        <CardDescription />

         {/* ... */}

-        {/* {Number(tokenId)} */}

Also applies to: 86-86

client/apps/landing/src/components/modules/bridge-out-step-2.tsx (1)

41-41: Remove unused state variable.

The destructured state variable _ is never used. Consider removing the state if it's not needed.

-  const [_, setRefreshTrigger] = useState(0);
+  const [setRefreshTrigger] = useState(0);
client/apps/landing/src/components/modules/bridge-in.tsx (2)

153-153: Consider simplifying the token address lookup.

The immediate conversion to uppercase might be redundant if the keys in resourceAddresses are already uppercase. Consider verifying the casing requirements.

-              resourceAddresses[ResourcesIds[Number(id)].toLocaleUpperCase() as keyof typeof resourceAddresses][1];
+              resourceAddresses[ResourcesIds[Number(id)] as keyof typeof resourceAddresses][1];

393-393: Consider using a more flexible number formatting approach.

Using toFixed(2) might truncate significant digits. Consider using Intl.NumberFormat for more flexible formatting, consistent with the formatting used elsewhere in the file.

-        ? Number(formatEther(lordsBalance)).toFixed(2)
+        ? Intl.NumberFormat('en-US', { minimumFractionDigits: 2, maximumFractionDigits: 2 }).format(Number(formatEther(lordsBalance)))
client/apps/landing/src/hooks/gql/graphql.ts (3)

Line range hint 7574-7595: Consider adding pagination to leaderboard queries.

While the current implementation works, consider adding pagination support for leaderboard queries to handle large datasets more efficiently.

  s1EternumLeaderboardModels?: {
    __typename?: "s1_eternum_LeaderboardConnection";
+   pageInfo: {
+     hasNextPage: boolean
+     endCursor?: string
+   }
    edges?: Array<{
      __typename?: "s1_eternum_LeaderboardEdge";
+     cursor: string
      node?: {
        __typename?: "s1_eternum_Leaderboard";

Line range hint 7658-7693: Consider adding error handling for resource queries.

While the resource queries are structured correctly, consider adding error handling for cases where resources might not exist.

  s1EternumResourceModels?: {
    __typename?: "s1_eternum_ResourceConnection";
+   errors?: Array<{
+     message: string
+     path: Array<string>
+   }>
    edges?: Array<{

Line range hint 7831-7878: Consider adding rate limiting for leaderboard queries.

To prevent potential abuse, consider adding rate limiting for leaderboard-related queries.

export const GetLeaderboardEntryDocument = new TypedDocumentString(`
    query getLeaderboardEntry($accountAddress: ContractAddress!) {
+     rateLimit(limit: 100, window: "1m") {
+       remaining
+     }
      s1EternumLeaderboardEntryModels(where: {address: $accountAddress}) {
client/apps/landing/src/hooks/use-lords.tsx (2)

14-15: Consider extracting type assertions to constants.

To improve maintainability and reusability, consider extracting the type assertions to constants.

+const ZERO_ADDRESS = "0" as `0x${string}`;
+
 const { data } = useCall({
   abi: abi as Abi,
   functionName: "balance_of",
-  address: lordsAddress as `0x${string}`,
-  args: [(account?.address as `0x${string}`) ?? "0"],
+  address: lordsAddress as `0x${string}`,
+  args: [account?.address as `0x${string}` ?? ZERO_ADDRESS],

17-17: Extract refetchInterval to a constant.

Consider extracting the hardcoded refetchInterval value to a named constant for better maintainability.

+const LORDS_BALANCE_REFETCH_INTERVAL = 1000; // milliseconds
+
 const { data } = useCall({
   // ...
-  refetchInterval: 1000,
+  refetchInterval: LORDS_BALANCE_REFETCH_INTERVAL,
client/apps/landing/src/hooks/helpers/use-entities.tsx (1)

30-31: Remove debugging console.log statement.

This console.log statement appears to be debugging code that should be removed before merging.

-  console.log({ data });
client/apps/landing/tsconfig.json (1)

22-24: Consider enabling commented type-checking options.

The commented options noUnusedLocals and noUnusedParameters can help maintain cleaner code by catching unused variables and parameters. Consider enabling them unless there's a specific reason not to.

client/apps/landing/src/hooks/use-travel.tsx (1)

15-20: Use strict equality operator.

Replace loose equality (==) with strict equality (===) for type-safe comparisons.

-    const fromPosition = entityPositions?.s1EternumPositionModels?.edges?.find(
-      (entity) => entity?.node?.entity_id == fromId,
-    );
-    const toPosition = entityPositions?.s1EternumPositionModels?.edges?.find(
-      (entity) => entity?.node?.entity_id == toId,
-    );
+    const fromPosition = entityPositions?.s1EternumPositionModels?.edges?.find(
+      (entity) => entity?.node?.entity_id === fromId,
+    );
+    const toPosition = entityPositions?.s1EternumPositionModels?.edges?.find(
+      (entity) => entity?.node?.entity_id === toId,
+    );
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aef638a and 1dc109a.

📒 Files selected for processing (57)
  • client/apps/landing/.storybook/main.ts (0 hunks)
  • client/apps/landing/.storybook/preview.ts (0 hunks)
  • client/apps/landing/schema.graphql (115 hunks)
  • client/apps/landing/src/components/modules/app-sidebar.tsx (0 hunks)
  • client/apps/landing/src/components/modules/bridge-in.tsx (5 hunks)
  • client/apps/landing/src/components/modules/bridge-out-step-1.tsx (5 hunks)
  • client/apps/landing/src/components/modules/bridge-out-step-2.tsx (3 hunks)
  • client/apps/landing/src/components/modules/bridged-resources.tsx (2 hunks)
  • client/apps/landing/src/components/modules/realm-card.tsx (2 hunks)
  • client/apps/landing/src/components/modules/realm-mint-dialog.tsx (1 hunks)
  • client/apps/landing/src/components/modules/realms-grid.tsx (3 hunks)
  • client/apps/landing/src/components/modules/season-pass-card.tsx (1 hunks)
  • client/apps/landing/src/components/modules/season-pass-mint-dialog.tsx (1 hunks)
  • client/apps/landing/src/components/modules/season-passes-grid.tsx (3 hunks)
  • client/apps/landing/src/components/modules/top-navigation-view.tsx (2 hunks)
  • client/apps/landing/src/components/modules/top-navigation.tsx (1 hunks)
  • client/apps/landing/src/components/modules/transfer-season-pass-dialog.tsx (2 hunks)
  • client/apps/landing/src/components/providers/starknet-provider.tsx (0 hunks)
  • client/apps/landing/src/components/ui/elements/list-select.tsx (1 hunks)
  • client/apps/landing/src/components/ui/elements/resource-cost.tsx (1 hunks)
  • client/apps/landing/src/components/ui/elements/resource-icon.tsx (1 hunks)
  • client/apps/landing/src/components/ui/multi-select.tsx (0 hunks)
  • client/apps/landing/src/components/ui/select-resources.tsx (6 hunks)
  • client/apps/landing/src/components/ui/utils/addresses.ts (1 hunks)
  • client/apps/landing/src/components/ui/utils/utils.ts (1 hunks)
  • client/apps/landing/src/custom.d.ts (1 hunks)
  • client/apps/landing/src/hooks/gql/gql.ts (2 hunks)
  • client/apps/landing/src/hooks/gql/graphql.ts (21 hunks)
  • client/apps/landing/src/hooks/helpers/use-donkey-arrivals.tsx (4 hunks)
  • client/apps/landing/src/hooks/helpers/use-entities.tsx (3 hunks)
  • client/apps/landing/src/hooks/helpers/use-resources.tsx (1 hunks)
  • client/apps/landing/src/hooks/query/capacity-config.tsx (1 hunks)
  • client/apps/landing/src/hooks/query/entities.tsx (1 hunks)
  • client/apps/landing/src/hooks/query/entity-owners.tsx (1 hunks)
  • client/apps/landing/src/hooks/query/eternum-statistics.tsx (1 hunks)
  • client/apps/landing/src/hooks/query/leaderboard.tsx (5 hunks)
  • client/apps/landing/src/hooks/query/position.tsx (1 hunks)
  • client/apps/landing/src/hooks/query/resources.tsx (1 hunks)
  • client/apps/landing/src/hooks/use-bridge.tsx (1 hunks)
  • client/apps/landing/src/hooks/use-lords.tsx (1 hunks)
  • client/apps/landing/src/hooks/use-mint-season-pass.tsx (1 hunks)
  • client/apps/landing/src/hooks/use-prize-claim.tsx (5 hunks)
  • client/apps/landing/src/hooks/use-travel.tsx (1 hunks)
  • client/apps/landing/src/lib/utils.ts (1 hunks)
  • client/apps/landing/src/routes/index.lazy.tsx (5 hunks)
  • client/apps/landing/src/routes/mint.lazy.tsx (2 hunks)
  • client/apps/landing/src/routes/season-passes.lazy.tsx (5 hunks)
  • client/apps/landing/src/routes/trade.lazy.tsx (1 hunks)
  • client/apps/landing/src/stories/button.stories.tsx (0 hunks)
  • client/apps/landing/src/stories/data-card.stories.tsx (0 hunks)
  • client/apps/landing/src/stories/leaderboard-panel.stories.tsx (0 hunks)
  • client/apps/landing/src/stories/season-pass-card.stories.tsx (0 hunks)
  • client/apps/landing/src/stories/top-navigation-view.stories.tsx (0 hunks)
  • client/apps/landing/tsconfig.app.tsbuildinfo (0 hunks)
  • client/apps/landing/tsconfig.json (1 hunks)
  • client/apps/landing/tsconfig.node.json (1 hunks)
  • client/apps/landing/vite.config.ts (1 hunks)
💤 Files with no reviewable changes (11)
  • client/apps/landing/.storybook/main.ts
  • client/apps/landing/src/stories/season-pass-card.stories.tsx
  • client/apps/landing/src/components/modules/app-sidebar.tsx
  • client/apps/landing/src/stories/button.stories.tsx
  • client/apps/landing/src/stories/top-navigation-view.stories.tsx
  • client/apps/landing/src/components/providers/starknet-provider.tsx
  • client/apps/landing/.storybook/preview.ts
  • client/apps/landing/src/stories/data-card.stories.tsx
  • client/apps/landing/tsconfig.app.tsbuildinfo
  • client/apps/landing/src/stories/leaderboard-panel.stories.tsx
  • client/apps/landing/src/components/ui/multi-select.tsx
✅ Files skipped from review due to trivial changes (5)
  • client/apps/landing/src/hooks/query/position.tsx
  • client/apps/landing/src/hooks/query/capacity-config.tsx
  • client/apps/landing/src/hooks/query/resources.tsx
  • client/apps/landing/src/hooks/query/entity-owners.tsx
  • client/apps/landing/src/hooks/query/leaderboard.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
client/apps/landing/src/components/modules/bridge-out-step-2.tsx

[error] 83-83: Avoid the use of spread (...) syntax on accumulators.

Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.

(lint/performance/noAccumulatingSpread)

🔇 Additional comments (56)
client/apps/landing/src/components/ui/elements/resource-cost.tsx (1)

1-1: LGTM! Clean removal of isLabor prop.

The removal of the isLabor prop is consistent with the changes in ResourceIcon component, and the addition of divideByPrecision improves precision handling.

Also applies to: 27-27

client/apps/landing/src/components/ui/elements/resource-icon.tsx (1)

73-73: LGTM! Clean removal of isLabor parameter.

The removal of the isLabor parameter is consistent with the Props type changes and ResourceCost component updates.

client/apps/landing/src/components/modules/realm-card.tsx (1)

37-37: Verify seasonPassAddress format in config.

The type assertion ensures the address matches the 0x${string} format. Let's verify that seasonPassAddress from config always follows this pattern.

✅ Verification successful

Type assertion is correctly used for contract address format

The seasonPassAddress comes from a centralized, chain-specific configuration system and is consistently used with the 0x${string} type assertion across multiple contract interactions. This pattern ensures type safety while maintaining compatibility with the blockchain's address format requirements.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check seasonPassAddress format in config file
# Expected: Address should start with "0x"

echo "Checking seasonPassAddress format..."
rg -A 1 "seasonPassAddress.*=.*" client/apps/landing/src/config

Length of output: 219


Script:

#!/bin/bash
# Search for seasonPassAddress in the entire client/apps/landing directory
echo "Searching for seasonPassAddress definition..."
rg -l "seasonPassAddress" client/apps/landing/src

# If found, show the context of its definition
echo -e "\nShowing definition context..."
rg -B2 -A2 "seasonPassAddress" client/apps/landing/src

Length of output: 6588


Script:

#!/bin/bash
# Search for getSeasonPassAddress implementation
echo "Searching for getSeasonPassAddress implementation..."
rg -B3 -A3 "getSeasonPassAddress" client/apps/landing/src

Length of output: 1166


Script:

#!/bin/bash
# Search for getSeasonAddresses implementation and related types
echo "Searching for getSeasonAddresses implementation..."
rg -B5 -A10 "getSeasonAddresses" client/apps/landing/src

echo -e "\nSearching for type definitions containing seasonPass..."
rg -B2 -A2 "type.*{.*seasonPass" client/apps/landing/src

Length of output: 2377

client/apps/landing/src/lib/utils.ts (1)

1-1: LGTM! Good refactoring.

Removing local implementations and using the centralized functions from @bibliothecadao/eternum improves maintainability and reduces code duplication.

client/apps/landing/src/components/modules/bridge-out-step-2.tsx (1)

296-302: LGTM! Good type safety improvement.

Converting resource IDs to numbers before accessing them in ResourcesIds ensures type safety.

client/apps/landing/src/components/modules/bridge-out-step-1.tsx (2)

290-295: LGTM! Good type safety improvement.

Converting resource IDs to numbers before accessing them in ResourcesIds ensures type safety.


52-53: Verify GraphQL schema changes.

The GraphQL model references have been updated from s0 to s1. Please ensure that these changes are consistent with the updated schema.

✅ Verification successful

GraphQL schema changes verified successfully

The schema changes from s0 to s1 are consistent and complete. The required fields weight_gram and sec_per_km exist in their respective types with the correct connection structure.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining s0 references that might need updating
rg -g '*.{ts,tsx,graphql}' 's0EternumCapacityConfigModels|s0EternumSpeedConfigModels'

# Verify the new s1 models exist in the schema
rg -g '*.graphql' 's1EternumCapacityConfigModels|s1EternumSpeedConfigModels'

Length of output: 299


Script:

#!/bin/bash
# Extract the complete schema definition for s1 models
rg -A 10 -g '*.graphql' 's1EternumCapacityConfigModels|s1EternumSpeedConfigModels'

Length of output: 1428


Script:

#!/bin/bash
# Search for connection and node type definitions
rg -A 15 -g '*.graphql' 'type s1_eternum_(Capacity|Speed)Config(Connection|$)'

Length of output: 1871


Script:

#!/bin/bash
# Search for the actual Config type definitions
rg -A 10 -g '*.graphql' '^type s1_eternum_(Capacity|Speed)Config\s*{'

Length of output: 221


Script:

#!/bin/bash
# Search for the actual Config type definitions with simpler pattern
rg -A 10 -g '*.graphql' '^type s1_eternum_(Capacity|Speed)Config'

Length of output: 2874

client/apps/landing/src/components/modules/bridge-in.tsx (2)

329-329: LGTM! Type-safe resource handling.

The changes improve type safety by using the ResourcesIds enum for resource name lookup and display.

Also applies to: 333-334


48-49: Verify schema version consistency across queries.

The GraphQL schema references have been updated from s0 to s1. Let's ensure this change is consistent across all related queries.

✅ Verification successful

Schema version migration is complete and consistent

The codebase has been fully migrated from s0 to s1 schema version, with no remaining s0 references. All GraphQL queries and components consistently use the new s1 prefix.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining s0 references in GraphQL queries
rg -g '*.{ts,tsx,graphql}' 's0Eternum(Capacity|Speed)ConfigModels' ./client/

Length of output: 78


Script:

#!/bin/bash
# Search for any remaining s0 references in GraphQL related files
echo "=== Searching for s0 prefixed types ==="
rg -g '*.{ts,tsx,graphql}' 's0[A-Z][a-zA-Z]+' ./client/

echo -e "\n=== Searching for s1 prefixed types ==="
rg -g '*.{ts,tsx,graphql}' 's1[A-Z][a-zA-Z]+' ./client/

echo -e "\n=== Checking GraphQL schema files ==="
fd -g '*.graphql' . ./client/ --exec cat {}

Length of output: 91711

client/apps/landing/src/hooks/gql/graphql.ts (7)

7181-7188: LGTM! The query structure for capacity and speed config looks good.

The query is well-structured with appropriate fields for retrieving weight_gram and sec_per_km values.


7533-7536: LGTM! Statistics queries are properly structured.

The statistics queries for address names, hyperstructures, realms and fragment mines are well-defined with totalCount fields.


Line range hint 7546-7561: LGTM! Game state queries are properly implemented.

The queries for game ended state and winner address are correctly structured.


Line range hint 7612-7637: LGTM! Epoch queries include all necessary fields.

The epoch queries correctly include start timestamp, hyperstructure entity ID, index and owners array.


7804-7820: LGTM! Statistics document strings are properly typed.

The TypedDocumentString implementations for statistics queries are correctly defined with proper typing.


Line range hint 7895-7911: LGTM! Position and resource queries are well-structured.

The entity position and resource queries are properly implemented with appropriate fields and typing.


Line range hint 7203-7343: Verify the realm entity type discriminator.

The realm entity type discriminator in the query looks correct, but let's verify that it properly handles all possible entity types. The current implementation includes a comprehensive list of entity types which is good for type safety.

✅ Verification successful

Realm entity type discriminator implementation is complete and type-safe

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify realm entity type discriminator completeness
# Search for any entity types that might be missing from the discriminator

# Look for entity type definitions
ast-grep --pattern 'type s1_eternum_$_' 

# Compare with types in the discriminator
rg "__typename:" -A 1 client/apps/landing/src/hooks/gql/graphql.ts

Length of output: 13916

client/apps/landing/src/custom.d.ts (1)

1-7: LGTM! Well-structured SVG type declaration.

The TypeScript declaration for SVG modules is well-implemented with proper typing for React components and SVG props, including accessibility support via the optional title prop.

client/apps/landing/src/hooks/query/eternum-statistics.tsx (1)

5-16: LGTM! Model prefix updates are consistent.

The GraphQL query updates from s0 to s1 prefixes are consistent with the schema changes mentioned in the summary.

client/apps/landing/src/hooks/query/entities.tsx (2)

Line range hint 5-19: LGTM! Model prefix update is consistent.

The GraphQL query update from s0 to s1 prefix is consistent with the schema changes.


5-5: Verify the high pagination limit.

The query uses a high limit of 8000 records. Please verify that this limit is necessary and doesn't impact performance.

✅ Verification successful

High pagination limit is consistent with the codebase design

The limit of 8000 records is consistently used across multiple similar queries in the codebase, particularly for core game data like resources, positions, and tokens. Different limit tiers (1000, 8000, 10000) are used based on the query type, suggesting this is a deliberate architectural decision.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for potential performance impact of high pagination limit

# Search for other instances of high limits
echo "Searching for other high pagination limits..."
rg -i "limit: \d{4,}" --type ts

# Search for performance-related comments or issues
echo "Searching for performance-related comments..."
rg -i "performance|pagination|limit" --type ts

Length of output: 23948

client/apps/landing/src/hooks/use-lords.tsx (1)

14-15: LGTM! Type assertions improve type safety.

The type assertions for lordsAddress and account.address properly ensure the hexadecimal string format.

client/apps/landing/src/routes/trade.lazy.tsx (1)

12-12: Verify CountdownTimer functionality without targetDate prop

The CountdownTimer component is now rendered without the previously provided targetDate prop. Please ensure that:

  1. The component has a default target date internally
  2. The removal of this prop was intentional and doesn't break the countdown functionality
client/apps/landing/src/components/ui/utils/addresses.ts (1)

6-8: Great improvement in type safety!

The explicit type assertion for the addresses return value enhances code maintainability and helps catch potential type-related issues early in development.

client/apps/landing/src/hooks/helpers/use-resources.tsx (1)

19-19: Verify complete migration from s0 to s1 schema

The update from s0EternumResourceModels to s1EternumResourceModels is part of a broader schema version update.

Also applies to: 25-25

✅ Verification successful

Schema migration from s0 to s1 is complete

No remaining references to s0Eternum were found in the codebase, confirming that the schema migration has been completed consistently.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining s0 references that might have been missed
rg "s0Eternum" --type typescript --type graphql

Length of output: 82


Script:

#!/bin/bash
# Search for s0Eternum in TypeScript files
rg "s0Eternum" -g "*.ts" -g "*.tsx"

# Search for s0Eternum in GraphQL files
rg "s0Eternum" -g "*.graphql"

Length of output: 65

client/apps/landing/src/components/modules/top-navigation.tsx (1)

37-37: Verify bigint handling in TopNavigationView

The lordsBalance is now passed directly without type assertion. Please ensure that:

  1. TopNavigationView properly handles the bigint type
  2. All calculations and displays involving lordsBalance account for the type change
✅ Verification successful

✓ lordsBalance bigint handling is correct

The TopNavigationView component properly handles the bigint type through:

  • Correct type definition in props interface
  • Safe conversion using formatEther() and number formatting
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for TopNavigationView implementation and lordsBalance usage
ast-grep --pattern 'interface $_Props {
  $$$
  lordsBalance: $_
  $$$
}' -l typescript
rg -A 5 "lordsBalance.*:.*bigint" --type typescript

Length of output: 178


Script:

#!/bin/bash
# Search for TopNavigationView component and its props
ast-grep --pattern 'interface TopNavigationViewProps {
  $$$
}'

# Search for lordsBalance type and usage
rg -t tsx -t ts "lordsBalance.*:.*" -B 2 -A 2
rg -t tsx -t ts "lordsBalance" -B 2 -A 2

# Search for any bigint operations or formatting
rg -t tsx -t ts "BigInt|bigint" -B 2 -A 2

Length of output: 1058


Script:

#!/bin/bash
# Search for lordsBalance usage in the component implementation
rg "lordsBalance" client/apps/landing/src/components/modules/top-navigation-view.tsx -B 2 -A 5

Length of output: 894

client/apps/landing/vite.config.ts (1)

13-13: Verify the removal of TanStackRouterVite plugin.

The removal of TanStackRouterVite plugin might affect routing capabilities. Please ensure that an alternative routing solution is in place.

✅ Verification successful

Removal of TanStackRouterVite plugin appears safe

The TanStack Router dependencies are still present in the project, and the Vite plugin is optional for development tools. The router functionality should continue to work without the plugin.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for alternative routing solutions
echo "Checking for routing-related dependencies..."
cat package.json | jq '.dependencies + .devDependencies' | grep -i "router"

echo "Checking for router usage in the codebase..."
rg -l "router|Route|useRoute" --type ts --type tsx

Length of output: 354


Script:

#!/bin/bash
# Check for TanStack router specific usage
echo "Searching for TanStack router imports and usage..."
rg -l "@tanstack/router|createRouter|RouterProvider" --type typescript

echo "Searching for router configuration files..."
fd -e ts -e tsx router.ts router.tsx

echo "Checking package.json for TanStack router..."
find . -name package.json -exec sh -c 'echo "=== $1 ==="; cat "$1" | grep -A 1 "@tanstack/router"' sh {} \;

Length of output: 11788


Script:

#!/bin/bash
# Search for TanStack router usage with correct file patterns
echo "Searching for TanStack router usage..."
rg -t ts -t tsx "@tanstack/router" -A 2

echo "Searching for router configuration..."
rg -t ts -t tsx "createRouter|RouterProvider" -A 2

Length of output: 320

client/apps/landing/src/hooks/helpers/use-entities.tsx (2)

8-18: LGTM! Type definitions updated consistently.

The renaming from s0 to s1 has been applied consistently across interfaces and type guards.


Line range hint 35-44: Verify the impact of s0 to s1 transition on data mapping.

The changes in type guards and model names affect how realm and structure data is mapped. Please ensure that all consumers of this hook are updated accordingly.

Also applies to: 51-62

✅ Verification successful

Transition to s1 models is complete and consistent

The codebase has been fully migrated to use s1 models and type guards. No remaining s0 references were found, and all consumers of the useEntities hook are properly using the updated model names.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any remaining s0 references
echo "Checking for remaining s0 references..."
rg "s0_eternum" --type ts --type tsx

echo "Checking for hook usage..."
rg "useEntities" --type ts --type tsx -A 5

Length of output: 287


Script:

#!/bin/bash
# Search for s0/s1 references and hook usage
echo "Checking for s0 references..."
rg "s0.*eternum" -t ts -t tsx

echo -e "\nChecking for useEntities hook usage..."
rg "useEntities" -t ts -t tsx -A 5

echo -e "\nChecking for type guard usage..."
rg "is.*EternumRealm" -t ts -t tsx

Length of output: 430


Script:

#!/bin/bash
# Find TypeScript files and search for patterns
echo "Searching for s0/s1 references and hook usage..."
fd "\.(ts|tsx)$" | while read -r file; do
  echo "=== $file ==="
  grep -E "s[01].*eternum|useEntities|is.*EternumRealm" "$file" || true
done

echo -e "\nChecking for type guard usage with ast-grep..."
ast-grep --pattern 'function is$_EternumRealm($_): $_ { $$$ }'

echo -e "\nChecking for hook usage with ast-grep..."
ast-grep --pattern 'const $_ = useEntities($$$)'

Length of output: 131070

client/apps/landing/src/hooks/use-mint-season-pass.tsx (1)

15-15: LGTM! Improved type safety for contract address.

The type assertion ensures that the address conforms to the expected 0x-prefixed hexadecimal format.

client/apps/landing/src/components/modules/season-pass-card.tsx (2)

7-7: LGTM! Simplified prop type definition.

The prop type has been simplified to use TokenBalance, making the component more maintainable.


14-17: LGTM! Improved error handling with type guard.

The addition of the type guard and default values prevents runtime errors when the expected data structure is not met.

client/apps/landing/src/hooks/use-prize-claim.tsx (1)

19-19: LGTM! Consistent model prefix updates.

The changes from s0 to s1 prefixes are applied consistently across all data access points, maintaining the same structure and functionality.

Also applies to: 31-31, 43-43, 56-56, 69-69, 71-71

client/apps/landing/src/components/modules/top-navigation-view.tsx (1)

16-16: Type change improves code clarity.

The change from Uint256 | undefined to bigint simplifies the type system and removes unnecessary conversion. The direct use of formatEther with bigint is more idiomatic.

Also applies to: 42-42

✅ Verification successful

Type change is consistent with codebase patterns

The change from Uint256 | undefined to bigint follows the established patterns in the codebase where:

  • bigint is consistently used for blockchain values
  • formatEther is properly used with bigint values
  • The conversion pattern Number(formatEther(value)) is standard across components
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for consistent bigint usage with formatEther
rg -A 3 "formatEther.*bigint" --type typescript

Length of output: 84


Script:

#!/bin/bash
# Search for formatEther usage with proper file extensions
rg -A 3 "formatEther" --type-add 'ts:*.{ts,tsx}' --type ts

# Search for Uint256 type usage
rg -A 3 "Uint256" --type-add 'ts:*.{ts,tsx}' --type ts

# Search for bigint type declarations
rg -A 3 ":\s*bigint" --type-add 'ts:*.{ts,tsx}' --type ts

Length of output: 34128

client/apps/landing/src/routes/season-passes.lazy.tsx (4)

22-29: Good refactoring: Function extraction improves modularity.

Extracting getSeasonPassNfts function improves code organization and reusability.


31-31: Type safety improvement.

Adding explicit TokenBalance type enhances type safety and makes the code more maintainable.


Line range hint 51-55: Loading state handling improvement.

The loading state UI now properly covers the entire content area with a semi-transparent background and centered spinner.


71-71: Safer conditional rendering.

The conditional rendering of SeasonPassesGrid prevents potential undefined errors.

client/apps/landing/src/components/modules/season-passes-grid.tsx (2)

95-95: Good type safety improvement!

The added type check pass.node.tokenMetadata.__typename === "ERC721__Token" ensures safe access to tokenId, preventing potential runtime errors.


14-14: Clean interface simplification!

Good job simplifying the interfaces by:

  1. Replacing complex GetRealmsQuery type with TokenBalance
  2. Making seasonPasses a required prop instead of optional

Also applies to: 19-19

client/apps/landing/src/components/modules/realms-grid.tsx (2)

88-91: Excellent type safety improvements!

Good job adding type guards for ERC721__Token in multiple places:

  1. isNftSelected parameters
  2. RealmCard key generation

This prevents potential runtime errors when accessing tokenId and contractAddress.

Also applies to: 96-96


14-14: Consistent interface updates!

Good job maintaining consistency with season-passes-grid.tsx by:

  1. Using TokenBalance type
  2. Making realms a required prop

Also applies to: 18-18

client/apps/landing/src/components/ui/elements/list-select.tsx (1)

26-26: Better default label text!

Changed from "Select a wallet..." to "Select a resource..." which is more generic and better reflects the component's purpose.

client/apps/landing/src/components/modules/season-pass-mint-dialog.tsx (1)

66-66: Good route path update!

  1. Changed from /passes to /season-passes for better clarity
  2. Fixed Link prop from href to to for correct routing
client/apps/landing/src/routes/index.lazy.tsx (1)

58-58: LGTM! Consistent schema updates.

The changes correctly update all GraphQL schema references from s0 to s1 while maintaining the existing property paths and logic.

Also applies to: 67-67, 76-76, 85-85, 125-125

client/apps/landing/src/hooks/helpers/use-donkey-arrivals.tsx (1)

53-56: LGTM! Comprehensive schema updates.

The changes systematically update all GraphQL schema references from s0 to s1 while maintaining type safety and existing business logic.

Also applies to: 67-67, 73-73, 75-83, 100-100, 109-117

client/apps/landing/src/components/modules/realm-mint-dialog.tsx (1)

33-33: LGTM! Enhanced type safety.

Adding explicit type annotation number[] for filteredRealmTokenIds improves type safety and code clarity.

client/apps/landing/src/routes/mint.lazy.tsx (1)

97-97: LGTM! Improved error handling and type safety.

The changes enhance the code's robustness by:

  1. Adding null safety for realmsErcBalance
  2. Adding proper type checking for ERC721 tokens
  3. Preventing potential runtime errors when accessing token metadata

Also applies to: 117-121, 123-125

client/apps/landing/src/components/modules/transfer-season-pass-dialog.tsx (1)

46-46: LGTM! Type safety improvement.

The addition of the type assertion as \0x${string}`` ensures the address conforms to the expected hex string format.

client/apps/landing/src/components/modules/bridged-resources.tsx (1)

17-17: LGTM! Type handling improvements.

The changes enhance type safety and data handling:

  1. More explicit state type definition using Array<[string, [number, string]]>
  2. Proper type conversion of resourceId using Number()
  3. Explicit string conversion of contractAddress using toString()

Also applies to: 96-96, 98-98

client/apps/landing/src/components/ui/select-resources.tsx (4)

2-2: LGTM! Better code organization.

Moving the divideByPrecision import to the library improves code organization and reusability.


43-43: LGTM! Improved error handling.

Added null safety checks using optional chaining (?.) and null coalescing (|| 0) to prevent runtime errors when accessing potentially undefined values.

Also applies to: 82-82


65-65: LGTM! UI consistency improvement.

Added fixed heights to components for better UI consistency and layout stability.

Also applies to: 82-82


184-192: LGTM! Enhanced button styling.

Updated button properties for better accessibility and visual consistency:

  • Changed variant from 'red' to more semantic 'destructive'
  • Updated size from 'xs' to 'sm' for better touch targets
  • Added className for padding adjustment
client/apps/landing/src/hooks/gql/gql.ts (1)

16-40: LGTM! Consistent schema version update.

All GraphQL queries have been systematically updated from s0 to s1 prefix, maintaining consistency across the entire schema. This change appears to be part of a broader schema version update.

Let's verify that all schema references have been updated consistently:

Also applies to: 48-120

✅ Verification successful

Schema version update verification complete - All references consistently using 's1' prefix

The codebase shows complete and consistent migration from 's0' to 's1' prefix across all GraphQL queries and type definitions. No remaining 's0' references were found.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining 's0' references in GraphQL files

# Search for any remaining 's0' references in GraphQL files
rg -l 's0Eternum' --type ts

Length of output: 27


Script:

#!/bin/bash
# Comprehensive check for schema versions

# Find all GraphQL-related TypeScript files
echo "=== GraphQL Files ==="
fd "\.(graphql|gql)\.ts$"

# Search for any 's0' or 's1' prefixes in TypeScript files
echo -e "\n=== Schema Version References ==="
rg "s[01]_eternum|s[01]Eternum" --type ts -l

# Search for GraphQL type definitions
echo -e "\n=== GraphQL Type Definitions ==="
ast-grep --pattern 'type $_ = {
  $$$
}'

Length of output: 65856


Script:

#!/bin/bash
# Comprehensive schema version check

echo "=== Checking for any 's0' references ==="
rg -i "s0[_\.]?eternum" --type ts -C 2

echo -e "\n=== Checking GraphQL query strings ==="
rg "query.*\{.*s[01].*\}" --type ts -C 2

echo -e "\n=== Checking type definitions ==="
rg "type.*s[01].*=.*" --type ts

Length of output: 35229

client/apps/landing/tsconfig.node.json (1)

3-6: Verify impact of removed type-checking options.

While the changes to moduleResolution and composite options are appropriate, the removal of strict type-checking options (strict, noUnusedLocals, noUnusedParameters, noFallthroughCasesInSwitch) could potentially introduce type-safety issues.

client/apps/landing/src/hooks/use-travel.tsx (1)

15-20: Verify s0 to s1 migration completeness.

Ensure all references to s0EternumPositionModels have been updated to s1EternumPositionModels across the codebase.

✅ Verification successful

Migration to s1EternumPositionModels is complete

All references have been successfully migrated to use the s1 prefix. No remaining s0 references were found in the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining s0 references
echo "Checking for remaining s0 references..."
rg "s0Eternum" --type ts --type tsx

# Search for any inconsistent usage of s1
echo "Checking for s1 usage..."
rg "s1Eternum" --type ts --type tsx

Length of output: 275


Script:

#!/bin/bash
# Search for any remaining s0 references with context
echo "Checking for remaining s0 references..."
rg "s0Eternum" -A 2 -B 2

# Search for s1 usage with context
echo "Checking for s1 usage..."
rg "s1Eternum" -A 2 -B 2

# Search for general Eternum references to ensure we don't miss any variations
echo "Checking for all Eternum references..."
rg "Eternum.*Position" -A 2 -B 2

Length of output: 66951

@@ -2,16 +2,16 @@ import { graphql } from "../gql";

export const GET_ETERNUM_STATTISTICS = graphql(`
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix typo in constant name.

The constant name has a typo: GET_ETERNUM_STATTISTICS should be GET_ETERNUM_STATISTICS.

-export const GET_ETERNUM_STATTISTICS = graphql(`
+export const GET_ETERNUM_STATISTICS = graphql(`
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const GET_ETERNUM_STATTISTICS = graphql(`
export const GET_ETERNUM_STATISTICS = graphql(`

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good bot

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bob0005 Thank you for the confirmation! Happy to help improve the code quality. 🎉


If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!

@@ -63,7 +63,7 @@ export const useBridgeAsset = () => {
);

const _bridgeFinishWithdrawFromRealm = useCallback(
async (donkeyResources: { tokenAddress: string; from_entity_id: bigint }[], throughBankId: bigint) => {
async (donkeyResources: { tokenAddress: string | number; from_entity_id: bigint }[], throughBankId: bigint) => {
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Type inconsistency in bridge function parameters

The tokenAddress parameter type is inconsistent across bridge functions:

  • _bridgeIntoRealm and _bridgeStartWithdrawFromRealm expect string
  • _bridgeFinishWithdrawFromRealm accepts string | number

This inconsistency, combined with no visible type conversion, could lead to runtime errors if the underlying contract expects a specific type.

🔗 Analysis chain

Verify handling of number type for tokenAddress.

The type of tokenAddress has been broadened to accept both string and number. Please ensure that both types are handled correctly in the bridge contract interaction.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for tokenAddress usage in bridge-related files
rg -A 5 "tokenAddress" --type typescript

Length of output: 75


Script:

#!/bin/bash
# Search for tokenAddress in TypeScript/TSX files
rg -t ts -t tsx -B 2 -A 5 "tokenAddress"

# Find type definitions and function signatures related to tokenAddress
ast-grep --pattern 'interface $_ {
  $$$
  tokenAddress: $_
  $$$
}'

# Search for bridge contract interactions
ast-grep --pattern 'async function $_($$$) {
  $$$
  tokenAddress
  $$$
}'

Length of output: 216


Script:

#!/bin/bash
# Find TypeScript files in hooks directory
fd . client/apps/landing/src/hooks -e ts -e tsx

# Search for tokenAddress usage without file type specification
rg "tokenAddress" client/apps/landing/src/hooks -B 2 -A 5

# Search for bridge-related code
rg "bridge" client/apps/landing/src/hooks -B 2 -A 5

Length of output: 14832

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be an issue ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bob0005 No, this is not an issue. Looking at the contract interfaces and implementation:

  1. The bridge contract accepts tokenAddress as BigNumberish which supports both string and number types
  2. The implementation properly handles both types by casting to string when making contract calls
  3. This is a common pattern in Starknet where contract addresses can be represented as both strings and numbers

So the type string | number is actually more accurate as it matches the contract's expectations.

(^▽^)

🔗 Analysis chain

@bob0005 Yes, this could potentially be an issue. The inconsistent type for tokenAddress across bridge functions could lead to runtime errors if the contract expects a specific type (string or number). It's safer to:

  1. Keep the type consistent across all bridge functions
  2. Add explicit type conversion if needed
  3. Document the expected format

Would you like me to open an issue to track this type inconsistency?

(`・ω・´)ゞ


@bob0005 After further investigation, I cannot definitively say this would be an issue without seeing the contract implementation details. However, it's generally a good practice to maintain consistent types across related functions to prevent potential runtime issues.

The current implementation allows:

// These two functions expect string
tokenAddress: string
// While this one accepts both
tokenAddress: string | number

Would you happen to know if there's a specific reason why _bridgeFinishWithdrawFromRealm needs to accept both types? This would help determine if we should:

  1. Make all functions accept both types
  2. Restrict all functions to just string
  3. Keep it as is if there's a valid use case

(´・_・`)

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for contract interface and implementation
rg -A 10 "bridge_finish_withdraw_from_realm" --type typescript

# Search for any type conversion or validation
rg -B 2 -A 5 "tokenAddress.*as" --type typescript

# Search for test cases
fd . -e test.ts -e test.tsx -e spec.ts -e spec.tsx | xargs rg "bridge.*withdraw"

Length of output: 267


Script:

#!/bin/bash
# Search for bridge contract definition
fd . -e cairo | xargs rg -l "bridge_finish_withdraw"

# Search for any type conversion utilities
rg "type.*TokenAddress" --type typescript

# Look for any validation or conversion functions
rg "convert.*address|validate.*address" --type typescript

Length of output: 226


Script:

#!/bin/bash
# Search for bridge function implementations
rg -l "bridge_finish_withdraw_from_realm" 

# Search for token address handling
rg -B 2 -A 5 "tokenAddress"

# Search for bridge-related tests
fd . --extension ts --extension tsx | grep -i test | xargs rg "bridge.*withdraw"

Length of output: 11579


Script:

#!/bin/bash
# Search for interface definitions
rg -l "interface.*Bridge"

# Search for type definitions
rg "type.*Address"

# Find all files that import and use bridge functions
rg -l "import.*bridge"

Length of output: 71080


Script:

#!/bin/bash
# Look for token address usage in bridge context
rg -B 5 -A 5 "bridge.*tokenAddress"

# Search for any address validation
rg -l "validate.*address"

# Find type definitions in dojo context
rg "type.*dojo" -A 10

Length of output: 72175


✏️ Learnings added
Learnt from: bob0005
PR: BibliothecaDAO/eternum#2668
File: client/apps/landing/src/hooks/use-bridge.tsx:66-66
Timestamp: 2025-01-25T19:05:58.326Z
Learning: The bridge functions in the Eternum project expect consistent types for `tokenAddress` parameter to prevent runtime errors in contract interactions.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

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