knip #2635
knip #2635
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
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. |
|
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
client/apps/balancing/src/components/ui/badge.tsxOops! Something went wrong! :( ESLint: 9.17.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. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by WalkthroughThis pull request introduces a comprehensive set of changes across multiple files in the client applications, primarily focusing on code organization, type visibility, and module restructuring. The modifications include removing exported entities, changing import paths, deleting entire files, and simplifying the codebase. The changes appear to be part of a broader effort to streamline the project's structure, reduce complexity, and potentially prepare for future architectural improvements. Changes
Poem
Finishing Touches
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 (
|
|
Failed to generate code suggestions for PR |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (3)
client/apps/landing/src/lib/utils.ts (1)
Line range hint
11-14: Handle edge cases for shorter addressesIf the input string is shorter than expected (less than 10 characters), the function may produce incorrect results or throw an error. Consider adding a check to handle such cases.
Apply this diff to fix the issue:
export function displayAddress(string: string) { if (string === undefined) return "unknown"; + if (string.length <= 10) return string; return string.substring(0, 6) + "..." + string.substring(string.length - 4); }client/apps/landing/src/components/modules/bridge-out-step-2.tsx (2)
Line range hint
41-49: Remove commented out code.The commented out
useMemohooks fordonkeysArrivalsanddonkeyArrivalsEntityIdsare no longer used. Clean up the codebase by removing these comments.- /*const donkeysArrivals = useMemo(() => { - if (bankPosition) { - return getOwnerArrivalsAtBank(realmEntityIds as number[]); - } - }, [realmEntityIds, refreshTrigger, bankPosition]);*/ - - /*const donkeyArrivalsEntityIds = useMemo(() => { - return donkeyArrivals?.map((entity) => { - const position = getComponentValue(dojo.setup.components.Position, entity); - return position?.entity_id; - }); - }, [donkeyArrivals]) as number[];*/
Line range hint
51-51: Remove commented out hook call.The commented out
useSyncEntityhook call is no longer needed.- //useSyncEntity(donkeyArrivalsEntityIds);
🧹 Nitpick comments (5)
client/apps/game/src/dojo/setup.ts (2)
Line range hint
31-35: Fix misleading comment about debounce time.The comment states "Increased debounce time to 1 second" but the actual value is set to 200ms. Please update the comment to reflect the actual debounce time.
- }, 200); // Increased debounce time to 1 second for larger batches + }, 200); // Debounce time set to 200ms for batching updates
Line range hint
201-201: Consider filtering entities for better performance.The
syncEntitiesDebouncedis called with an empty array forentityKeyClause, which means it's syncing all entities without filtering. For large-scale deployments, this could lead to performance issues.Consider adding appropriate entity key clauses to filter and sync only the required entities, similar to how specific models are filtered in the
getEventscalls below.client/apps/landing/src/lib/utils.ts (1)
7-9: Consider usingIntl.NumberFormatfor locale-aware number formattingThe current implementation uses a regular expression to add thousand separators. Utilizing
Intl.NumberFormatcan provide better locale support and simplify the code.Apply this diff to refactor the function:
-export const formatNumber = (num: number, decimals: number): string => { - return num.toFixed(decimals).replace(/(\d)(?=(\d{3})+(?!\d))/g, "$1,"); -}; +export const formatNumber = (num: number, decimals: number): string => { + return new Intl.NumberFormat(undefined, { + minimumFractionDigits: decimals, + maximumFractionDigits: decimals, + }).format(num); +};client/apps/game/src/hooks/useUISound.tsx (2)
Line range hint
134-136: Remove unnecessary eslint disable comment.The switch statement is actually quite large, so the
sonarjs/no-small-switchdisable comment appears to be outdated and can be removed.
Line range hint
134-220: Consider refactoring the resource sound mapping.The large switch statement could be replaced with a more maintainable object mapping.
Here's a suggested refactor:
- // eslint-disable-next-line sonarjs/no-small-switch - const playResourceSound = (resourceId: ResourcesIds) => { - switch (resourceId) { - case ResourcesIds.Fish: - playFishingVillage(); - break; - // ... many more cases - } - }; + const resourceSoundMap = { + [ResourcesIds.Fish]: playFishingVillage, + [ResourcesIds.Wheat]: playFarm, + [ResourcesIds.Wood]: playAddWood, + // ... map remaining resources + } as const; + + const playResourceSound = (resourceId: ResourcesIds) => { + resourceSoundMap[resourceId]?.(); + };This approach:
- Eliminates the need for a switch statement
- Reduces code duplication
- Makes it easier to maintain and update
- Provides better type safety
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (73)
.knip.json(1 hunks)client/.knip.json(0 hunks)client/apps/balancing/src/components/ui/badge.tsx(2 hunks)client/apps/balancing/src/components/ui/table.tsx(1 hunks)client/apps/game/.eslintrc.json(0 hunks)client/apps/game/pwa-assets.config.ts(0 hunks)client/apps/game/src/dojo/contractComponents.ts(0 hunks)client/apps/game/src/dojo/debouncedQueries.ts(0 hunks)client/apps/game/src/dojo/queries.ts(0 hunks)client/apps/game/src/dojo/setup.ts(1 hunks)client/apps/game/src/dojo/worker.ts(0 hunks)client/apps/game/src/hooks/context/mainnet-policies.tsx(0 hunks)client/apps/game/src/hooks/helpers/useQuests.tsx(1 hunks)client/apps/game/src/hooks/store/useQuestStore.tsx(0 hunks)client/apps/game/src/hooks/store/useWorldLoading.tsx(1 hunks)client/apps/game/src/hooks/useUISound.tsx(1 hunks)client/apps/game/src/three/components/FogManager.ts(0 hunks)client/apps/game/src/three/scenes/constants.ts(1 hunks)client/apps/game/src/ui/components/resources/realm-transfer.tsx(1 hunks)client/apps/game/src/ui/elements/Switch.tsx(0 hunks)client/apps/game/src/ui/utils/utils.tsx(1 hunks)client/apps/landing/env.ts(0 hunks)client/apps/landing/src/components/modules/bridge-out-step-1.tsx(1 hunks)client/apps/landing/src/components/modules/bridge-out-step-2.tsx(1 hunks)client/apps/landing/src/components/modules/bridged-resources.tsx(1 hunks)client/apps/landing/src/components/modules/cartridge-connect-button.tsx(0 hunks)client/apps/landing/src/components/modules/filters.tsx(0 hunks)client/apps/landing/src/components/modules/guild-leaderboard-panel.tsx(0 hunks)client/apps/landing/src/components/modules/leaderboard.tsx(0 hunks)client/apps/landing/src/components/modules/realm-card.tsx(1 hunks)client/apps/landing/src/components/ui/command.tsx(0 hunks)client/apps/landing/src/components/ui/custom-iframe.tsx(0 hunks)client/apps/landing/src/components/ui/utils/utils.ts(1 hunks)client/apps/landing/src/config.ts(2 hunks)client/apps/landing/src/dojo/contractComponents.ts(1 hunks)client/apps/landing/src/dojo/createClientComponents.ts(1 hunks)client/apps/landing/src/dojo/queries.ts(0 hunks)client/apps/landing/src/hooks/context/DojoContext.tsx(1 hunks)client/apps/landing/src/hooks/get-guilds.tsx(0 hunks)client/apps/landing/src/hooks/gql/fragment-masking.ts(1 hunks)client/apps/landing/src/hooks/helpers/use-contributions.tsx(0 hunks)client/apps/landing/src/hooks/helpers/useEntities.tsx(1 hunks)client/apps/landing/src/hooks/query/leaderboard.tsx(0 hunks)client/apps/landing/src/hooks/query/players.tsx(0 hunks)client/apps/landing/src/hooks/use-entities-utils.tsx(0 hunks)client/apps/landing/src/hooks/use-get-all-players.tsx(0 hunks)client/apps/landing/src/hooks/use-lords-bridged.tsx(0 hunks)client/apps/landing/src/hooks/use-realms-settled.tsx(0 hunks)client/apps/landing/src/hooks/useCollectionTokens.tsx(0 hunks)client/apps/landing/src/hooks/usePrizeClaim.tsx(0 hunks)client/apps/landing/src/lib/ark/getCollectionTokens.ts(0 hunks)client/apps/landing/src/lib/utils.ts(1 hunks)client/apps/landing/src/types/index.ts(0 hunks)client/apps/landing/src/utils/leaderboard.tsx(0 hunks)client/apps/landing/tsconfig.app.tsbuildinfo(1 hunks)client/config/speed.ts(0 hunks)client/sdk/packages/eternum/src/dojo/index.ts(1 hunks)client/sdk/packages/eternum/src/index.ts(1 hunks)client/sdk/packages/eternum/src/modelManager/ArmyManager.ts(1 hunks)client/sdk/packages/eternum/src/modelManager/ArmyMovementManager.ts(1 hunks)client/sdk/packages/eternum/src/modelManager/BattleManager.ts(1 hunks)client/sdk/packages/eternum/src/modelManager/ConfigManager.ts(1 hunks)client/sdk/packages/eternum/src/modelManager/LeaderboardManager.ts(2 hunks)client/sdk/packages/eternum/src/modelManager/MarketManager.ts(1 hunks)client/sdk/packages/eternum/src/modelManager/ResourceInventoryManager.ts(1 hunks)client/sdk/packages/eternum/src/modelManager/ResourceManager.ts(1 hunks)client/sdk/packages/eternum/src/modelManager/StaminaManager.ts(1 hunks)client/sdk/packages/eternum/src/modelManager/TileManager.ts(1 hunks)client/sdk/packages/eternum/src/modelManager/types/index.ts(1 hunks)client/sdk/packages/eternum/src/modelManager/utils/ArmyMovementUtils.ts(1 hunks)client/sdk/packages/eternum/src/modelManager/utils/LeaderboardUtils.ts(1 hunks)client/sdk/packages/eternum/src/scripts/extract.ts(0 hunks)client/sdk/packages/eternum/src/scripts/extractBuildings.ts(0 hunks)
💤 Files with no reviewable changes (35)
- client/apps/landing/src/hooks/use-realms-settled.tsx
- client/config/speed.ts
- client/apps/landing/env.ts
- client/apps/landing/src/hooks/query/players.tsx
- client/.knip.json
- client/apps/landing/src/hooks/useCollectionTokens.tsx
- client/apps/game/src/hooks/context/mainnet-policies.tsx
- client/apps/game/.eslintrc.json
- client/apps/landing/src/components/modules/guild-leaderboard-panel.tsx
- client/apps/game/src/dojo/contractComponents.ts
- client/apps/landing/src/hooks/query/leaderboard.tsx
- client/apps/landing/src/components/modules/filters.tsx
- client/sdk/packages/eternum/src/scripts/extract.ts
- client/apps/landing/src/hooks/use-entities-utils.tsx
- client/apps/landing/src/components/ui/custom-iframe.tsx
- client/apps/landing/src/hooks/use-lords-bridged.tsx
- client/apps/landing/src/hooks/get-guilds.tsx
- client/apps/landing/src/hooks/usePrizeClaim.tsx
- client/apps/landing/src/components/modules/leaderboard.tsx
- client/apps/game/pwa-assets.config.ts
- client/apps/game/src/three/components/FogManager.ts
- client/apps/landing/src/hooks/helpers/use-contributions.tsx
- client/apps/game/src/ui/elements/Switch.tsx
- client/sdk/packages/eternum/src/scripts/extractBuildings.ts
- client/apps/landing/src/dojo/queries.ts
- client/apps/game/src/dojo/worker.ts
- client/apps/landing/src/components/modules/cartridge-connect-button.tsx
- client/apps/landing/src/hooks/use-get-all-players.tsx
- client/apps/game/src/hooks/store/useQuestStore.tsx
- client/apps/landing/src/utils/leaderboard.tsx
- client/apps/game/src/dojo/debouncedQueries.ts
- client/apps/game/src/dojo/queries.ts
- client/apps/landing/src/components/ui/command.tsx
- client/apps/landing/src/lib/ark/getCollectionTokens.ts
- client/apps/landing/src/types/index.ts
✅ Files skipped from review due to trivial changes (19)
- client/sdk/packages/eternum/src/modelManager/LeaderboardManager.ts
- client/apps/landing/src/components/modules/realm-card.tsx
- client/sdk/packages/eternum/src/modelManager/TileManager.ts
- client/sdk/packages/eternum/src/modelManager/utils/ArmyMovementUtils.ts
- client/apps/game/src/ui/components/resources/realm-transfer.tsx
- client/sdk/packages/eternum/src/modelManager/utils/LeaderboardUtils.ts
- client/sdk/packages/eternum/src/modelManager/ResourceInventoryManager.ts
- client/sdk/packages/eternum/src/modelManager/ConfigManager.ts
- client/sdk/packages/eternum/src/modelManager/ArmyMovementManager.ts
- client/apps/landing/tsconfig.app.tsbuildinfo
- client/sdk/packages/eternum/src/modelManager/BattleManager.ts
- client/sdk/packages/eternum/src/modelManager/ResourceManager.ts
- client/sdk/packages/eternum/src/dojo/index.ts
- client/sdk/packages/eternum/src/modelManager/StaminaManager.ts
- client/sdk/packages/eternum/src/modelManager/MarketManager.ts
- client/sdk/packages/eternum/src/modelManager/ArmyManager.ts
- .knip.json
- client/apps/landing/src/hooks/gql/fragment-masking.ts
- client/sdk/packages/eternum/src/modelManager/types/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (17)
- GitHub Check: test-various (test_ownership_systems)
- GitHub Check: test-various (transport_test)
- GitHub Check: test-various (guild_test)
- GitHub Check: test-various (realm_test)
- GitHub Check: test-various (trade_test)
- GitHub Check: test-various (resource_bridge_test)
- GitHub Check: test-various (map_test)
- GitHub Check: test-various (bank_test)
- GitHub Check: test-various (hyperstructure_test)
- GitHub Check: test-various (config_test)
- GitHub Check: test-various (resources_test)
- GitHub Check: test-various (position_test)
- GitHub Check: test-various (owner_test)
- GitHub Check: test-various (combat_test)
- GitHub Check: test-various (season_test)
- GitHub Check: clippy
- GitHub Check: Run pr agent on every pull request, respond to user comments
🔇 Additional comments (18)
client/sdk/packages/eternum/src/index.ts (1)
3-3: LGTM! The new dojo module export follows the established pattern.The addition of
export * from "./dojo"maintains consistency with the existing barrel file structure and aligns with the module organization pattern used throughout the file.Let's verify the dojo module's existence and structure:
✅ Verification successful
The dojo module export is valid and properly structured
The module exists at the correct location and follows the established barrel file pattern, re-exporting from
contractComponentsandcreateClientComponentssub-modules.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the dojo module structure and its exports # Check if the dojo module exists fd -t f "index.ts" client/sdk/packages/eternum/src/dojo # Examine the dojo module's exports rg -t ts "export \*" client/sdk/packages/eternum/src/dojoLength of output: 333
client/apps/game/src/dojo/setup.ts (1)
22-22: LGTM! Good encapsulation of internal functionality.The removal of the export declaration is a positive change that properly encapsulates this utility function within its module, as it's only used internally by the
setupfunction.client/apps/landing/src/lib/utils.ts (1)
1-6: Code changes look goodThe imports and functions
cn,multiplyByPrecision, anddivideByPrecisionare correctly implemented and adhere to best practices.Also applies to: 15-20
client/apps/landing/src/dojo/createClientComponents.ts (1)
4-4: LGTM! Safe removal of unnecessary type export.The
ClientComponentstype is correctly scoped as internal since it's only used within this file. External consumers can still derive the type usingtypeof createClientComponentsif needed.client/apps/balancing/src/components/ui/badge.tsx (1)
23-23: LGTM! Improved component encapsulation.Good cleanup of the public API surface. Only the
Badgecomponent needs to be exported, whileBadgePropsandbadgeVariantsare correctly kept as internal implementation details.Also applies to: 29-29
client/apps/game/src/hooks/store/useWorldLoading.tsx (1)
20-22: LGTM! Appropriate scoping of internal type.The
LoadingStatetype is correctly made internal since it's just an implementation detail. External consumers have all they need with the exportedLoadingStateKeyenum andWorldStoreinterface.client/apps/landing/src/config.ts (1)
3-7: Verify removal of configuration exports.While making these configuration entities internal improves encapsulation, please verify that:
- No other modules need the
ChainorTokenenums for type checking- The
tokensandchainconstants aren't needed for configuration elsewhere- Exporting only the derived addresses is sufficient for all use cases
Also applies to: 9-13, 15-67, 68-68
client/apps/landing/src/hooks/helpers/useEntities.tsx (1)
8-19: LGTM! Good encapsulation of internal type guards.The removal of exports for
S0EternumRealminterface and type guard functions improves encapsulation since these are only used internally within the module.client/apps/balancing/src/components/ui/table.tsx (1)
76-76: Verify removal of TableFooter and TableCaption exports.Let's confirm that these components aren't used elsewhere in the codebase.
✅ Verification successful
Removal of TableFooter and TableCaption exports is safe
These components are not being used anywhere in the codebase. No JSX usage or imports were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any usage of TableFooter or TableCaption components echo "Searching for TableFooter usage..." rg "TableFooter" --type tsx --type jsx echo "Searching for TableCaption usage..." rg "TableCaption" --type tsx --type jsxLength of output: 294
Script:
#!/bin/bash # Search for any usage or imports of TableFooter or TableCaption echo "Searching for TableFooter..." rg "TableFooter" -g "*.{tsx,jsx,ts,js}" -B 1 -A 1 echo -e "\nSearching for TableCaption..." rg "TableCaption" -g "*.{tsx,jsx,ts,js}" -B 1 -A 1Length of output: 2568
Script:
#!/bin/bash # Search for actual usage of TableFooter and TableCaption in JSX and imports echo "Searching for TableFooter usage..." rg "<TableFooter" -g "*.{tsx,jsx,ts,js}" -B 1 -A 1 rg "import.*TableFooter" -g "*.{tsx,jsx,ts,js}" -B 1 -A 1 echo -e "\nSearching for TableCaption usage..." rg "<TableCaption" -g "*.{tsx,jsx,ts,js}" -B 1 -A 1 rg "import.*TableCaption" -g "*.{tsx,jsx,ts,js}" -B 1 -A 1Length of output: 393
client/apps/landing/src/hooks/context/DojoContext.tsx (1)
26-33: LGTM! Proper encapsulation of React Context internals.Good practice to only expose the provider and hook while keeping the context and types internal. This follows React's context pattern and reduces the public API surface.
client/apps/game/src/hooks/useUISound.tsx (1)
1-2: LGTM! Clean imports.The imports are minimal and focused on the required functionality.
client/apps/landing/src/components/modules/bridged-resources.tsx (1)
107-107: LGTM! Good encapsulation.Making
BridgeResourcea local component is a good choice since it's only used within this module. This change improves encapsulation and reduces the public API surface.client/apps/game/src/hooks/helpers/useQuests.tsx (1)
176-176: LGTM! Good encapsulation.Making
useQuestClaimStatusa local hook is appropriate since it's only used within this module. This change improves encapsulation and reduces the public API surface.client/apps/game/src/three/scenes/constants.ts (1)
80-80: LGTM! Good encapsulation.Making
BiomeFilenamesa local enum is appropriate since it's only used within this module bybiomeModelPaths. This change improves encapsulation and reduces the public API surface.client/apps/landing/src/components/ui/utils/utils.ts (1)
376-376: LGTM! Good encapsulation.Making
ResourceAddressesa local interface is appropriate since it's only used within this module bygetSeasonAddresses. This change improves encapsulation and reduces the public API surface.client/apps/landing/src/components/modules/bridge-out-step-1.tsx (1)
Line range hint
318-367: LGTM! Good architectural improvement.Converting
SelectResourceRowto a locally scoped component improves encapsulation. This component is only used within this file, so removing its export reduces the public API surface area.client/apps/game/src/ui/utils/utils.tsx (1)
444-450: LGTM! Good encapsulation of internal utilities.Making
getSeasonAddressesPathandgetJSONFileprivate functions improves encapsulation. These functions are implementation details ofgetSeasonAddressesand don't need to be exposed.client/apps/landing/src/dojo/contractComponents.ts (1)
5-5: LGTM! Good type encapsulation.Making
ContractComponentsa private type improves encapsulation. This type appears to be an implementation detail of the contract components system.
Summary by CodeRabbit
Here are the release notes for the pull request:
Configuration
.knip.jsonto specify file and directory exclusion patternsCode Cleanup
Module Restructuring
Component Visibility
These changes appear to be focused on code maintenance, reducing complexity, and improving the overall project structure.