Knip #3925
Knip #3925
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Skipped Deployments
|
WalkthroughThe pull request introduces various modifications across multiple files, primarily focusing on changing the visibility of types, interfaces, and constants from exported to local scope. Key changes include the addition and removal of certain imports, updates to function signatures, and the refactoring of components to enhance encapsulation. Additionally, some components and functions have been removed entirely, and specific logic has been refined, particularly in relation to handling feature flags and rendering conditions. Changes
Possibly related PRs
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: 9
🧹 Outside diff range and nitpick comments (74)
packages/web/components/ui/input.tsx (1)
11-14: Consider extracting base input styles to a constant.The className string is quite long and contains multiple styling rules. Consider extracting it to a constant for better maintainability and reusability.
+const baseInputStyles = "placeholder:text-muted-foreground focus-visible:ring-ring flex h-9 w-full rounded-md border border-osmoverse-600 bg-transparent px-3 py-1 text-sm shadow-sm transition-colors file:border-0 file:bg-transparent file:text-sm file:font-medium focus-visible:outline-none focus-visible:ring-1 disabled:cursor-not-allowed disabled:opacity-50"; const Input = React.forwardRef<HTMLInputElement, React.InputHTMLAttributes<HTMLInputElement>>( ({ className, type, ...props }, ref) => { return ( <input type={type} - className={classNames( - "placeholder:text-muted-foreground focus-visible:ring-ring flex h-9 w-full rounded-md border border-osmoverse-600 bg-transparent px-3 py-1 text-sm shadow-sm transition-colors file:border-0 file:bg-transparent file:text-sm file:font-medium focus-visible:outline-none focus-visible:ring-1 disabled:cursor-not-allowed disabled:opacity-50", - className - )} + className={classNames(baseInputStyles, className)} ref={ref} {...props} /> ); } );packages/web/hooks/use-disclosure.ts (1)
Line range hint
8-37: Consider adding test coverage for the hook.While the implementation is solid, it would be beneficial to add unit tests to verify the behavior of
onOpen,onClose, andonTogglefunctions, as well as thedefaultIsOpenprop.Would you like me to help create a test file with comprehensive test cases for this hook?
packages/web/hooks/user-settings/use-hide-dust-filter.ts (1)
1-5: LGTM! Good architectural improvementMoving the dust threshold to a centralized constant in HideDustUserSetting improves maintainability and consistency across the application.
packages/web/components/complex/portfolio/cypher-card.tsx (2)
11-13: Consider extracting the storage key as a constant.The storage key could be defined as a named constant alongside
CYPHER_CARD_URLfor better maintainability and reusability.export const CYPHER_CARD_URL = "https://pay.osmosis.zone?ref=osmosis"; +export const CYPHER_CARD_CLOSED_KEY = "cypher-card-closed"; export const CypherCard = () => { const { t } = useTranslation(); - const [isClosed, setIsClosed] = useLocalStorage("cypher-card-closed", false); + const [isClosed, setIsClosed] = useLocalStorage(CYPHER_CARD_CLOSED_KEY, false);
34-42: Enhance button accessibility.While the close button implementation is good, consider these accessibility improvements:
- Add
aria-labelfor screen readers- Consider adding a tooltip to explain the action
- Verify color contrast ratio between the icon and background
<button onClick={(e) => { e.preventDefault(); setIsClosed(true); }} - className="absolute right-2 top-1/2 -translate-y-1/2 p-1 text-osmoverse-300 hover:text-white-full" + className="absolute right-2 top-1/2 -translate-y-1/2 p-1 text-osmoverse-300 hover:text-white-full" + aria-label={t("cypherCard.close")} + title={t("cypherCard.close")} > <Icon id="close" width={16} height={16} /> </button>packages/web/components/complex/portfolio/allocation-tabs.tsx (1)
Line range hint
41-63: Consider enhancing accessibility attributes.The tab implementation could benefit from additional ARIA attributes to improve accessibility.
Consider adding these attributes to the tab buttons:
<button key={`swap-tab-${tab.value}`} onClick={() => setTab(tab.value)} + role="tab" + aria-selected={isActive} + aria-controls={`panel-${tab.value}`} className={classNames( "h-full w-1/3 rounded-3xl transition-colors", {Also, consider wrapping the tabs in a container with appropriate ARIA roles:
-<div className="flex h-8 w-full items-center rounded-3xl border border-osmoverse-700"> +<div + role="tablist" + aria-label={t("portfolio.allocation")} + className="flex h-8 w-full items-center rounded-3xl border border-osmoverse-700">packages/web/hooks/mutations/one-click-trading/use-remove-one-click-trading-session.ts (1)
Line range hint
26-71: Consider enhancing error handling and maintainability.While the implementation is solid, consider these improvements:
- Make the error message more specific to help with debugging:
- throw new Error("Osmosis account not found"); + throw new Error("Cannot remove one-click trading session: Osmosis account not connected");
- Add more granular transaction error handling:
- reject(new Error("Transaction failed")); + reject(new Error(`Transaction failed with code ${tx.code}: ${tx.rawLog}`));
- Consider moving the analytics event name to a centralized constants file:
+// In a constants file +export const ANALYTICS_EVENTS = { + ONE_CLICK_TRADING: { + END_SESSION: 'one_click_trading_end_session' + } +} as const; // In this file - logEvent([EventName.OneClickTrading.endSession]); + logEvent([ANALYTICS_EVENTS.ONE_CLICK_TRADING.END_SESSION]);packages/web/components/earn/filters/filter-context.tsx (2)
Line range hint
27-31: Remove redundant type inSetFilterFndefinition.The type union
ListOption<string> | ListOption<string>contains a redundant type. This can be simplified.Apply this diff to fix the type definition:
type SetFilterFn = ( key: keyof Filters, - value: string | boolean | ListOption<string> | ListOption<string> + value: string | boolean | ListOption<string> ) => void;
Line range hint
47-106: Consider these improvements to the filter state management.While the implementation is solid, here are some suggestions to enhance type safety and documentation:
- Type the multi-option actions array more strictly:
-const MULTI_OPTION_ACTIONS: MultiAction[] = [ +const MULTI_OPTION_ACTIONS: readonly MultiAction[] = [ "specialTokens", "strategyMethod", "platform", ] as const;
- Convert the comment block to JSDoc for better IDE support:
- /** - * This code chooses what new array - * to return based on the existence of the incoming filter value. - * - * If the incoming value is already present in the state value, - * then remove it. If it doesn't, then add it. - */ + /** + * Updates the filter array based on the existence of the incoming value. + * @description If the value exists, it's removed. If it doesn't exist, it's added. + * This implements a toggle behavior for multi-select filters. + */
- Make the filter value type narrowing more explicit:
- const filterValue = value as ListOption<string>; + if (!isListOption(value)) { + console.warn('Expected ListOption<string> for multi-option filter'); + return; + } + const filterValue = value;Add this type guard:
function isListOption(value: unknown): value is ListOption<string> { return typeof value === 'object' && value !== null && 'value' in value; }packages/web/components/ad-banner/ad-banner.tsx (2)
Line range hint
63-71: Enhance accessibility for interactive elements.While the component correctly switches between button and anchor elements, consider adding appropriate ARIA labels for better accessibility, especially for screen readers.
Consider applying these changes:
<Component className="z-50 flex w-full items-center gap-5 rounded-3xl px-4 py-3 hover:cursor-pointer" style={gradientStyle} target="_blank" rel="noopener noreferrer" + aria-label={getDeepValue(currentLocalization, headerOrTranslationKey) ?? headerOrTranslationKey} {...(isButton ? { onClick } : { href: externalUrl })} >
Line range hint
72-81: Consider making image dimensions responsive.The Image component uses fixed dimensions (64x72), which might not be optimal for all screen sizes. Consider using responsive dimensions or aspect ratio preservation.
Consider applying these changes:
<Image src={iconImageUrl} alt={ getDeepValue(currentLocalization, iconImageAltOrTranslationKey) ?? iconImageAltOrTranslationKey ?? "" } - width={64} - height={72} + width={0} + height={0} + sizes="100vw" + className="h-[72px] w-[64px] object-contain" className="object-contain" />packages/web/components/control/language-select.tsx (1)
Line range hint
15-106: Consider performance and error handling improvements.While the component's architecture is solid, here are some suggestions for improvement:
- Memoize the options lookup to prevent unnecessary recalculations
- Consider error boundaries for store access
- Simplify the useEffect in ListBoxContent
Here's how you could implement these improvements:
export const LanguageSelect: FunctionComponent<LanguageSelectProps> = observer( (props) => { const { options } = props; const { t } = useTranslation(); const { userSettings } = useStore(); + + // Memoize the current option lookup + const currentOption = React.useMemo(() => { const languageSetting = userSettings.getUserSettingById( "language" ) as LanguageUserSetting; const currentLanguage = languageSetting?.state.language; - const currentOption = options.find( + return options.find( (option) => option.value === currentLanguage ); - return currentOption; + }, [userSettings, options]); + + // Handle potential store access errors + if (!userSettings) { + return <div>Error: Unable to access user settings</div>; + }For the ListBoxContent component:
const ListBoxContent: FunctionComponent< LanguageSelectProps & { open: boolean; languageSetting: LanguageUserSetting; } > = observer(({ open, options, languageSetting }) => { - useEffect(() => { - languageSetting.setState({ isControlOpen: open }); - }, [languageSetting, open]); + // Directly set the state when open changes + languageSetting.setState({ isControlOpen: open });packages/web/components/complex/pool/create/select-type.tsx (2)
Line range hint
89-93: Consider extracting image dimensions to constants.The hardcoded dimensions (64x64) should be moved to a constant for better maintainability.
+const POOL_TYPE_ICON_SIZE = 64; + {iconId ? ( - <Icon id={iconId} width={64} height={64} /> + <Icon id={iconId} width={POOL_TYPE_ICON_SIZE} height={POOL_TYPE_ICON_SIZE} /> ) : ( <Image src={imageSrc} alt={caption} - height={64} - width={64} + height={POOL_TYPE_ICON_SIZE} + width={POOL_TYPE_ICON_SIZE} /> )}
Line range hint
108-120: Add error handling for test network pool creation.The test network pool creation lacks error handling and uses hardcoded values. Consider adding try-catch and extracting test values to constants.
+const TEST_POOL_CONFIG = { + token1: "uion", + token2: "uosmo", + tickSpacing: 100, + spreadFactor: 0.0001, +}; + {IS_TESTNET && ( <div> <Button onClick={() => { - account?.osmosis.sendCreateConcentratedPoolMsg( - "uion", - "uosmo", - 100, - 0.0001 - ); + try { + await account?.osmosis.sendCreateConcentratedPoolMsg( + TEST_POOL_CONFIG.token1, + TEST_POOL_CONFIG.token2, + TEST_POOL_CONFIG.tickSpacing, + TEST_POOL_CONFIG.spreadFactor + ); + } catch (error) { + console.error("Failed to create test pool:", error); + // TODO: Add user notification of failure + } }} >packages/web/__tests__/test-utils.tsx (1)
Line range hint
1-146: Consider additional test utility improvementsWhile the current changes improve encapsulation, consider these additional enhancements:
- Add JSDoc comments for the exported functions to document their usage
- Consider grouping related test utilities (e.g., wallet-related functions) into separate files for better organization
- Add error handling for edge cases in
connectTestWalletThese improvements would further enhance maintainability and developer experience.
packages/web/components/alert/cypher-card-toast.tsx (2)
Line range hint
18-116: Consider enhancing the toast notification implementationWhile the implementation is functional, there are several opportunities for improvement:
- The click handlers could be extracted to separate functions for better readability
- Consider adding an auto-dismiss feature after a certain duration
- Add error handling for localStorage operations
- Consider using an established toast library like
react-hot-toastorreact-toastifyfor better accessibility and consistent behaviorHere's a suggested refactor for the click handlers and localStorage error handling:
export function CypherCardToast() { const { t } = useTranslation(); - const [doNotShowAgain, setDoNotShowAgain] = useLocalStorage( - CypherCardFloatingBannerDoNotShowKey, - false - ); + const [doNotShowAgain, setDoNotShowAgain] = useLocalStorage( + CypherCardFloatingBannerDoNotShowKey, + false, + { + onError: (error) => { + console.error('Failed to access localStorage:', error); + // Fallback to showing the toast when localStorage fails + return false; + } + } + ); + + const handleClose = (e: React.MouseEvent) => { + e.stopPropagation(); + setDoNotShowAgain(true); + }; + + const handleLearnMore = (e: React.MouseEvent) => { + e.stopPropagation(); + setDoNotShowAgain(true); + };Then update the event handlers:
- onClick={(e) => { - e.stopPropagation(); - onClose(); - }} + onClick={handleLearnMore} - onClick={(e) => { - e.stopPropagation(); - onClose(); - }} + onClick={handleClose}
Toast notification needs ARIA improvements for better accessibility
The current implementation lacks proper ARIA attributes for toast notifications. Add the following attributes:
- Add
role="alert"orrole="status"to the toast container- Add
aria-live="polite"for non-critical notifications oraria-live="assertive"for important alerts- Ensure toast content is properly labeled with
aria-labeloraria-labelledbyExample locations needing updates:
packages/web/components/alert/cypher-card-toast.tsxpackages/web/components/alert/toast.tsxpackages/web/components/alert/limit-orders-toast.tsx🔗 Analysis chain
Line range hint
18-116: Verify accessibility complianceThe toast notification should follow ARIA best practices for notifications.
Let's check if other toast notifications in the codebase follow similar patterns:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other toast implementations to ensure consistency rg -t tsx -t jsx "toast|notification|alert|banner" --glob "!{node_modules,dist}/**"Length of output: 113
Script:
#!/bin/bash # Search for other toast/notification implementations with proper file extension rg -t ts -t js -t jsx "toast|notification|alert|banner" --glob "!{node_modules,dist}/**" # Also search for ARIA attributes in the codebase rg -t ts -t js -t jsx "aria-|role=" --glob "!{node_modules,dist}/**"Length of output: 217
Script:
#!/bin/bash # Search for toast/notification implementations and ARIA attributes without file type restrictions rg "toast|notification|alert|banner" --type-not md --type-not json --glob "!{node_modules,dist,build}/**" # Search specifically in the components directory rg "aria-|role=" "./packages/web/components" --type-not md --type-not jsonLength of output: 67500
packages/web/components/complex/portfolio/allocation.tsx (2)
75-84: LGTM: Good performance optimization with room for improvementThe use of
useMemoforselectedListand the implementation ofuseHideDustUserSettingare well done. Consider memoizingdustFilteredListas well since it's derived fromselectedListand used in multiple render sections.- const dustFilteredList = useHideDustUserSetting( - selectedList, - (item) => item.fiatValue - ); + const dustFilteredList = useMemo( + () => useHideDustUserSetting(selectedList, (item) => item.fiatValue), + [selectedList] + );
118-135: LGTM: Good handling of edge casesThe rendering logic handles negligible percentages well and maintains consistent width calculations. Consider extracting the repeated colorClass calculation into a utility function since it's used in both render sections.
+ const getColorClass = (index: number, option: AllocationOptions) => + COLORS[option][index % COLORS[option].length]; // Then in both map functions: - const colorClass = COLORS[selectedOption][index % COLORS[selectedOption].length]; + const colorClass = getColorClass(index, selectedOption);Also applies to: 138-138
packages/web/components/earn/table/columns.tsx (2)
Line range hint
31-65: Consider enhancing component documentation and prop types.The component is well-structured with proper TypeScript types, but could benefit from some refinements:
- Consider grouping related props using a dedicated type:
type TooltipProps = { tooltipDescription?: string; tooltipClassname?: string; }; type ColumnCellHeaderProps = PropsWithChildren< TooltipProps & { tKey: TranslationPath; className?: string; } >;
- Use consistent naming for className props (either
classNameorclassname).
Line range hint
108-117: Remove or restore commented code.There's commented-out code in the
nameaccessor that should either be removed if no longer needed or restored if it represents a valid alternative implementation.packages/web/components/chart/light-weight-charts/chart.tsx (2)
Line range hint
150-169: Consider optimizing the date comparison logic.The current implementation checks every data point in every series to determine if they fall within a one-day range. This could be optimized by checking only the first and last points of each series.
Consider this optimization:
const isOneDay = series?.every((value) => - value.data.every((data) => { - const date = dayjs(timepointToDate(data.time)); - const diffDays = Math.abs(date.diff(Date.now(), "days")); - return diffDays <= 1 && diffDays >= 0; - }) + value.data.length > 0 && (() => { + const firstDate = dayjs(timepointToDate(value.data[0].time)); + const lastDate = dayjs(timepointToDate(value.data[value.data.length - 1].time)); + const now = dayjs(); + return Math.abs(firstDate.diff(now, "days")) <= 1 && + Math.abs(lastDate.diff(now, "days")) <= 1; + })() );
Line range hint
219-226: Consider optimizing the resize subscription selector.The selector function in
useSyncExternalStorealways returnstrue, which might cause unnecessary re-renders. Consider tracking the actual dimensions or returning a stable value.Here's a suggested improvement:
useSyncExternalStore( resizeSubscribe, () => { chart.current?.resize(); }, - () => true + () => container?.clientWidth ?? 0 );packages/web/components/assets/highlights-categories.tsx (1)
Line range hint
132-146: Remove unused propdisableLinking.The
disableLinkingprop is declared in the component's props interface but is never used in the implementation.Apply this diff to remove the unused prop:
const AssetHighlights: FunctionComponent< { title: string; subtitle?: string; onClickSeeAll?: () => void; assets: { asset: { coinDenom: string; coinName: string; coinImageUrl?: string; }; extraInfo: ReactNode; }[]; isLoading?: boolean; - disableLinking?: boolean; highlight: Highlight; } & CustomClasses >packages/web/components/transactions/transaction-row.tsx (1)
Line range hint
199-233: LGTM! Consider minor improvements to TokenTransfer component.Making
TokenTransferlocally scoped is appropriate as it's only used within this file. However, consider these improvements:
- Extract the value formatting logic to a utility function
- Simplify the conditional classNames using a mapping object
Consider this refactor:
+ const getStatusClassName = (status: TransactionStatus) => ({ + 'text-osmoverse-400': status === 'pending', + 'text-osmoverse-100': status === 'success', + 'text-rust-400': status === 'failed', + }); + const formatTransferValue = (value: PricePretty, direction: 'deposit' | 'withdraw') => + `${direction === 'withdraw' ? '-' : '+'} ${value.symbol}${Number(value.toDec().abs().toString()).toFixed(2)}`; const TokenTransfer: FunctionComponent<{ status: TransactionStatus; } & NonNullable<TransactionRow["transfer"]>> = ({ status, direction, amount, value }) => ( <div className="flex items-center gap-4"> <FallbackImg alt={amount.denom} src={amount.currency.coinImageUrl} fallbacksrc="/icons/question-mark.svg" height={32} width={32} /> <div className="body2 text-osmoverse-400"> {formatPretty(amount, { maxDecimals: 6 })} </div> {value && ( <div className={classNames("subtitle1", getStatusClassName(status))}> - {direction === "withdraw" ? "-" : "+"} {value.symbol} - {Number(value.toDec().abs().toString()).toFixed(2)} + {formatTransferValue(value, direction)} </div> )} </div> );packages/web/components/buttons/button.tsx (2)
Line range hint
7-196: Consider extracting variant definitions to improve maintainability.The button variants configuration is quite extensive. Consider extracting the variant definitions into a separate configuration file to improve maintainability and make the component file more focused.
Create a new file
button.config.ts:import { cva } from "class-variance-authority"; export const buttonVariants = { mode: { primary: [ /* ... */ ], // ... other modes }, size: { xs: "h-6 px-2 button tracking-wide text-xs", // ... other sizes }, }; export const buttonDefaults = { mode: "primary", size: "normal", }; export const buttonCVA = cva( "flex w-full group place-content-center items-center py-2 text-center transition-colors disabled:cursor-default", { variants: buttonVariants, defaultVariants: buttonDefaults, } );Then import and use it in the button component:
import { buttonCVA } from "./button.config";
Line range hint
196-234: Consider using React.memo for performance optimization.The Button component is likely to be used frequently throughout the application. Consider wrapping it with React.memo to prevent unnecessary re-renders when props haven't changed.
export const Button = memo( forwardRef<HTMLButtonElement, ButtonProps>((props, ref) => { // ... existing implementation }) );packages/web/components/chart/concentrated-liquidity-depth.tsx (1)
Line range hint
196-204: Consider improving TypeScript type safety.There are a few areas where type safety could be enhanced:
- The
@ts-ignorecomments could be replaced with proper type definitions- The
scale: anytype in DragContainer props could be more specificConsider addressing these in a future PR to improve type safety. Would you like me to help create a GitHub issue to track these improvements?
Also applies to: 219-225
packages/web/components/main-menu.tsx (1)
Line range hint
87-93: Consider improving type safety for thehrefprop.The
hrefprop type is currently defined asstring | any, which is overly permissive. Since it's used to handle either a URL string or a click handler function, we can make it more type-safe.Consider updating the type definition:
const MenuLink: FunctionComponent<{ - href: string | any; + href: string | MouseEventHandler<HTMLAnchorElement>; secondaryLogo?: React.ReactNode; children: MaybeRenderProp<{ showSubTitle: boolean; selected: boolean }>; selectionTest?: RegExp; showMore?: boolean; }> = ({ href, children, secondaryLogo, selectionTest, showMore }) => {packages/web/components/table/index.tsx (4)
Line range hint
39-47: Consider optimizing row hover state management.The current implementation recreates the entire hover state array on each hover change. Consider using a more efficient approach.
-const setRowHovered = useCallback( - (rowIndex: number, value: boolean) => - setRowsHovered( - replaceAt( - value, - data.map(() => false), - rowIndex - ) - ), - [data] -); +const setRowHovered = useCallback( + (rowIndex: number, value: boolean) => + setRowsHovered((prev) => { + const next = [...prev]; + next[rowIndex] = value; + return next; + }), + [] +);
Line range hint
82-98: Enhance accessibility for sortable columns.The sortable columns should have proper ARIA attributes to indicate their current sort state and interactivity.
<th key={colIndex} className={classNames( { "cursor-pointer select-none items-center": colDef?.sort?.onClickHeader, }, colDef.className )} + role={colDef?.sort?.onClickHeader ? "columnheader button" : "columnheader"} + aria-sort={ + colDef?.sort?.currentDirection === "ascending" + ? "ascending" + : colDef?.sort?.currentDirection === "descending" + ? "descending" + : "none" + } onClick={() => colDef?.sort?.onClickHeader(colIndex)} >
Line range hint
186-191: Enhance button accessibility in ClickableContent.The button wrapper should have proper ARIA attributes and role.
const ClickableContent = ({ isButton = false, children, }: PropsWithChildren<{ isButton?: boolean }>) => - isButton ? <button>{children}</button> : <>{children}</>; + isButton ? ( + <button type="button" role="button" aria-pressed="false"> + {children} + </button> + ) : ( + <>{children}</> + );
Props interface should remain exported
The Props interface should remain exported as there is evidence it was previously exported and the change to make it non-exported would be breaking. The git history shows the interface was exported before (
export interface Props), and we can see multiple components importing from the table component that might be affected:
packages/web/modals/superfluid-validator.tsxpackages/web/components/complex/pools-table.tsxpackages/web/components/table/cells/pool-composition.tsx- And several other components that depend on the table component
🔗 Analysis chain
Line range hint
18-31: Verify the impact of Props interface visibility change.The Props interface is no longer exported, which could be a breaking change if other components were importing and using this type.
Let's verify if any other files are importing this interface:
The previous search might have been too specific. Let's broaden our search to find any imports from the table component and usage of the Props type, as well as check the git history to confirm if this was previously exported.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for imports of Props from this file rg "import.*Props.*from.*components/table"Length of output: 44
Script:
#!/bin/bash # Let's try a broader search for any imports from the table component rg "from ['\"].*components/table" -A 2 # Also search for direct usage of Props type rg "Props[<\s].*table" # Let's also check if this file was previously exporting Props git log -p packages/web/components/table/index.tsx | rg "export.*interface Props"Length of output: 3907
packages/web/components/swap-tool/split-route.tsx (3)
Line range hint
102-252: Add keyboard navigation support for pool buttons.The pool buttons in the
Poolscomponent lack keyboard navigation attributes. Consider adding proper ARIA labels and keyboard interaction support.<button className="flex items-center gap-1 rounded-full bg-osmoverse-800 py-1 px-2 hover:bg-osmoverse-700" onClick={() => { if (!isMobile) router.push("/pool/" + id); }} + aria-label={`View pool ${id} details`} + role="link" >
Line range hint
254-272: Consider adding loading state for currency images.The
DenomImagecomponent could benefit from a loading state for the currency images to prevent layout shifts.<Image src={currency.coinImageUrl} alt="token icon" width={size} height={size} + loading="eager" + onError={(e) => { + e.currentTarget.src = '/fallback-token-icon.svg'; + }} />
Silent currency resolution failures should be logged for monitoring
The codebase already has error handling patterns and error boundaries (
error-boundary.tsx), and uses console logging in other swap-tool components for error cases. However, the currency resolution failure insplit-route.tsxis silently handled. While returning null is acceptable for the UI, we should log these cases for monitoring.- if (!sendCurrency || !lastOutCurrency) return null; + if (!sendCurrency || !lastOutCurrency) { + console.warn("Failed to resolve currencies in split route", { + sendDenom: (route as RouteOutGivenIn).tokenInDenom || (route as RouteInGivenOut).tokenInDenoms?.[0], + outDenom: (route as RouteInGivenOut).tokenOutDenom || (route as RouteOutGivenIn).tokenOutDenoms?.at(-1) + }); + return null; + }🔗 Analysis chain
Line range hint
29-89: Consider adding error boundary for currency resolution.The
RouteLanecomponent silently returns null when currencies aren't found. Consider implementing an error boundary or logging mechanism to track these scenarios.- if (!sendCurrency || !lastOutCurrency) return null; + if (!sendCurrency || !lastOutCurrency) { + console.warn('Failed to resolve currencies:', { + sendCurrency: route.tokenInDenom || route.tokenInDenoms?.[0], + lastOutCurrency: route.tokenOutDenom || route.tokenOutDenoms?.[route.tokenOutDenoms?.length - 1] + }); + return null; + }🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for error boundary implementations in the codebase rg -l "ErrorBoundary|componentDidCatch|getDerivedStateFromError" --type typescriptLength of output: 119
Script:
#!/bin/bash # Check for error boundary implementations in the codebase with ripgrep # Using multiple passes to cover different file extensions rg -l "ErrorBoundary|componentDidCatch|getDerivedStateFromError" --type-add 'ts:*.{ts,tsx}' --type ts # Check for existing error logging patterns in swap-tool components rg "console\.(error|warn)" "packages/web/components/swap-tool" --type-add 'ts:*.{ts,tsx}' --type ts -C 2 # Check for existing null handling patterns in similar route/swap components rg "(![\w]+)\s*\?\?\s*null|return\s+null" "packages/web/components/swap-tool" --type-add 'ts:*.{ts,tsx}' --type ts -C 2Length of output: 1970
packages/web/components/stepper/index.tsx (2)
Line range hint
1-347: Consider enhancing the component with additional features.While the implementation is solid, here are some suggestions for future improvements:
- Add support for keyboard navigation (left/right arrows)
- Consider adding a
pauseOnFocusoption alongsidestopOnHover- Add support for swipe gestures on touch devices
- Consider adding an
onStepChangecallback prop for analyticsWould you like me to provide implementation details for any of these suggestions?
Line range hint
14-14: Consider using a more specific import path.-import { createContext } from "~/utils/react-context"; +import { createContext } from "~/utils/react/context";This helps maintain a clearer separation between React-specific utilities and general utilities.
packages/web/components/alert/toast.tsx (2)
Line range hint
241-259: TODO comment requires implementationThe TODO comment indicates missing modal integration functionality. This could affect the user experience as the convert functionality is not fully implemented.
Would you like me to help create a GitHub issue to track the implementation of the modal integration? The issue would include:
- Modal implementation requirements
- State management considerations
- Integration with the existing toast component
Line range hint
267-341: Consider extracting common toast patternsThe toast implementation could be more maintainable by:
- Extracting common icon configurations to constants
- Creating a shared base toast component for common styling patterns
Example refactor for icon configurations:
const ASSET_ICONS = { variants: [ { id: 'usdc-variant-static', className: 'z-10' }, { id: 'btc-variant-static', className: 'z-20' }, { id: 'eth-variant-static', className: 'z-30' }, ], static: [ { id: 'usdc-static', className: 'z-10' }, { id: 'btc-static', className: 'z-20' }, { id: 'eth-static', className: 'z-30' }, ] } as const;This would simplify the JSX and make icon management more maintainable:
-<div className="flex -space-x-2"> - <Icon id="usdc-variant-static" height={32} width={32} className="z-10" /> - <Icon id="btc-variant-static" height={32} width={32} className="z-20" /> - <Icon id="eth-variant-static" height={32} width={32} className="z-30" /> -</div> +<div className="flex -space-x-2"> + {ASSET_ICONS.variants.map(({ id, className }) => ( + <Icon key={id} id={id} height={32} width={32} className={className} /> + ))} +</div>packages/web/config/utils.ts (1)
Line range hint
191-208: Consider addressing the technical debt in staking token handling.The comment indicates a temporary workaround for chains without staking tokens. This "hacky fix" involves using a placeholder token to maintain compatibility with older Keplr types.
Consider creating a tracking issue to:
- Upgrade to newer Keplr chain types that support missing staking tokens
- Remove the placeholder token workaround
- Clean up the keplr stores code
Would you like me to create a GitHub issue to track this technical debt?
packages/web/hooks/limit-orders/use-orderbook.ts (4)
Line range hint
183-183: Consider using a translation constantThe hardcoded error message
"errors.noOrderbook"should be defined in a centralized translations file for better maintainability and i18n support.
Line range hint
217-217: Document the default maker fee valueConsider adding a comment explaining why
Dec(0)is used as the default maker fee during loading state, as this might have implications for UI calculations.
Line range hint
299-299: Consider enabling keepPreviousDataSetting
keepPreviousData: falsemight cause UI flicker during refetches. Consider enabling it to provide a smoother user experience, especially for paginated data.
Line range hint
445-445: Remove or uncomment the refetch callThere's a commented out
refetchcall in theclaimAllOrdersfunction. Either remove it if it's no longer needed or uncomment it if the refetch is required after claiming orders.packages/web/components/swap-tool/trade-details.tsx (3)
366-367: Consider adding JSDoc documentationGiven the complexity of this function and its parameters, adding JSDoc documentation would improve maintainability.
Line range hint
374-377: Replace 'var' with 'const' and enhance error loggingTwo improvements needed:
- Use
constinstead ofvarfor better scoping- Add more context to the warning message
- var inBaseOutQuoteSpotPrice = + const inBaseOutQuoteSpotPrice = swapState?.inBaseOutQuoteSpotPrice?.toDec() ?? new Dec(1); if (inBaseOutQuoteSpotPrice.isZero()) { - console.warn("ExpectedRate: inBaseOutQuoteSpotPrice is Zero"); + console.warn( + "ExpectedRate: Invalid spot price (zero) detected", + { fromAsset: swapState.fromAsset?.coinDenom, toAsset: swapState.toAsset?.coinDenom } + ); return; }
Line range hint
366-487: Consider refactoring for improved maintainabilityThe function has high cyclomatic complexity with multiple nested conditions and price calculations. Consider breaking it down into smaller, focused functions for each price calculation scenario (stable coins, outAsBase, etc.).
Example structure:
function calculateStableInPrice(swapState, inBaseOutQuoteSpotPrice, priceOverride) { // Handle treatAsStable === "in" case } function calculateStableOutPrice(swapState, inBaseOutQuoteSpotPrice, priceOverride) { // Handle treatAsStable === "out" case } function calculateTokenPrice(swapState, outAsBase, inBaseOutQuoteSpotPrice) { // Handle regular token price calculations } function ExpectedRate(...) { if (inBaseOutQuoteSpotPrice.isZero()) { // Handle error case } if (treatAsStable === "in") { return calculateStableInPrice(...) } if (treatAsStable === "out") { return calculateStableOutPrice(...) } return calculateTokenPrice(...) }packages/web/components/table/asset-info.tsx (1)
Line range hint
620-665: Consider performance and type safety improvementsThe implementation is solid with good event handling and clear conditional rendering. Consider these improvements:
- Memoize the click handler to prevent unnecessary re-renders
- Use strict boolean type for
showUnverifiedAssetsSettingconst AssetActionsCell: AssetCellComponent<{ - showUnverifiedAssetsSetting?: boolean; + showUnverifiedAssetsSetting: boolean; confirmUnverifiedAsset: (asset: { coinDenom: string; coinImageUrl?: string; }) => void; }> = ({ coinDenom, coinMinimalDenom, coinImageUrl, isVerified, showUnverifiedAssetsSetting, confirmUnverifiedAsset, }) => { const { t } = useTranslation(); + const handleActivation = useCallback((e: React.MouseEvent) => { + e.preventDefault(); + confirmUnverifiedAsset({ coinDenom, coinImageUrl }); + }, [coinDenom, coinImageUrl, confirmUnverifiedAsset]); const needsActivation = !isVerified && !showUnverifiedAssetsSetting; return ( <div className="flex items-center gap-2 text-wosmongton-200"> {needsActivation ? ( <Button variant="ghost" className="flex gap-2 text-wosmongton-200 hover:text-rust-200" - onClick={(e) => { - e.preventDefault(); - confirmUnverifiedAsset({ coinDenom, coinImageUrl }); - }} + onClick={handleActivation} > {t("assets.table.activate")} </Button> ) : ( <HistoricalPriceSparkline coinMinimalDenom={coinMinimalDenom} timeFrame="1W" /> )} </div> ); };packages/web/components/complex/pools-table.tsx (3)
Line range hint
177-308: Consider optimizing columns definitionThe columns array is recreated whenever
shouldDisplayVolumeDataorshouldDisplayFeesDatachanges, which could impact performance. Consider splitting the columns definition into smaller memoized parts or using a reducer pattern.Here's a suggested approach:
// Memoize base columns const baseColumns = useMemo(() => { const columnHelper = createColumnHelper<Pool>(); return [ columnHelper.accessor((row) => row, { id: "pool", header: t("pools.allPools.sort.poolName"), cell: PoolCompositionCell, }), ]; }, [t]); // Memoize volume columns const volumeColumns = useMemo(() => { if (!shouldDisplayVolumeData) return []; const columnHelper = createColumnHelper<Pool>(); return [ columnHelper.accessor( (row) => row.market?.volume24hUsd?.toString() ?? "N/A", { id: "market.volume24hUsd", header: () => ( <SortHeader<MarketIncentivePoolsSortKey> label={t("pools.allPools.sort.volume24h")} sortKey="market.volume24hUsd" disabled={isLoading} currentSortKey={sortKey} currentDirection={sortParams.allPoolsSortDir} setSortDirection={setSortDirection} setSortKey={setSortKey} /> ), } ), ]; }, [shouldDisplayVolumeData, t, isLoading, sortKey, sortParams.allPoolsSortDir, setSortDirection, setSortKey]); // Combine columns in final useMemo const columns = useMemo(() => { return [...baseColumns, ...volumeColumns, /* other column groups */]; }, [baseColumns, volumeColumns, /* other column group dependencies */]);
Line range hint
580-593: Consider extracting URLs to constants and improving type safetyThe hardcoded URLs should be moved to constants for better maintainability. Additionally, the pool type handling could be more type-safe.
const POOL_URLS = { 'cosmwasm-transmuter': 'https://celatone.osmosis.zone/osmosis-1/pools', 'cosmwasm-astroport-pcl': 'https://osmosis.astroport.fi/pools', 'cosmwasm-whitewhale': 'https://app.whitewhale.money/osmosis/pools', } as const; function getPoolLink(pool: Pool): string { const externalUrl = POOL_URLS[pool.type as keyof typeof POOL_URLS]; if (externalUrl) { return `${externalUrl}/${pool.id}`; } return `/pool/${pool.id}`; }
Line range hint
595-604: Simplify pool target logic using type-safe approachThe current implementation can be simplified and made more type-safe.
const EXTERNAL_POOL_TYPES = [ 'cosmwasm-transmuter', 'cosmwasm-astroport-pcl', 'cosmwasm-whitewhale', ] as const; function getPoolTypeTarget(pool: Pool): '_blank' | '' { return EXTERNAL_POOL_TYPES.includes(pool.type as typeof EXTERNAL_POOL_TYPES[number]) ? '_blank' : ''; }packages/web/components/bridge/bridge-wallet-select-modal.tsx (3)
Line range hint
98-424: Consider splitting component into smaller, focused componentsThe
BridgeWalletSelectScreenscomponent has multiple responsibilities and complex state management. Consider splitting it into smaller, focused components:
- WalletConnectionManager
- ChainSwitcher
- AddressValidationForm
This would improve maintainability and testability.
Line range hint
196-208: Add error handling for chain switching failureThe chain switching logic swallows errors silently. Consider adding user feedback:
if (shouldSwitchChain) { try { setIsSwitchingChain(true); await switchChainAsync({ chainId: evmChain.chainId as EthereumChainIds, }); } catch (error) { setIsSwitchingChain(false); + // Show error to user + console.error('Failed to switch chain:', error); + // Consider adding a toast notification or error state return; } }
Line range hint
133-146: Add cleanup function to useEffectThe useEffect hook that manages modal height should clean up its state on unmount:
useEffect(() => { if ( !isNil(connectingWagmiVariables?.connector) || (isEvmWalletConnected && isSwitchingChain) ) { return setRemoveMinHeight?.(true); } setRemoveMinHeight?.(false); + return () => { + setRemoveMinHeight?.(false); + }; }, [ connectingWagmiVariables?.connector, isEvmWalletConnected, isSwitchingChain, setRemoveMinHeight, ]);packages/web/components/bridge/deposit-address-screen.tsx (2)
Line range hint
491-511: Consider consolidating shared types and patternsThe EstimatedTimeRow, ProviderFeesRow, and NetworkFeeRow components share similar props and patterns. Consider extracting the common interface and creating a base component to reduce duplication.
Consider this refactor to improve maintainability:
interface BaseDetailRowProps { depositData: RouterOutputs["bridgeTransfer"]["getDepositAddress"]["depositData"]; isRefetchingQuote: boolean; fromChainName?: string; } const BaseDetailRow: FunctionComponent< BaseDetailRowProps & { label: string; renderValue: (props: BaseDetailRowProps) => ReactNode; } > = ({ label, renderValue, ...props }) => ( <QuoteDetailRow label={label} isLoading={props.isRefetchingQuote} > {renderValue(props)} </QuoteDetailRow> ); // Usage example: const EstimatedTimeRow: FunctionComponent<BaseDetailRowProps> = (props) => ( <BaseDetailRow {...props} label={t("transfer.estimatedTime")} renderValue={({ depositData }) => ( <div className="flex items-center gap-1"> <Icon id="stopwatch" className="h-4 w-4 text-osmoverse-400" /> <p className="text-osmoverse-100 first-letter:capitalize"> {t(depositData.estimatedTime)} </p> </div> )} /> );Also applies to: 512-526, 527-583
Line range hint
82-98: Enhance accessibility for QR code and copy functionalityWhile the implementation handles the core functionality well, there are opportunities to improve accessibility:
- The QR code should have proper aria-labels and role attributes
- The copy functionality should announce its state to screen readers
Consider these accessibility improvements:
const QRCode = dynamic( () => import("~/components/qrcode").then((module) => module.QRCode), { loading: () => <Spinner className="text-black" />, } ); + // Add proper aria attributes for QR code + const QRCodeWrapper: FunctionComponent<{ value: string; size: number; logoUrl?: string }> = (props) => ( + <div + role="img" + aria-label={t("transfer.qrCodeFor", { address: props.value })} + > + <QRCode {...props} /> + </div> + ); // In the component: -<QRCode +<QRCodeWrapper value={data?.depositData?.depositAddress} size={220} logoUrl={canonicalAsset.coinImageUrl} /> // For copy functionality: +const copyButtonId = "copy-address-button"; +const copyStatusId = "copy-status"; <IconButton + id={copyButtonId} + aria-describedby={copyStatusId} icon={ <Icon id={hasCopied ? "check-mark-slim" : "copy"} className={hasCopied ? "text-white-full" : "..."} /> } onClick={() => { logEvent([EventName.DepositWithdraw.addressCopied]); onCopy(); }} disabled={isLoading || isExpired} /> +<span + id={copyStatusId} + role="status" + className="sr-only" +> + {hasCopied ? t("transfer.copied") : ""} +</span>Also applies to: 264-307
packages/web/components/table/portfolio-asset-balances.tsx (1)
Line range hint
597-598: Replace temporary alert with proper modal implementation.The TODO comment and
alert()call indicate incomplete implementation of the conversion modal. This should be addressed before merging.Would you like me to help implement the conversion modal or create a GitHub issue to track this task?
packages/web/hooks/limit-orders/use-place-limit.ts (5)
Line range hint
332-341: Consider extracting magic numbers to named constants.The outlier USD value threshold check uses a magic number. Consider extracting this to a named constant for better maintainability and documentation.
- if (isNaN(valueUsd) || valueUsd > OUTLIER_USD_VALUE_THRESHOLD) { + const isOutlierValue = isNaN(valueUsd) || valueUsd > OUTLIER_USD_VALUE_THRESHOLD; + if (isOutlierValue) { valueUsd = 0; }
Line range hint
342-351: Consider using TypeScript enums for event names.The analytics event tracking could benefit from using TypeScript enums to ensure type safety and prevent typos in event names.
enum LimitOrderEventName { PlaceOrderStarted = 'limitOrder.placeOrderStarted', PlaceOrderCompleted = 'limitOrder.placeOrderCompleted', PlaceOrderFailed = 'limitOrder.placeOrderFailed', }
Line range hint
516-543: Consider extracting gas estimation logic.The gas estimation logic could be extracted into a separate hook for better maintainability and reusability.
const useGasEstimation = ({ isMarket, encodedMsg, accountAddress, insufficientFunds, isTyping, }: GasEstimationParams) => { const shouldEstimate = useMemo(() => { return ( !isMarket && !!encodedMsg && !!accountAddress && !insufficientFunds && !isTyping ); }, [isMarket, encodedMsg, accountAddress, insufficientFunds, isTyping]); // ... rest of the gas estimation logic };
Line range hint
614-632: Consider simplifying price validation logic.The price validation logic could be simplified by extracting the validation rules into a separate function for better readability and maintainability.
const isPriceValid = (price: string, spotPrice: Dec): boolean => { const decPrice = new Dec(price); return Boolean(price) && price.length > 0 && !decPrice.isZero() && decPrice.isPositive() && decPrice.gte(minPrice) && decPrice.lte(maxPrice); };
Line range hint
509-515: Consider adding decimal precision configuration.The decimal precision handling is currently hardcoded. Consider making it configurable based on the asset type or user preferences.
interface DecimalPrecisionConfig { price: number; percentage: number; amount: number; } const DEFAULT_PRECISION: DecimalPrecisionConfig = { price: 12, percentage: 3, amount: 8, };packages/web/components/swap-tool/index.tsx (6)
195-203: Simplify redundant ternary expressions inresetSlippagefunctionThe ternary operators are redundant since both outcomes are the same (
DefaultSlippageand0). You can simplify the code by removing the unnecessary ternary expressions.Apply this diff to simplify the code:
- const defaultSlippage = - quoteType === "in-given-out" ? DefaultSlippage : DefaultSlippage; ... - slippageConfig.select(quoteType === "in-given-out" ? 0 : 0); + const defaultSlippage = DefaultSlippage; ... + slippageConfig.select(0);
337-337: Remove redundantBooleanconversionThe
Booleanfunction call is unnecessary here because the condition will coerceswapState.quoteto a boolean automatically.Apply this diff to simplify the condition:
- !Boolean(swapState.quote) || + !swapState.quote ||
387-388: Simplify conditional with optional chainingYou can simplify the expression by using optional chaining.
Apply this diff to refactor the code:
- (swapState.inAmountInput.balance && - swapState.inAmountInput.balance.toDec().isZero()) + swapState.inAmountInput.balance?.toDec().isZero()🧰 Tools
🪛 Biome
[error] 387-388: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
642-642: Remove redundantBooleanconversionThe
Booleanfunction call is unnecessary here because the condition will coerceswapState.quoteto a boolean automatically.Apply this diff to simplify the condition:
- !Boolean(swapState.quote) || + !swapState.quote ||🧰 Tools
🪛 Biome
[error] 642-642: Avoid redundant
BooleancallIt is not necessary to use
Booleancall when a value will already be coerced to a boolean.
Unsafe fix: Remove redundantBooleancall(lint/complexity/noExtraBooleanCast)
644-644: Remove redundantBooleanconversionThe
Booleanfunction call is unnecessary here because the condition will coerceswapState.errorto a boolean automatically.Apply this diff to simplify the condition:
- Boolean(swapState.error) || + swapState.error ||
645-645: Remove redundantBooleanconversionThe
Booleanfunction call is unnecessary here because the condition will coerceswapState.networkFeeErrorto a boolean automatically.Apply this diff to simplify the condition:
- (Boolean(swapState.networkFeeError) && + (swapState.networkFeeError &&packages/tx/src/__tests__/gas.spec.ts (5)
Line range hint
48-51: Refactor repeated mock setups into helper functionsThe mock setup for
queryBaseAccountis repeated across multiple test cases. Consider extracting this into a helper function to improve maintainability and reduce duplication.Here's how you might refactor:
// Helper function at the top function mockQueryBaseAccount(sequence = "1") { (queryBaseAccount as jest.Mock).mockResolvedValue({ account: { "@type": BaseAccountTypeStr, sequence, }, } as Awaited<ReturnType<typeof queryBaseAccount>>); } // In your test cases, replace the repeated mock setups with: mockQueryBaseAccount();Also applies to: 79-82, 113-116
Line range hint
69-69: Typo in '@type' string valueThere's a typo in the
@typestring'non-base-type-assummed-vesting'. The word'assummed'should be'assumed'.Correct the typo:
- "@type": "non-base-type-assummed-vesting", + "@type": "non-base-type-assumed-vesting",
Line range hint
347-349: 'expect' statements outside of test casesThe
expectstatements at the end of thedescribe("getGasPriceByFeeDenom", ...)block are outside of anyittest case. In Jest, all assertions must be insideitblocks.Move the
expectstatements into an appropriateitblock or remove them if unnecessary.- expect(queryFeesBaseGasPrice).not.toHaveBeenCalled(); - expect(queryFeeTokenSpotPrice).not.toHaveBeenCalled();
Line range hint
384-387: 'expect' statements should be within test casesThe
expectstatements at the end of thedescribe("getDefaultGasPrice", ...)block are outside of anyittest case. Assertions should be placed insideitblocks to ensure they are executed properly.Encapsulate the
expectstatements within anitblock or remove them if they are redundant.- expect(queryFeesBaseGasPrice).not.toHaveBeenCalled(); - expect(queryFeeTokenSpotPrice).not.toHaveBeenCalled();
Line range hint
102-106: Avoid casting to 'any' in mock rejectionsCasting to
anyreduces type safety. Instead of casting the mocked rejection toany, consider defining the appropriate types to maintain type safety.Adjust your mocked error to match the expected error type without using
as any.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (6)
packages/web/knip.jsonis excluded by!**/*.jsonpackages/web/localizations/de.jsonis excluded by!**/*.jsonpackages/web/localizations/en.jsonis excluded by!**/*.jsonpackages/web/localizations/pt-br.jsonis excluded by!**/*.jsonpackages/web/localizations/tr.jsonis excluded by!**/*.jsonpackages/web/package.jsonis excluded by!**/*.json
📒 Files selected for processing (82)
packages/tx/src/__tests__/gas.spec.ts(1 hunks)packages/web/__tests__/test-utils.tsx(2 hunks)packages/web/components/ad-banner/ad-banner.tsx(1 hunks)packages/web/components/alert/cypher-card-toast.tsx(1 hunks)packages/web/components/alert/toast.tsx(2 hunks)packages/web/components/assets/highlights-categories.tsx(1 hunks)packages/web/components/bridge/bridge-wallet-select-modal.tsx(1 hunks)packages/web/components/bridge/deposit-address-screen.tsx(3 hunks)packages/web/components/buttons/button.tsx(1 hunks)packages/web/components/chart/concentrated-liquidity-depth.tsx(1 hunks)packages/web/components/chart/light-weight-charts/chart-controller.ts(1 hunks)packages/web/components/chart/light-weight-charts/chart.tsx(2 hunks)packages/web/components/complex/pool/create/select-type.tsx(1 hunks)packages/web/components/complex/pools-table.tsx(7 hunks)packages/web/components/complex/portfolio/allocation-tabs.tsx(1 hunks)packages/web/components/complex/portfolio/allocation.tsx(4 hunks)packages/web/components/complex/portfolio/cypher-card.tsx(3 hunks)packages/web/components/complex/portfolio/portfolio-dust.ts(0 hunks)packages/web/components/complex/portfolio/portfolio-page.tsx(4 hunks)packages/web/components/control/index.ts(0 hunks)packages/web/components/control/language-select.tsx(1 hunks)packages/web/components/control/sort-menu.tsx(0 hunks)packages/web/components/control/types.ts(0 hunks)packages/web/components/earn/filters/filter-context.tsx(1 hunks)packages/web/components/earn/table/columns.tsx(1 hunks)packages/web/components/earn/table/utils.ts(1 hunks)packages/web/components/input/input-box.tsx(1 hunks)packages/web/components/layouts/main.tsx(4 hunks)packages/web/components/main-menu.tsx(1 hunks)packages/web/components/navbar/index.tsx(5 hunks)packages/web/components/one-click-trading/one-click-trading-toast.tsx(1 hunks)packages/web/components/place-limit-tool/index.tsx(1 hunks)packages/web/components/screen-manager.tsx(1 hunks)packages/web/components/stepper/index.tsx(1 hunks)packages/web/components/stepper/use-steps.ts(1 hunks)packages/web/components/swap-tool/alt.tsx(0 hunks)packages/web/components/swap-tool/index.tsx(9 hunks)packages/web/components/swap-tool/split-route.tsx(1 hunks)packages/web/components/swap-tool/swap-tool-tabs.tsx(1 hunks)packages/web/components/swap-tool/trade-details.tsx(2 hunks)packages/web/components/table/asset-info.tsx(1 hunks)packages/web/components/table/cells/asset-name.tsx(0 hunks)packages/web/components/table/cells/balance.tsx(0 hunks)packages/web/components/table/cells/index.ts(0 hunks)packages/web/components/table/cells/transfer-button.tsx(0 hunks)packages/web/components/table/cells/types.ts(0 hunks)packages/web/components/table/index.tsx(1 hunks)packages/web/components/table/portfolio-asset-balances.tsx(9 hunks)packages/web/components/table/types.ts(1 hunks)packages/web/components/tooltip/apr-disclaimer.tsx(1 hunks)packages/web/components/trade-tool/index.tsx(5 hunks)packages/web/components/transactions/recent-activity/recent-activity-transaction-row.tsx(1 hunks)packages/web/components/transactions/transaction-row.tsx(2 hunks)packages/web/components/types.ts(0 hunks)packages/web/components/ui/input.tsx(1 hunks)packages/web/components/ui/pagination.tsx(0 hunks)packages/web/components/ui/select.tsx(0 hunks)packages/web/config/env.ts(0 hunks)packages/web/config/utils.ts(2 hunks)packages/web/hooks/data/data-sorter.ts(1 hunks)packages/web/hooks/data/types.ts(0 hunks)packages/web/hooks/index.ts(0 hunks)packages/web/hooks/language/types.ts(2 hunks)packages/web/hooks/limit-orders/use-orderbook.ts(2 hunks)packages/web/hooks/limit-orders/use-place-limit.ts(1 hunks)packages/web/hooks/mutations/one-click-trading/use-remove-one-click-trading-session.ts(1 hunks)packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts(1 hunks)packages/web/hooks/ui-config/use-asset-info-config.ts(1 hunks)packages/web/hooks/use-controllable-state.ts(1 hunks)packages/web/hooks/use-deterministic.ts(0 hunks)packages/web/hooks/use-dimension.ts(1 hunks)packages/web/hooks/use-disclosure.ts(1 hunks)packages/web/hooks/use-feature-flags.ts(0 hunks)packages/web/hooks/use-nav-bar.ts(1 hunks)packages/web/hooks/use-show-preview-assets.ts(1 hunks)packages/web/hooks/user-settings/use-hide-dust-filter.ts(2 hunks)packages/web/hooks/window/use-visibility-state.ts(0 hunks)packages/web/hooks/window/use-window-scroll.ts(0 hunks)packages/web/hooks/window/use-window-size.ts(1 hunks)packages/web/integrations/core-walletconnect/types.ts(1 hunks)packages/web/integrations/keplr-walletconnect/index.ts(0 hunks)packages/web/integrations/keplr-walletconnect/registry.ts(0 hunks)
⛔ Files not processed due to max files limit (17)
- packages/web/modals/external-links-modal.tsx
- packages/web/modals/one-click-trading-intro-modal.tsx
- packages/web/modals/remove-concentrated-liquidity.tsx
- packages/web/modals/trade-tokens.tsx
- packages/web/modals/validator-squad-modal.tsx
- packages/web/modals/wallet-select/use-selectable-wallets.ts
- packages/web/pages/_app.tsx
- packages/web/pages/assets/[denom].tsx
- packages/web/pages/index.tsx
- packages/web/stores/nav-bar.ts
- packages/web/stores/queries-external/github/get-file.ts
- packages/web/stores/user-settings/hide-dust.tsx
- packages/web/styles/globals.css
- packages/web/utils/array.ts
- packages/web/utils/formatter.ts
- packages/web/utils/trpc.ts
- packages/web/utils/url.ts
💤 Files with no reviewable changes (22)
- packages/web/components/complex/portfolio/portfolio-dust.ts
- packages/web/components/control/index.ts
- packages/web/components/control/sort-menu.tsx
- packages/web/components/control/types.ts
- packages/web/components/swap-tool/alt.tsx
- packages/web/components/table/cells/asset-name.tsx
- packages/web/components/table/cells/balance.tsx
- packages/web/components/table/cells/index.ts
- packages/web/components/table/cells/transfer-button.tsx
- packages/web/components/table/cells/types.ts
- packages/web/components/types.ts
- packages/web/components/ui/pagination.tsx
- packages/web/components/ui/select.tsx
- packages/web/config/env.ts
- packages/web/hooks/data/types.ts
- packages/web/hooks/index.ts
- packages/web/hooks/use-deterministic.ts
- packages/web/hooks/use-feature-flags.ts
- packages/web/hooks/window/use-visibility-state.ts
- packages/web/hooks/window/use-window-scroll.ts
- packages/web/integrations/keplr-walletconnect/index.ts
- packages/web/integrations/keplr-walletconnect/registry.ts
✅ Files skipped from review due to trivial changes (4)
- packages/web/components/one-click-trading/one-click-trading-toast.tsx
- packages/web/components/place-limit-tool/index.tsx
- packages/web/hooks/data/data-sorter.ts
- packages/web/hooks/ui-config/use-add-concentrated-liquidity-config.ts
🧰 Additional context used
🪛 Biome
packages/web/components/complex/pools-table.tsx
[error] 503-503: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
packages/web/components/complex/portfolio/portfolio-page.tsx
[error] 63-63: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/web/components/swap-tool/index.tsx
[error] 336-336: Avoid redundant Boolean call
It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call
(lint/complexity/noExtraBooleanCast)
[error] 387-388: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 642-642: Avoid redundant Boolean call
It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call
(lint/complexity/noExtraBooleanCast)
🔇 Additional comments (89)
packages/web/hooks/use-show-preview-assets.ts (2)
Line range hint 5-14: LGTM! Clean hook implementation.
The hook implementation is focused, uses appropriate dependencies, and maintains a clean public API.
3-3: LGTM! Good encapsulation of the storage key.
The change to make ShowPreviewAssetsKey module-scoped aligns with encapsulation principles, as this constant is only used internally by the hook.
Let's verify there are no external references to this constant:
✅ Verification successful
✅ Safe to remove export - no external usage found
The constant ShowPreviewAssetsKey is only used within its own file (packages/web/hooks/use-show-preview-assets.ts), confirming that removing the export is safe and aligns with good encapsulation practices.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any external usage of ShowPreviewAssetsKey
# Expect: No matches outside this file, as it should only be used internally
rg -l "ShowPreviewAssetsKey" --type ts --type tsx
Length of output: 77
Script:
#!/bin/bash
# Let's try a different approach using ripgrep without file type filters
# and then filter the results ourselves
# Search for ShowPreviewAssetsKey in all files
rg -l "ShowPreviewAssetsKey"
Length of output: 74
packages/web/hooks/use-nav-bar.ts (2)
Line range hint 17-24: LGTM: Effect cleanup is properly implemented
The effect hook correctly manages the lifecycle of navigation bar state:
- Sets title and CTA buttons when provided
- Properly cleans up by resetting state on unmount
- Dependencies array correctly includes only the used variables
12-12:
Unused parameter hideTitle
The hideTitle parameter is added to the interface but is not utilized in the implementation. This could lead to confusion for consumers of this hook who might expect the parameter to have an effect.
Let's verify if this parameter is actually needed by the NavBar component:
packages/web/components/tooltip/apr-disclaimer.tsx (2)
Line range hint 11-24: LGTM! Well-structured component implementation.
The component demonstrates good practices:
- Proper use of TypeScript types
- Internationalization support via translation hooks
- Semantic HTML structure with appropriate styling classes
11-11: Verify removal of export statement.
The removal of the export statement for AprDisclaimer aligns with the component's usage pattern, as it's only used internally by AprDisclaimerTooltip. This change helps reduce the public API surface.
✅ Verification successful
Export removal of AprDisclaimer is safe
The search results confirm that only AprDisclaimerTooltip is imported and used in other files (pools-table.tsx and apr-breakdown.tsx). The AprDisclaimer component itself is not imported or used anywhere else in the codebase, making its export removal safe.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that AprDisclaimer is not used outside this file
# Expected: No matches should be found
# Search for any imports or usage of AprDisclaimer
rg "AprDisclaimer" --type ts --type tsx --glob '!**/apr-disclaimer.tsx'
Length of output: 99
Script:
#!/bin/bash
# Let's try a different approach using ripgrep without file type filters
# Search for any imports or usage of AprDisclaimer in all files except the component file itself
rg "AprDisclaimer" --glob '!**/apr-disclaimer.tsx'
# Also search with ast-grep to find any potential imports
ast-grep --pattern 'import { $$ AprDisclaimer $$ } from'
Length of output: 519
packages/web/components/ui/input.tsx (1)
4-7: LGTM! Good simplification of type definitions.
The change to use React's built-in InputHTMLAttributes instead of a custom interface is a positive improvement that:
- Reduces code duplication
- Ensures complete type coverage for all HTML input attributes
- Makes the component more maintainable
packages/web/integrations/core-walletconnect/types.ts (1)
Line range hint 15-20: LGTM! Good optimization of the module's public API surface.
The removal of the export keyword from WCDirectSignDoc is a positive change that helps reduce unnecessary exports while maintaining type safety, as the interface remains accessible through the exported WCSignDirectRequest and WCSignDirectResponse interfaces that reference it.
packages/web/hooks/use-disclosure.ts (1)
3-6: LGTM! Verify usage of UseDisclosureProps.
The removal of the export keyword from UseDisclosureProps is a good encapsulation decision since this interface is only used internally by the useDisclosure hook.
Let's verify there are no external dependencies on this interface:
✅ Verification successful
Interface removal is safe - no external dependencies found
The UseDisclosureProps interface is only used within use-disclosure.ts file - once in the interface declaration and once in the hook parameter. Removing the export keyword is safe as there are no external dependencies on this interface.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any external usage of UseDisclosureProps
# This should only find the current file
rg "UseDisclosureProps" --type ts
Length of output: 202
packages/web/hooks/user-settings/use-hide-dust-filter.ts (1)
30-31: Verify the intentional omission of getValueOfItem from dependencies
The removal of getValueOfItem from the dependencies array could lead to stale closures if the function changes. While the eslint-disable comment suggests this is intentional:
- Could you confirm if this is an optimization to prevent unnecessary recalculations?
- Are there guarantees that getValueOfItem's behavior remains consistent during the component's lifecycle?
✅ Verification successful
Now I have enough context to understand the issue. The useHideDustUserSetting hook uses getValueOfItem inside a useMemo callback but excludes it from the dependencies array. This is safe because:
getValueOfItemis a parameter passed to the hook, not a value created inside the component- The function is used purely for filtering and its identity changes won't affect the correctness of the cached result as long as its behavior remains consistent
- Including it would cause unnecessary recalculations when the function reference changes but its behavior doesn't change
Omitting getValueOfItem from dependencies is intentional and correct
The eslint-disable comment is appropriate here as it's a performance optimization that avoids unnecessary recalculations while maintaining correctness. The function's behavior is expected to remain consistent during the component's lifecycle since it's passed as a parameter.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for similar patterns in other hooks
# Expected: Find other useMemo hooks that might follow similar patterns
rg -B 2 -A 2 "useMemo.*getValueOf.*" --type ts
Length of output: 48
Script:
#!/bin/bash
# Let's first check the implementation of getValueOfItem to understand its usage
ast-grep --pattern 'const getValueOfItem = $_'
# Also check the useMemo implementation where it's used
ast-grep --pattern 'useMemo(() => {
$$$
}, [$$$])'
# Additionally, let's see if there are any other similar patterns with eslint-disable
rg "eslint-disable.*react-hooks/exhaustive-deps" -B 2 -A 2
Length of output: 243549
Script:
#!/bin/bash
# Let's check the implementation of getValueOfItem and its usage in the hook
ast-grep --pattern 'const getValueOfItem = $_'
# Also check the useMemo implementation where it's used
ast-grep --pattern 'useMemo(() => {
return items.filter((item) => {
$$$
});
}, [$$$]);'
Length of output: 146
Script:
#!/bin/bash
# Let's check the entire file content to understand the context
cat packages/web/hooks/user-settings/use-hide-dust-filter.ts
# Also check for similar patterns of dependency array exclusions in the codebase
rg "// eslint-disable.*react-hooks/exhaustive-deps.*\n.*\[.*\]" -A 1 -B 1
Length of output: 1398
packages/web/hooks/language/types.ts (1)
3-3: LGTM! Improved type encapsulation.
The removal of export keywords from utility types improves encapsulation by keeping implementation details private while maintaining the public API through TranslationPath, Translations, and other exported types.
Let's verify that these types aren't directly referenced elsewhere in the codebase:
Also applies to: 5-9, 11-19, 21-21
✅ Verification successful
Based on the search results, I can generate the final response:
Verified: Type visibility changes are safe
The removal of export keywords from utility types is safe as they are only used internally within packages/web/hooks/language/types.ts. The search results confirm:
LanguageTranslationsis only referenced within the same file and innavbar/index.tsxwhere it's accessing properties, not using the typePathsToStringPropsis only used within the same fileJoinmatches in search are unrelated function/method names, not the typeStringWithAutocompleteis only used within the same file
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for direct usage of the now-internal types
# Expected: No matches outside this file, as these types should only be used internally
echo "Checking for LanguageTranslations usage..."
rg "LanguageTranslations[^.]" --type ts
echo "Checking for PathsToStringProps usage..."
rg "PathsToStringProps[^.]" --type ts
echo "Checking for Join type usage..."
rg "Join[^.]" --type ts
echo "Checking for StringWithAutocomplete usage..."
rg "StringWithAutocomplete[^.]" --type ts
Length of output: 45176
packages/web/components/stepper/use-steps.ts (2)
Line range hint 10-57: LGTM! Clean and well-implemented hook.
The implementation follows React best practices with proper memoization, controlled/uncontrolled state pattern, and safe boundary checks.
5-8: LGTM! Verify no external usage of UseStepsProps type.
Making the interface internal aligns with good encapsulation practices since it's only used as a parameter type for the useSteps hook.
Let's verify there are no external dependencies on this type:
✅ Verification successful
✅ Safe to make UseStepsProps internal
The interface is only used within its own file (use-steps.ts) as a parameter type for the useSteps hook. No external code depends on this type, so removing the export is safe.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any external usage of UseStepsProps type
rg "UseStepsProps" --type ts --type tsx
Length of output: 67
Script:
#!/bin/bash
# Search for any external usage of UseStepsProps type, using ripgrep without file type filters
# as the previous attempt had issues with tsx extension
rg "UseStepsProps"
Length of output: 183
packages/web/hooks/window/use-window-size.ts (2)
Line range hint 3-9: LGTM! Good encapsulation of the WindowSize interface.
Making the WindowSize interface internal is a positive change as it's only used by the useWindowSize hook. This follows the principle of information hiding and reduces the public API surface.
Line range hint 1-58: LGTM! Well-structured hook with clear documentation.
The hook is well-implemented with:
- Clear documentation linking to Tailwind config
- Proper cleanup of event listeners
- Type-safe breakpoint handling
- Comprehensive window size tracking including mobile and desktop breakpoints
packages/web/components/table/types.ts (2)
14-17: LGTM! Good encapsulation practice.
Making ColumnSortDef non-exported is appropriate since it's only used internally by the ColumnDef interface. This change helps reduce the public API surface area and follows the principle of information hiding.
14-17: Verify no external usage of ColumnSortDef.
Let's ensure this change doesn't break any existing imports.
✅ Verification successful
ColumnSortDef interface changes are safe to proceed
The interface is only used internally within the table/types.ts file:
- Defined as an interface in the file itself
- Used as an optional property in another type definition within the same file
- No external imports or extensions of this interface were found in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any external usage of ColumnSortDef that might break
# Expected: No direct imports of ColumnSortDef should exist outside this file
# Search for any imports of ColumnSortDef
rg "import.*ColumnSortDef.*from.*table/types" --type ts
Length of output: 57
Script:
#!/bin/bash
# Let's try a more comprehensive search approach
# Search for any usage of ColumnSortDef in the codebase
rg "ColumnSortDef" --type ts -A 2
# Also search for type imports from table/types
rg "import.*type.*from.*table/types" --type ts
# Search for any usage in type declarations
ast-grep --pattern "type $_ = ColumnSortDef"
ast-grep --pattern "interface $_ extends ColumnSortDef"
Length of output: 657
packages/web/components/complex/portfolio/cypher-card.tsx (1)
2-2: LGTM! Good choice of persistence mechanism.
The useLocalStorage hook from react-use is a reliable choice for persisting user preferences.
packages/web/components/complex/portfolio/allocation-tabs.tsx (1)
7-8: LGTM! Appropriate scoping of the interface.
The removal of the export keyword from AllocationTabProps is correct as this interface is only used internally by the AllocationTabs component. This change aligns with the PR's objective to clean up unnecessary exports.
Let's verify that this interface isn't used elsewhere in the codebase:
✅ Verification successful
Removal of export keyword is correct - interface is only used locally
The verification confirms that AllocationTabProps is only referenced in packages/web/components/complex/portfolio/allocation-tabs.tsx and is not imported or used anywhere else in the codebase. This validates that removing the export keyword is the right decision as the interface is purely for internal use within this component file.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that AllocationTabProps isn't imported or used in other files
# Expect: No matches outside this file
rg "AllocationTabProps" --type ts --type tsx
Length of output: 72
Script:
#!/bin/bash
# Let's try a different approach using ripgrep without file type filters
# and manually filter the results
rg "AllocationTabProps" -l
Length of output: 88
Script:
#!/bin/bash
# As a backup, let's also use ast-grep to find any type references
# This will catch TypeScript interfaces and type aliases
ast-grep --pattern 'interface AllocationTabProps'
Length of output: 51
Script:
#!/bin/bash
# Let's also check for any imports of this type
ast-grep --pattern 'import { $$$AllocationTabProps$$$ }'
Length of output: 58
packages/web/components/swap-tool/swap-tool-tabs.tsx (1)
12-15: LGTM! Good improvement in type visibility.
Making SwapToolTabsProps internal is a positive change that:
- Reduces the public API surface area
- Maintains type safety through the exported component
- Follows TypeScript best practices for interface visibility
packages/web/hooks/use-dimension.ts (2)
7-12: LGTM! Good encapsulation of internal types.
The removal of export keywords from these type declarations is a positive change that:
- Reduces the public API surface area
- Makes the module more maintainable as these types can be modified without affecting consumers
- Properly encapsulates implementation details
7-12: Verify removal of type exports doesn't break consumers.
While encapsulating these types is good practice, let's verify that no other modules were depending on these type declarations.
✅ Verification successful
Removing type exports is safe - no external usage found
The search results confirm that these types (UseDimensionRect, UseDimensionRef, and UseDimensionResult) are only used within the use-dimension.ts file itself and have no external consumers. Encapsulating these types as internal implementation details is a safe refactoring.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any usage of these types outside this file
# This will help identify if any modules were depending on these exported types
echo "Searching for UseDimensionRect usage..."
rg "UseDimensionRect" --type typescript --glob '!packages/web/hooks/use-dimension.ts'
echo "Searching for UseDimensionRef usage..."
rg "UseDimensionRef" --type typescript --glob '!packages/web/hooks/use-dimension.ts'
echo "Searching for UseDimensionResult usage..."
rg "UseDimensionResult" --type typescript --glob '!packages/web/hooks/use-dimension.ts'
Length of output: 632
Script:
#!/bin/bash
# Let's try with a different approach using ripgrep without file type filtering
# and manually filter typescript/javascript files
echo "Searching for UseDimensionRect usage..."
rg "UseDimensionRect" --glob '!packages/web/hooks/use-dimension.ts' --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx'
echo "Searching for UseDimensionRef usage..."
rg "UseDimensionRef" --glob '!packages/web/hooks/use-dimension.ts' --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx'
echo "Searching for UseDimensionResult usage..."
rg "UseDimensionResult" --glob '!packages/web/hooks/use-dimension.ts' --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx'
Length of output: 1378
packages/web/components/transactions/recent-activity/recent-activity-transaction-row.tsx (1)
Line range hint 8-18: LGTM: RecentActivityRow component scope change is appropriate.
The component serves as an internal layout wrapper used only by SwapRow. Making it locally scoped:
- Reduces the public API surface
- Makes the code more maintainable by clearly showing component boundaries
- Follows the principle of least privilege
Let's verify this component isn't imported elsewhere:
#!/bin/bash
# Search for any imports of RecentActivityRow
rg "import.*RecentActivityRow" --type typescript --type typescriptreactpackages/web/hooks/mutations/one-click-trading/use-remove-one-click-trading-session.ts (1)
Line range hint 9-13: LGTM! Appropriate scope reduction.
The removal of the export keyword is correct as this type is only used internally within this module. This change aligns with the principle of minimizing the public API surface area.
Let's verify this type isn't used elsewhere in the codebase:
✅ Verification successful
Removing export keyword is correct - type is only used internally
The verification confirms that UseRemoveOneClickTradingMutationOptions is only used within its own file (use-remove-one-click-trading-session.ts), appearing exactly twice:
- In the type declaration
- As a parameter type in the same file
This validates that removing the export keyword was the right decision as the type is indeed only used internally.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that UseRemoveOneClickTradingMutationOptions is only used in this file
# Expect: Only matches in this file
rg "UseRemoveOneClickTradingMutationOptions"
Length of output: 343
packages/web/components/chart/light-weight-charts/chart-controller.ts (1)
Line range hint 30-34: LGTM! Appropriate scope restriction for ChartControllerEvents.
The removal of the export keyword is appropriate as this type is only used internally by the ChartController class for event handling.
Let's verify there are no external dependencies on this type:
✅ Verification successful
LGTM! Safe to remove export from ChartControllerEvents type
The verification confirms that ChartControllerEvents is only used internally within chart-controller.ts. While there are several files importing from chart-controller.ts, none of them reference this type. The type is only used as a generic parameter for the internal EventEmitter instance.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any external usage of ChartControllerEvents
# Expected: No results outside this file
# Search for direct references to ChartControllerEvents
rg "ChartControllerEvents" --type ts --glob '!**/chart-controller.ts'
Length of output: 69
Script:
#!/bin/bash
# Let's verify with a more comprehensive search strategy
# 1. First, get the exact path of the file
fd "chart-controller.ts"
# 2. Search for any imports from this file
rg -l "from.*chart-controller" --type ts
# 3. Search for any potential usage of the type even without direct import
# This could catch cases where the type is used through re-exports
rg "ChartControllerEvents[<\s]" --type ts
Length of output: 718
packages/web/components/earn/filters/filter-context.tsx (2)
Line range hint 32-45: LGTM! Well-structured context implementation.
The FilterContext implementation follows React best practices with proper typing and default values.
Line range hint 54-59: Verify search parameter sanitization.
The component directly uses the search parameter from the router query. Ensure that proper input sanitization is in place before using this value in filters.
✅ Verification successful
Search parameter handling is secure
The search parameter implementation is safe because:
- Type checking is performed before usage:
typeof search === "string" - Empty string validation is in place:
search !== "" - The search value is only used for filtering and not rendered directly in HTML
- No dangerous DOM manipulation methods (
innerHTML,dangerouslySetInnerHTML) are used - The filter context is properly encapsulated with React's state management
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for input sanitization in related files
echo "Checking for input sanitization..."
rg -l "sanitize|escape|validate" "packages/web/components/earn"
# Check for XSS prevention patterns
echo "Checking for potential XSS vectors..."
rg -l "dangerouslySetInnerHTML|innerHTML" "packages/web/components/earn"
Length of output: 347
Script:
#!/bin/bash
# Let's check how the search parameter is used in the filter context and related components
echo "Checking search parameter usage..."
rg -A 5 "search.*router\.query" packages/web/components/earn
# Check the rewards component for validation logic
echo "Checking rewards component..."
cat packages/web/components/earn/rewards/index.tsx
# Look for filter value processing
echo "Checking filter processing..."
rg -A 5 "setFilters|defaultFilters" packages/web/components/earn
Length of output: 10839
packages/web/hooks/use-controllable-state.ts (2)
Line range hint 12-89: Excellent documentation and implementation!
The hook is well-documented with:
- Clear purpose and behavior explanation
- Practical examples for both controlled and uncontrolled usage
- Type-safe implementation with proper React hooks usage
- Performance optimization through optional
shouldUpdatepredicate
Line range hint 5-10: LGTM: Interface visibility change aligns with encapsulation principles.
Making UseControllableStateProps internal is a good practice since it's only used by the useControllableState hook.
Let's verify that this interface isn't used externally:
✅ Verification successful
Interface visibility change is safe - no external usage found
The verification confirms that UseControllableStateProps is only used within packages/web/hooks/use-controllable-state.ts - once for the interface declaration and once for the hook parameter. Making it internal is a safe refactoring that improves encapsulation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any external usage of UseControllableStateProps
# This should only find references within this file
rg "UseControllableStateProps" --type ts
Length of output: 209
packages/web/components/ad-banner/ad-banner.tsx (1)
10-12: Consider documenting the interaction pattern choices.
The Ad type now supports both onClick handlers and externalUrl for interactions. Consider adding JSDoc comments to document when to use each approach and their intended use cases.
Let's verify the usage patterns of these interaction methods across the codebase:
packages/web/components/control/language-select.tsx (1)
12-14: LGTM: Appropriate encapsulation of internal type.
The removal of the export keyword from LanguageSelectProps is a good practice since this type is only used internally within this file.
packages/web/components/complex/pool/create/select-type.tsx (2)
Line range hint 21-107: LGTM! Well-structured component with good practices.
The component demonstrates good React practices:
- Proper use of hooks (useState, useCallback)
- Type safety with TypeScript
- Clean separation of concerns
- Efficient rendering with proper button state management
Line range hint 13-19: Verify removal of exported types doesn't break external dependencies.
The change to make PoolType and PoolTypeConfig locally scoped aligns with good encapsulation practices. However, let's verify no other components depend on these types.
✅ Verification successful
Making types local is safe - no external dependencies found
The search results confirm that while PoolType is used throughout the codebase, these usages refer to different type definitions (from various modules like ObservableQueryPool, Pool, etc.). The local PoolType in this component is just using the type from ObservableQueryPool and not exporting a new type.
The PoolTypeConfig interface is not used anywhere else in the codebase, confirming it's safe to keep it as a local type.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any external usage of these types
echo "Checking for PoolType usage..."
rg "PoolType" --type typescript --glob '!**/select-type.tsx'
echo "Checking for PoolTypeConfig usage..."
rg "PoolTypeConfig" --type typescript --glob '!**/select-type.tsx'
Length of output: 351
Script:
#!/bin/bash
# Search for any external usage of these types with correct file extension
echo "Checking for PoolType usage..."
rg "PoolType" --type ts,tsx --glob '!**/select-type.tsx'
echo "Checking for PoolTypeConfig usage..."
rg "PoolTypeConfig" --type ts,tsx --glob '!**/select-type.tsx'
Length of output: 335
Script:
#!/bin/bash
# Search for any external usage of these types without file type filtering
echo "Checking for PoolType usage..."
rg "PoolType" --glob '!**/select-type.tsx'
echo "Checking for PoolTypeConfig usage..."
rg "PoolTypeConfig" --glob '!**/select-type.tsx'
Length of output: 7708
packages/web/__tests__/test-utils.tsx (3)
5-5: LGTM: Import refactoring improves type organization
The change to import AvailableFlags from @osmosis-labs/types instead of an internal path indicates better type organization at the package level.
21-21: LGTM: Appropriate scoping of TRPC client
Converting trpcReact to a local constant improves encapsulation as it's only used within this file's context.
Line range hint 25-57: LGTM: Appropriate scoping of provider wrapper
Converting withTRPC to a local function improves encapsulation as it's only used internally by the render helper functions.
packages/web/components/alert/cypher-card-toast.tsx (1)
14-15: LGTM: Appropriate scoping of localStorage key constant
The conversion of CypherCardFloatingBannerDoNotShowKey to a local constant is correct as it's only used within this component.
packages/web/components/earn/table/utils.ts (1)
Line range hint 135-150: LGTM! Good encapsulation of internal utility.
The removal of the export keyword from _getKey is appropriate since:
- The function is only used internally by
getDefaultFiltersState - The underscore prefix already indicates it's a private implementation detail
Let's confirm this function isn't imported elsewhere:
✅ Verification successful
Removal of export from _getKey is safe and appropriate ✅
The verification confirms that _getKey is only used internally within utils.ts:
- It appears only in two locations, both within the same file
- One occurrence is its definition
- The other is its usage within the same file for mapping filter keys
- No external imports or usages were found in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify _getKey isn't imported outside this file
# Expect: No matches as it should only be used internally
rg "import.*_getKey.*from.*earn/table/utils"
Length of output: 46
Script:
#!/bin/bash
# Let's verify more thoroughly with multiple approaches:
# 1. Check for any usage of _getKey in the codebase
echo "=== Searching for _getKey usage ==="
rg "_getKey" -A 2
# 2. Check the specific file to understand the context better
echo -e "\n=== Checking utils.ts content ==="
rg "getKey" packages/web/components/earn/table/utils.ts -A 2
# 3. Look for any potential imports with different naming
echo -e "\n=== Checking for potential renamed imports ==="
rg "from.*earn/table/utils.*}" -A 1
Length of output: 937
packages/web/components/screen-manager.tsx (2)
Line range hint 9-14: LGTM! Good encapsulation of internal interface.
The removal of the export keyword from ScreenManagerState is a positive change as this interface is an implementation detail that should only be used internally. External consumers already have access to the typed context through useScreenManager hook.
Line range hint 1-156: Verify no external dependencies on the previously exported interface.
The components and their implementations look good. Let's verify that no external code was depending on the previously exported ScreenManagerState interface.
✅ Verification successful
Interface visibility change is safe to proceed
The search results confirm that ScreenManagerState interface is only referenced within its own file (screen-manager.tsx). External components are only importing and using the exported components (Screen, ScreenManager, useScreenManager) and not the interface itself, making this visibility change safe.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any external usage of ScreenManagerState type/interface
# This will help ensure the visibility change doesn't break any existing code
# Look for direct type imports or interface extensions
rg -l "import.*\{.*ScreenManagerState.*\}" --type ts --type tsx
rg -l "extends\s+ScreenManagerState\b" --type ts --type tsx
# Look for explicit type annotations using ScreenManagerState
rg -l ":\s*ScreenManagerState\b" --type ts --type tsx
Length of output: 267
Script:
#!/bin/bash
# Description: Search for any external usage of ScreenManagerState type/interface
# Using different file type flags for TypeScript/React files
# Look for direct type imports
rg -t typescript -t typescriptreact "import.*\{.*ScreenManagerState.*\}"
# Look for interface extensions
rg -t typescript -t typescriptreact "extends\s+ScreenManagerState\b"
# Look for explicit type annotations
rg -t typescript -t typescriptreact ":\s*ScreenManagerState\b"
# Look for any other references to ScreenManagerState
rg -t typescript -t typescriptreact "ScreenManagerState"
Length of output: 407
Script:
#!/bin/bash
# Search for any usage of ScreenManagerState across all files
# Using simpler ripgrep commands without file type filters
# Look for the interface name across all files
rg "ScreenManagerState" --type-not md --type-not json
# Also search for any potential imports of the type
rg "from.*screen-manager.*" --type-not md --type-not json
Length of output: 2016
packages/web/components/complex/portfolio/allocation.tsx (2)
Line range hint 1-61: LGTM: Clean prop interface simplification
The removal of the hideDust prop in favor of the useHideDustUserSetting hook is a good architectural decision as it:
- Centralizes dust filtering logic
- Reduces prop drilling
- Makes the component's interface cleaner
61-61: Verify the impact of removing hideDust prop
The removal of the hideDust prop might affect parent components that were previously passing this prop.
packages/web/components/input/input-box.tsx (1)
13-13: LGTM! Verify no external dependencies on this interface.
The removal of the export keyword from the Button interface aligns with the PR's objective of improving visibility scopes. This change enhances encapsulation as the interface appears to be used only internally within this file.
Let's verify there are no external dependencies on this interface:
packages/web/components/trade-tool/index.tsx (4)
Line range hint 19-24: LGTM: Appropriate scoping of TradeToolProps interface.
The removal of the export keyword is a good change as this interface is only used internally by the TradeTool component. This aligns with the principle of minimal exports and the PR's objective of fixing unused exports.
8-8: Verify removal of AltSwapTool component.
The import change suggests AltSwapTool has been removed. Let's verify this removal doesn't affect other parts of the codebase.
✅ Verification successful
AltSwapTool removal is safe
The search results confirm that AltSwapTool and its source file alt.tsx have been completely removed from the codebase with no remaining references or imports. This change appears to be safe as part of the "Knip fix" cleanup.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to AltSwapTool
rg "AltSwapTool"
# Search for any remaining imports from alt.tsx
rg -g "*.{ts,tsx}" "from ['\"]~/components/swap-tool/alt['\"]"
Length of output: 275
Script:
#!/bin/bash
# Search for AltSwapTool in the codebase
rg "AltSwapTool" --type ts --type tsx
# Check if swap-tool/alt.tsx file exists
fd "alt.tsx" packages/web/components/swap-tool
# Look for any file importing from swap-tool/alt
rg "from.*swap-tool/alt" --type ts --type tsx
Length of output: 188
Script:
#!/bin/bash
# Search for AltSwapTool in any file
rg "AltSwapTool"
# Check if alt.tsx exists anywhere in the repository
fd "alt.tsx"
# Look for any imports from alt
rg "from.*alt['\"]"
Length of output: 53
61-61: Verify removal of ClientOnly wrapper.
The replacement of ClientOnly with React fragments could potentially cause hydration mismatches if the component renders different content on server vs client. Please ensure this change has been tested for SSR compatibility.
Also applies to: 163-163
✅ Verification successful
Removal of ClientOnly wrapper appears safe
The trade-tool component doesn't contain any browser-specific APIs (window, document, localStorage) that would cause hydration mismatches. The only useEffect found is for handling prop changes, which is safe for SSR. The ClientOnly component was likely unnecessary here as it was wrapping content that doesn't need client-side only rendering.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other uses of ClientOnly to understand the pattern
rg "ClientOnly" -A 3
# Look for potential SSR-sensitive code in the component
ast-grep --pattern 'typeof window !== "undefined"'
ast-grep --pattern 'process.browser'
Length of output: 275
Script:
#!/bin/bash
# Search for ClientOnly usage patterns with better context
rg -g "*.tsx" -g "*.ts" "ClientOnly" -B 2 -A 2
# Look for window/document usage which might need client-side only rendering
rg -g "*.tsx" -g "*.ts" "window\.|document\." packages/web/components/trade-tool/
# Check for any imports or definitions of ClientOnly
rg -g "*.tsx" -g "*.ts" "import.*ClientOnly|export.*ClientOnly"
Length of output: 7074
Script:
#!/bin/bash
# Check the trade-tool component for any client-side specific code
rg -g "*.tsx" -g "*.ts" "useState|useEffect|window\.|document\.|localStorage\.|sessionStorage" packages/web/components/trade-tool/
# Check implementation of ClientOnly component
cat packages/web/components/client-only.tsx
# Look for hydration related code or comments in trade-tool
rg -g "*.tsx" -g "*.ts" "hydrat|SSR|server-side" packages/web/components/trade-tool/
Length of output: 778
Line range hint 113-127: Verify new SwapTool props implementation.
The addition of useOtherCurrencies and useQueryParams props suggests new functionality. Please ensure:
- These props are properly documented in the SwapTool component
- The behavior differences when these props are enabled/disabled are tested
✅ Verification successful
Props are well-implemented and documented
The verification shows that both props are properly implemented:
useOtherCurrenciescontrols whether users can select additional currencies for swapping beyond the default onesuseQueryParamsdetermines if token denominations should be managed via URL query parameters
The props are:
- Properly typed in
SwapToolPropsinterface - Well documented in
use-swap.tsxhook - Consistently used across different components (SwapTool, trade-tokens modal, limit orders)
- Have sensible defaults (both default to
truein the hook)
The implementation shows proper integration with URL state management and currency selection functionality, maintaining backward compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for documentation and prop types
rg -t typescript "useOtherCurrencies|useQueryParams" -A 5 "packages/web/components/swap-tool"
# Search for tests covering these props
rg -g "*test*" "useOtherCurrencies|useQueryParams"
Length of output: 275
Script:
#!/bin/bash
# Look for SwapTool component definition and its props
ast-grep --pattern 'interface SwapToolProps {
$$$
}'
# Search for documentation and implementation of these props
rg "useOtherCurrencies|useQueryParams" -A 5
# Look for test files
fd ".*test.*" --type f --exec rg "useOtherCurrencies|useQueryParams" {} \;
Length of output: 14538
🧰 Tools
🪛 Biome
[error] 110-110: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
packages/web/components/earn/table/columns.tsx (2)
Line range hint 66-234: Well-structured table configuration with type-safe columns.
The table configuration demonstrates good practices:
- Type-safe column definitions using
createColumnHelper - Proper separation of concerns between display and filtering columns
- Consistent use of custom cell components
- Well-organized sorting and filtering functions
Line range hint 219-229: Verify empty implementations.
Several columns have empty header and cell implementations. If these are intentionally empty for filtering purposes, consider adding a comment explaining their purpose:
columnHelper.accessor("platform", {
header: () => {}, // Empty header - column used only for filtering
cell: () => {}, // Empty cell - column used only for filtering
filterFn: listOptionValueEquals,
enableHiding: true,
}),✅ Verification successful
Let me gather more information about the columns implementation to verify if they are used for filtering purposes.
Add comments to explain filter-only columns
The verification confirms that these columns (depositAssets.coinDenom, platform, holdsTokens, hasLockingDuration) are indeed used only for filtering purposes, as they have empty header/cell implementations but have specific filter functions assigned. Adding comments as suggested would improve code clarity.
Example locations that need comments:
packages/web/components/earn/table/columns.tsx:depositAssets.coinDenomcolumnplatformcolumnholdsTokenscolumnhasLockingDurationcolumn
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usages of these columns to verify they're only used for filtering
rg -A 5 'depositAssets\.coinDenom|platform' packages/web/components/earn/
Length of output: 10904
Script:
#!/bin/bash
# Search for column definitions and their usages
rg -A 10 'columnHelper\.accessor\("(holdsTokens|hasLockingDuration|platform|depositAssets\.coinDenom)"' packages/web/components/earn/
# Search for filter functions used in these columns
rg 'listOptionValueEquals|boolEqualsString|lockDurationFilter' packages/web/components/earn/
Length of output: 2967
packages/web/components/chart/light-weight-charts/chart.tsx (2)
41-41: LGTM! Improved encapsulation by removing export.
The removal of the export keyword from defaultOptions is a positive change that better encapsulates the chart's default configuration within its module.
Line range hint 192-199: LGTM! Well-structured interface with proper encapsulation.
The interface is well-typed with appropriate generic constraints and the removal of export properly encapsulates it within the module.
packages/web/components/complex/portfolio/portfolio-page.tsx (5)
24-24: LGTM: Type import for enhanced type safety
The addition of HideDustState type import aligns with the migration to centralized user settings management.
32-34: LGTM: Clean store integration
The addition of userSettings and simplified wallet loading check improves code organization.
37-37: LGTM: Clean analytics implementation
The analytics setup is properly implemented with clear event tracking.
56-56: LGTM: Improved boolean check clarity
The addition of the Boolean wrapper enhances type safety and readability.
164-164: Verify internal dust handling in Allocation component
The hideDust prop has been removed from the Allocation component. Please ensure that the component now handles dust filtering internally using the useHideDustUserSetting hook as mentioned in the summary.
✅ Verification successful
Dust filtering is correctly handled internally in Allocation component
The verification confirms that the Allocation component now properly handles dust filtering internally using the useHideDustUserSetting hook. The implementation can be found in packages/web/components/complex/portfolio/allocation.tsx where it applies the dust filter to the selected list using:
const dustFilteredList = useHideDustUserSetting(
selectedList,
(item) => item.fiatValue
);🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for dust handling in Allocation component
ast-grep --pattern 'function Allocation($$$) {
$$$
useHideDustUserSetting
$$$
}'
Length of output: 88
Script:
#!/bin/bash
# Let's first locate the Allocation component file
fd "allocation" --type f
# Then examine its implementation for dust handling
rg "useHideDustUserSetting|hideDust" -A 5
Length of output: 11746
packages/web/components/assets/highlights-categories.tsx (2)
Line range hint 147-267: Implementation looks good!
The component implementation demonstrates:
- Good separation of concerns
- Proper type safety
- Appropriate event tracking
- Responsive design
- Accessible link handling
132-132: Verify removal of export.
The component is now locally scoped, which aligns with the PR's "Knip fix" objective. Let's verify that this component is not imported elsewhere.
✅ Verification successful
Removal of export is safe to proceed
The component AssetHighlights is only used internally within the highlights-categories.tsx file. The only export from this file is HighlightsCategories, which is correctly imported in asset-info.tsx. The removal of the export from AssetHighlights component aligns with the PR's "Knip fix" objective as it's now properly scoped as a local component.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for imports of AssetHighlights across the codebase
# Expect: No results as the component should not be imported elsewhere
rg "import.*AssetHighlights.*from.*highlights-categories"
Length of output: 59
Script:
#!/bin/bash
# Let's try a broader search to find any references to AssetHighlights
# First, let's check direct imports or references to AssetHighlights
rg "AssetHighlights" -A 2
# Also search for any imports from the highlights-categories file
rg "from ['\"].*highlights-categories['\"]" -A 2
Length of output: 1563
Script:
#!/bin/bash
# Let's check the content of asset-info.tsx to understand how HighlightsCategories is used
cat packages/web/components/table/asset-info.tsx
Length of output: 20620
packages/web/components/buttons/button.tsx (1)
7-7: LGTM! Good encapsulation of button styles.
The removal of the export keyword for buttonCVA is a positive change that:
- Reduces the public API surface
- Prevents external dependencies on internal styling implementation
- Maintains better encapsulation of the button's styling logic
packages/web/components/chart/concentrated-liquidity-depth.tsx (1)
17-20: LGTM! Encapsulating the DepthData type.
The removal of the export keyword aligns with good TypeScript practices by keeping the type scoped to this module where it's exclusively used.
Let's verify that this type isn't being imported elsewhere in the codebase:
✅ Verification successful
Confirmed: DepthData type is only used internally
The verification results show that DepthData is only used within concentrated-liquidity-depth.tsx and has no external dependencies. Removing the export keyword is the correct approach for better encapsulation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any imports of DepthData from this file
rg "import.*DepthData.*concentrated-liquidity-depth"
# Search for any direct usage of DepthData
rg "DepthData[^a-zA-Z]" --type ts --type tsx
Length of output: 129
Script:
#!/bin/bash
# Search for any imports of DepthData, accounting for different path formats
rg "import.*DepthData.*concentrated-liquidity-depth|from.*concentrated-liquidity-depth.*DepthData"
# Search for any direct usage of DepthData, using ripgrep's built-in type for TypeScript
rg "DepthData[^a-zA-Z]" -t ts
# Find all TypeScript/TSX files that might reference DepthData
fd "\.(ts|tsx)$" | xargs rg "DepthData"
Length of output: 1816
packages/web/components/main-menu.tsx (1)
84-85: LGTM! Clear and flexible type definition.
The MaybeRenderProp<P> type definition effectively combines both direct content and render prop patterns, providing flexibility while maintaining type safety. This follows TypeScript best practices for component props that can accept either direct content or a render function.
packages/web/components/table/index.tsx (1)
Line range hint 193-193: LGTM: Export statements are appropriate.
The export statements maintain a clean public API by exporting only the necessary types and components.
packages/web/hooks/ui-config/use-asset-info-config.ts (4)
193-195: LGTM: Appropriate encapsulation of internal types.
The removal of exports for AssetChartAvailableDataTypes and its type definition improves encapsulation, as these are only used internally within the module.
197-202: LGTM: Proper scoping of chart mode constants.
The removal of exports for AssetChartModes and its type definition is appropriate as these are implementation details of the chart configuration.
207-210: LGTM: Appropriate scoping of internal interface.
The removal of export for the ChartTick interface is correct as it represents an internal data structure used only within this module.
193-210: Verify removal of exports.
Let's confirm that these types are not used elsewhere in the codebase.
✅ Verification successful
Removal of exports is safe
The search results show that all types and constants are only used within the same file (packages/web/hooks/ui-config/use-asset-info-config.ts). There are no external references to these types in other files, confirming that removing the exports won't break any dependent code.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any external usage of the previously exported types
# This helps verify that the removal of exports won't break any dependent code
echo "Checking for potential usage of previously exported types..."
# Check for AssetChartAvailableDataTypes usage
rg "AssetChartAvailableDataTypes" --type ts --type tsx
# Check for AssetChartDataType usage
rg "AssetChartDataType" --type ts --type tsx
# Check for AssetChartModes usage
rg "AssetChartModes" --type ts --type tsx
# Check for AssetChartMode usage
rg "AssetChartMode" --type ts --type tsx
# Check for ChartTick interface usage
rg "ChartTick" --type ts --type tsx
Length of output: 489
Script:
#!/bin/bash
# Let's fix the ripgrep command to search without file type restrictions
# since the tsx extension wasn't recognized
echo "Checking for usage of previously exported types..."
# Search for each type, showing 2 lines of context before and after
echo "=== AssetChartAvailableDataTypes ==="
rg "AssetChartAvailableDataTypes" -B 2 -A 2
echo "=== AssetChartDataType ==="
rg "AssetChartDataType" -B 2 -A 2
echo "=== AssetChartModes ==="
rg "AssetChartModes" -B 2 -A 2
echo "=== AssetChartMode ==="
rg "AssetChartMode" -B 2 -A 2
echo "=== ChartTick ==="
rg "ChartTick" -B 2 -A 2
Length of output: 6179
packages/web/components/swap-tool/split-route.tsx (1)
1-24: LGTM! Clean up of unused imports.
The removal of unused imports and the organization of type definitions look good. The remaining imports are all actively used in the components.
packages/web/components/stepper/index.tsx (1)
347-347: LGTM! Export cleanup aligns with Knip recommendations.
The removal of the useStepperContext export is a good practice as it prevents external components from directly accessing the Stepper's internal context. This encapsulation ensures that the Stepper's state management remains controlled through the provided components only.
Let's verify that useStepperContext is not used outside this file:
✅ Verification successful
Export cleanup is safe - useStepperContext is only used internally
The verification confirms that useStepperContext is only used within the stepper component file itself (packages/web/components/stepper/index.tsx). There are no external usages across the codebase, making its removal from exports a safe change that improves encapsulation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any external usage of useStepperContext
rg "useStepperContext" --type ts --type tsx
Length of output: 71
Script:
#!/bin/bash
# Search for any external usage of useStepperContext with ripgrep
# Note: Using --type-add to properly handle tsx files
rg --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' "useStepperContext" -g '*.{ts,tsx}'
Length of output: 585
packages/web/components/alert/toast.tsx (2)
17-20: LGTM: Proper encapsulation of ToastOptions type
The removal of the export modifier aligns with the PR's objective of fixing Knip-related issues while maintaining the type's functionality within the module.
Line range hint 224-231: Verify local storage error handling
The component uses useLocalStorage hook but doesn't handle potential storage errors. Consider adding error handling for cases where local storage might be unavailable or full.
packages/web/config/utils.ts (3)
Line range hint 1-308: LGTM: Export visibility changes improve code organization.
The changes align well with the PR's objective of fixing unused exports ("Knip fix"). The remaining exported functions serve clear purposes:
getOsmosisChainId: Chain identificationgetImageRelativeFilePath: Public image path resolutionsaveAssetImageToTokensDir: Asset managementgetChainList: Chain configuration aggregation
The encapsulation of internal utility functions improves the module's API surface.
Line range hint 93-308: LGTM: Function visibility change is appropriate.
The removal of the export modifier is correct as this function is only used internally by getChainList.
Let's verify there are no external usages of this function:
✅ Verification successful
LGTM: Function visibility change is appropriate
The search results confirm that getKeplrCompatibleChain is only used internally within the same file by the exported getChainList function. The removal of the export modifier is safe and aligns with proper encapsulation principles.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any external usage of getKeplrCompatibleChain
rg "getKeplrCompatibleChain" --type ts --type tsx
Length of output: 77
Script:
#!/bin/bash
# Search for any usage of getKeplrCompatibleChain with proper file extensions
rg "getKeplrCompatibleChain" -t ts -t js -t jsx -t tsx
# Also search for exports of this function to be thorough
rg "export.*getKeplrCompatibleChain" -t ts -t js -t jsx -t tsx
Length of output: 175
Script:
#!/bin/bash
# Search for any usage of getKeplrCompatibleChain in the entire codebase
rg "getKeplrCompatibleChain" packages/
# Also check the file itself to see how it's used
cat packages/web/config/utils.ts
Length of output: 12372
Line range hint 33-39: LGTM: Function visibility change and path update.
The removal of the export modifier is appropriate as this function is only used internally by saveAssetImageToTokensDir. The addition of "/public" prefix aligns with web conventions for static asset storage.
Let's verify there are no external usages of this function:
✅ Verification successful
LGTM: Function visibility change is safe
The getNodeImageRelativeFilePath function is only used internally within saveAssetImageToTokensDir in the same file. The removal of the export modifier is appropriate as there are no external usages of this function across the codebase. The function maintains its core functionality while properly handling public asset paths.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any external usage of getNodeImageRelativeFilePath
rg "getNodeImageRelativeFilePath" --type ts --type tsx
Length of output: 82
Script:
#!/bin/bash
# Search for any usage of getNodeImageRelativeFilePath with proper file extensions
rg "getNodeImageRelativeFilePath" -t ts -t js -t jsx -t tsx
# Also search with ast-grep to catch any potential renamed imports
ast-grep --pattern 'import { $_ as $_ } from "$_"' | rg -i "getNodeImageRelativeFilePath"
Length of output: 177
Script:
#!/bin/bash
# Search for any usage of getNodeImageRelativeFilePath in the entire codebase
# Using simpler ripgrep command without file type filters
rg "getNodeImageRelativeFilePath"
# Let's also check the content of the utils.ts file to see internal usage
cat packages/web/config/utils.ts
Length of output: 12428
packages/web/hooks/limit-orders/use-orderbook.ts (3)
3-3: LGTM: Appropriate scope reduction
The removal of unused exports and internal type definitions aligns with the PR's objective of cleaning up the public API surface. The useOrderbooks hook is correctly kept internal as it's only used within this module.
Also applies to: 19-19
Line range hint 297-298: Review cache configuration
Setting cacheTime equal to refetchInterval might cause unnecessary cache invalidation. Consider setting cacheTime to a larger value to prevent data from being immediately evicted from the cache after a refetch.
Line range hint 1-445: Overall implementation looks solid
The codebase demonstrates good React hooks practices with proper separation of concerns, efficient data fetching strategies, and feature flag support for different query implementations. The reduction in public API surface through removal of unnecessary exports aligns well with the PR's objectives.
packages/web/components/swap-tool/trade-details.tsx (2)
Line range hint 350-364: LGTM: Clean implementation of the Closer utility component
The component correctly handles its single responsibility of automatically closing trade details when the input is empty. The removal of the export keyword aligns with the component's scope.
Line range hint 489-575: LGTM: Well-structured route handling implementation
The code demonstrates good practices:
- Clear type definitions using RouterOutputs
- Proper memoization with useMemo
- Smart handling of loading states with usePreviousWhen
packages/web/components/table/asset-info.tsx (1)
620-620: LGTM: Improved encapsulation by removing export
The removal of the export declaration for AssetActionsCell is a good change as the component is only used internally within this file. This change reduces the public API surface and improves encapsulation.
packages/web/components/bridge/bridge-wallet-select-modal.tsx (2)
98-98: LGTM: Improved encapsulation by making component local
The change from an exported to a local constant improves encapsulation since BridgeWalletSelectScreens is only used internally within this file.
Line range hint 425-516: LGTM: Well-implemented address validation form
The SendToAnotherAddressForm component has good validation logic and user feedback for different blockchain address types.
packages/web/components/bridge/deposit-address-screen.tsx (1)
Line range hint 67-81: Consider enhancing error handling visualization
While the useQuery hook is configured with useErrorBoundary: true, there's no visible error state UI for failed queries. Consider adding a fallback UI component to handle and display error states gracefully.
Let's check if there's an error boundary component in the codebase:
Consider implementing an error boundary component if one doesn't exist:
interface ErrorFallbackProps {
error: Error;
resetErrorBoundary: () => void;
}
const ErrorFallback: FunctionComponent<ErrorFallbackProps> = ({
error,
resetErrorBoundary,
}) => (
<div className="flex flex-col items-center gap-4 p-4">
<Icon id="alert-triangle" className="text-rust-400" />
<p className="text-osmoverse-200">{error.message}</p>
<Button onClick={resetErrorBoundary}>
{t("transfer.tryAgain")}
</Button>
</div>
);packages/web/components/table/portfolio-asset-balances.tsx (1)
30-30: LGTM: Good use of centralized dust filtering.
The addition of useHideDustUserSetting hook promotes code reuse and ensures consistent dust filtering behavior across components.
packages/web/components/navbar/index.tsx (2)
55-55: LGTM: Title handling improvements
The changes to title prop handling are well-structured:
- Making the title optional provides more flexibility
- Using navBarStore as the primary source maintains a single source of truth
- Conditional rendering prevents unnecessary DOM elements
Also applies to: 61-67, 164-164, 225-225
170-170: Verify the new background color's accessibility
The background color has been changed to use bg-osmoverse-1000. Let's verify this color exists in the theme and maintains proper contrast with the content.
✅ Verification successful
Background color change is valid and maintains proper contrast
The change from bg-osmoverse-900 to bg-osmoverse-1000 is valid as:
- The color exists in the theme configuration (
osmoverse-1000: "#090524") - It's consistently used across the application for dark backgrounds (found in 19 other locations)
- The navbar content uses high-contrast text colors:
text-white-high(95% opacity white)text-osmoverse-200(light gray)- Other elements use proper contrasting colors like
text-osmoverse-400
The color change appears to be part of a cohesive design system and maintains accessibility standards.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the new background color exists in the theme configuration
# and look for other usages of this color class
# Check theme configuration for the color
echo "Checking theme configuration for osmoverse-1000:"
rg -A 2 "osmoverse.*1000" tailwind.config.*
# Look for other usages of this background class
echo "Checking other usages of bg-osmoverse-1000:"
rg "bg-osmoverse-1000"
Length of output: 4044
Script:
#!/bin/bash
# Let's check the theme configuration and color definitions
# First, let's find the tailwind config file
echo "Looking for tailwind config files:"
fd -e js -e ts tailwind.config
# Then, let's look for any color definitions in the styles directory
echo -e "\nChecking color definitions in styles:"
fd . packages/web/styles -e css -e scss | xargs rg -A 3 "osmoverse.*1000|colors?.*1000"
# Finally, let's check the content that appears on this navbar to verify contrast
echo -e "\nChecking content within navbar:"
rg -A 5 "className.*bg-osmoverse-1000.*navbar" packages/web/components/navbar/index.tsx
Length of output: 535
Script:
#!/bin/bash
# Let's check the tailwind config file content and navbar content
# Check the tailwind config content
echo "Checking tailwind config content:"
cat packages/web/tailwind.config.js
# Check the full navbar component to understand the content and color usage
echo -e "\nChecking navbar component content:"
cat packages/web/components/navbar/index.tsx
Length of output: 33705
packages/web/components/layouts/main.tsx (2)
4-8: LGTM: Clean type imports
The explicit type imports using the 'type' keyword follow TypeScript best practices for better type-only import handling.
40-46: LGTM: Effective memoization of menu item selection
Good performance optimization using useMemo with appropriate dependencies. This prevents unnecessary recalculations of the selected menu item when unrelated state changes occur.
packages/web/hooks/limit-orders/use-place-limit.ts (2)
Line range hint 40-53: LGTM: Interface encapsulation and structure.
The interface is well-structured with clear parameter types and sensible defaults. The removal of the export keyword appropriately encapsulates the interface for internal use.
Line range hint 1-766: Overall implementation is robust and well-structured.
The code demonstrates good error handling, comprehensive validation, and proper state management. While there are opportunities for improvement in terms of code organization and type safety, the current implementation is solid and maintainable.
| ? getValueOfItem(item) | ||
| ?.toDec() | ||
| .gte(HideDustUserSetting.DUST_THRESHOLD) ?? true |
There was a problem hiding this comment.
💡 Codebase verification
The DUST_THRESHOLD value has changed from 0.01 to 0.02
The verification reveals that the DUST_THRESHOLD constant in HideDustUserSetting is set to new Dec(0.02), which is different from the previous hardcoded value of 0.01. This change will result in more aggressive dust filtering as items with values between 0.01 and 0.02 will now be considered dust.
🔗 Analysis chain
Verify the DUST_THRESHOLD constant value
The logic is correct, but let's verify that HideDustUserSetting.DUST_THRESHOLD maintains the same threshold value to avoid unintended changes in filtering behavior.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the DUST_THRESHOLD value in user settings
# Expected: Should find the constant declaration with its value
rg -A 1 "DUST_THRESHOLD.*=" --type ts
Length of output: 243
| @@ -3,9 +3,9 @@ import { FunctionComponent } from "react"; | |||
| import { FallbackImg, Icon } from "~/components/assets"; | |||
| import { TransactionRow } from "~/components/transactions/transaction-row"; | |||
| import { formatFiatPrice } from "~/utils/formatter"; | |||
| export type TransactionStatus = "pending" | "success" | "failed"; | |||
| type TransactionStatus = "pending" | "success" | "failed"; | |||
There was a problem hiding this comment.
💡 Codebase verification
Warning: TransactionStatus type is duplicated and should be shared
The type TransactionStatus is defined identically in two files:
packages/web/components/transactions/transaction-row.tsxpackages/web/components/transactions/recent-activity/recent-activity-transaction-row.tsx
This duplication should be avoided by:
- Moving the type to a shared location (e.g., a types file)
- Exporting it for reuse across these components
🔗 Analysis chain
LGTM: TransactionStatus type scope change is appropriate.
The type is only used within this file by RecentActivityRow and SwapRow, making the local scope more appropriate.
Let's verify this type isn't needed elsewhere:
Note: Let me correct the file extension flags for ripgrep to properly search TypeScript files.
Note: Let's try without file type filters as the environment might not have the TypeScript file type configurations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any other uses of TransactionStatus type
rg "TransactionStatus" --type typescript --type typescriptreact
Length of output: 108
Script:
#!/bin/bash
# Search for any other uses of TransactionStatus type with correct file extensions
rg "TransactionStatus" -t ts -t tsx
Length of output: 63
Script:
#!/bin/bash
# Search for TransactionStatus using basic ripgrep without file type filters
rg "TransactionStatus"
Length of output: 1956
| const hideDustSettingStore = | ||
| userSettings.getUserSettingById<HideDustState>("hide-dust"); | ||
| const hideDust = Boolean(hideDustSettingStore?.state?.hideDust); |
There was a problem hiding this comment.
Consider adding null safety to settings access
The current implementation might be unsafe if settings are not properly initialized.
Consider this safer implementation:
- const hideDustSettingStore =
- userSettings.getUserSettingById<HideDustState>("hide-dust");
- const hideDust = Boolean(hideDustSettingStore?.state?.hideDust);
+ const hideDustSettingStore = userSettings.getUserSettingById<HideDustState>("hide-dust");
+ const hideDust = Boolean(hideDustSettingStore?.state?.hideDust ?? false);📝 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.
| const hideDustSettingStore = | |
| userSettings.getUserSettingById<HideDustState>("hide-dust"); | |
| const hideDust = Boolean(hideDustSettingStore?.state?.hideDust); | |
| const hideDustSettingStore = userSettings.getUserSettingById<HideDustState>("hide-dust"); | |
| const hideDust = Boolean(hideDustSettingStore?.state?.hideDust ?? false); |
| ) : userHasNoAssets ? ( | ||
| <UserZeroBalanceTableSplash /> | ||
| ) : ( | ||
| <TabPanels> | ||
| <TabPanel> | ||
| <PortfolioAssetBalancesTable | ||
| hideDust={Boolean(hideDust)} | ||
| setHideDust={setHideDust} | ||
| setHideDust={(hideDust) => | ||
| hideDustSettingStore?.setState({ hideDust }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider simplifying the hideDust setter implementation
The current implementation could be more concise while maintaining functionality.
Consider this simplified implementation:
<PortfolioAssetBalancesTable
hideDust={Boolean(hideDust)}
- setHideDust={(hideDust) =>
- hideDustSettingStore?.setState({ hideDust })
- }
+ setHideDust={(value) => hideDustSettingStore?.setState({ hideDust: value })}
tableTopPadding={overviewHeight + tabsHeight}
/>📝 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.
| ) : userHasNoAssets ? ( | |
| <UserZeroBalanceTableSplash /> | |
| ) : ( | |
| <TabPanels> | |
| <TabPanel> | |
| <PortfolioAssetBalancesTable | |
| hideDust={Boolean(hideDust)} | |
| setHideDust={setHideDust} | |
| setHideDust={(hideDust) => | |
| hideDustSettingStore?.setState({ hideDust }) | |
| } | |
| ) : userHasNoAssets ? ( | |
| <UserZeroBalanceTableSplash /> | |
| ) : ( | |
| <TabPanels> | |
| <TabPanel> | |
| <PortfolioAssetBalancesTable | |
| hideDust={Boolean(hideDust)} | |
| setHideDust={(value) => hideDustSettingStore?.setState({ hideDust: value })} |
| @@ -8,7 +8,7 @@ import { formatFiatPrice, formatPretty } from "~/utils/formatter"; | |||
|
|
|||
| import { Spinner } from "../loaders"; | |||
|
|
|||
| export type TransactionStatus = "pending" | "success" | "failed"; | |||
| type TransactionStatus = "pending" | "success" | "failed"; | |||
There was a problem hiding this comment.
💡 Codebase verification
Making TransactionStatus local is problematic - it's used in other components
The change to make TransactionStatus locally scoped will break the codebase. Found multiple independent usages:
recent-activity-transaction-row.tsxdefines and uses the same type, indicating it was previously imported fromtransaction-row.tsx- Both files use identical type definition:
type TransactionStatus = "pending" | "success" | "failed"
The type should remain exported to maintain compatibility with dependent components.
🔗 Analysis chain
LGTM! Verify no external dependencies on TransactionStatus type.
Making TransactionStatus locally scoped aligns with the principle of least privilege since it's only used within this file.
Let's verify no external dependencies exist:
The previous command failed due to typescript file type not being recognized. Let's try searching without file type filters to see all occurrences of TransactionStatus in the codebase.
Let's examine how TransactionStatus is used in these files to determine if they're using the same type or if they're independent definitions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any external usage of TransactionStatus type
rg "TransactionStatus" --type typescript --type typescriptreact
Length of output: 98
Script:
#!/bin/bash
# Search for any external usage of TransactionStatus type using ripgrep without file type filters
rg "TransactionStatus" -l
Length of output: 282
Script:
#!/bin/bash
# Let's check the actual usage context in these files
echo "=== transaction-row.tsx ==="
rg "TransactionStatus" -C 2 packages/web/components/transactions/transaction-row.tsx
echo -e "\n=== recent-activity-transaction-row.tsx ==="
rg "TransactionStatus" -C 2 packages/web/components/transactions/recent-activity/recent-activity-transaction-row.tsx
echo -e "\n=== transfer-status.ts ==="
rg "TransactionStatus" -C 2 packages/bridge/src/squid/transfer-status.ts
echo -e "\n=== squid-transfer-status.spec.ts ==="
rg "TransactionStatus" -C 2 packages/bridge/src/squid/__tests__/squid-transfer-status.spec.ts
Length of output: 2999
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
packages/web/localizations/scripts/omitted-keys.mjs (1)
1-6: Enhance documentation and type safetyConsider adding more context to the documentation and TypeScript-style type definitions using JSDoc:
-/** Add key paths here to skip them in localizations tests. */ +/** + * Paths to be excluded from localization tests. + * + * These keys are skipped because: + * - assets.categories: Dynamic content that doesn't require translation + * - limitOrders.historyTable.columns: Table headers handled separately + * - transfer.nomic.estimatedTime: Dynamic timing information + * + * @type {readonly string[]} + */ -export const omittedKeyPaths = [ +export const omittedKeyPaths = Object.freeze([ "assets.categories", "limitOrders.historyTable.columns", "transfer.nomic.estimatedTime", -]; +]);packages/web/localizations/scripts/remove-all-unused.mjs (2)
6-9: Remove redundant comment about import placement.The import organization looks good, with related imports grouped together. However, the comment on line 6 is redundant since the prettier import is already appropriately placed with other imports.
-// Add prettier import at the top with other imports import prettier from "prettier";
Line range hint
1-118: Consider performance improvements for large codebases.The script uses synchronous file operations (
readFileSync,writeFileSync) which could impact performance with large codebases. Consider:
- Using async/await with Promise.all for parallel file processing
- Adding a progress indicator for better user feedback
- Implementing batch processing for large numbers of files
Would you like me to provide an example of how to implement these improvements?
packages/web/localizations/__tests__/localizations.spec.ts (1)
Line range hint
32-72: Consider enhancing test coverage for omitted keys.The test verifies key usage in source files but could be more robust by:
- Adding a test case to verify that omitted keys are actually present in the localization files
- Updating the test description to accurately reflect that it checks both .ts and .tsx files
Here's a suggested test case to add:
it("should have all omitted keys present in localization files", () => { const [, firstLocaleObj] = localizationObjs[0]; omittedKeyPaths.forEach(path => { const keys = path.split('.'); let obj = firstLocaleObj; for (const key of keys) { expect(obj).toHaveProperty(key); obj = obj[key]; } }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (19)
packages/web/localizations/de.jsonis excluded by!**/*.jsonpackages/web/localizations/en.jsonis excluded by!**/*.jsonpackages/web/localizations/es.jsonis excluded by!**/*.jsonpackages/web/localizations/fa.jsonis excluded by!**/*.jsonpackages/web/localizations/fr.jsonis excluded by!**/*.jsonpackages/web/localizations/gu.jsonis excluded by!**/*.jsonpackages/web/localizations/hi.jsonis excluded by!**/*.jsonpackages/web/localizations/ja.jsonis excluded by!**/*.jsonpackages/web/localizations/ko.jsonis excluded by!**/*.jsonpackages/web/localizations/pl.jsonis excluded by!**/*.jsonpackages/web/localizations/pt-br.jsonis excluded by!**/*.jsonpackages/web/localizations/ro.jsonis excluded by!**/*.jsonpackages/web/localizations/ru.jsonis excluded by!**/*.jsonpackages/web/localizations/tr.jsonis excluded by!**/*.jsonpackages/web/localizations/zh-cn.jsonis excluded by!**/*.jsonpackages/web/localizations/zh-hk.jsonis excluded by!**/*.jsonpackages/web/localizations/zh-tw.jsonis excluded by!**/*.jsonpackages/web/package.jsonis excluded by!**/*.jsonyarn.lockis excluded by!**/yarn.lock,!**/*.lock,!**/*.lock
📒 Files selected for processing (3)
packages/web/localizations/__tests__/localizations.spec.ts(1 hunks)packages/web/localizations/scripts/omitted-keys.mjs(1 hunks)packages/web/localizations/scripts/remove-all-unused.mjs(3 hunks)
🔇 Additional comments (2)
packages/web/localizations/scripts/omitted-keys.mjs (1)
2-6: Verify the existence of omitted key paths
Let's verify that these paths actually exist in the localization files to prevent potential typos or outdated references.
✅ Verification successful
Based on the results, I can now generate the final response as we have verified all three paths:
All omitted key paths are present and valid in localization files
assets.categoriesexists in all localization files with category translationslimitOrders.historyTable.columnsexists in all localization files with column translationstransfer.nomic.estimatedTimeexists in all localization files with time estimate translations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the omitted key paths exist in localization files
# Find all JSON files in the localizations directory
echo "Searching for key paths in localization files..."
for key in "assets.categories" "limitOrders.historyTable.columns" "transfer.nomic.estimatedTime"; do
echo "Checking for key: $key"
# Convert dot notation to nested object access for jq
jq_path=$(echo $key | sed 's/\././g')
fd -e json -e JSON --exec sh -c "echo 'File: {}' && jq '.$jq_path' '{}'" . packages/web/localizations
done
Length of output: 21090
packages/web/localizations/__tests__/localizations.spec.ts (1)
9-10: LGTM! Verify the .mjs extension compatibility.
Good refactoring to centralize the omitted keys. This change improves maintainability by removing duplication.
Let's verify the module extension compatibility:
✅ Verification successful
The .mjs extension usage is verified and safe
The codebase shows proper ESM support:
- The .mjs file is located at
packages/web/localizations/scripts/omitted-keys.mjs - Multiple .mjs files are being used in the project
- The web package (where the test file resides) has proper module configuration in its tsconfig.json
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any build configurations that might affect .mjs imports
rg -l "moduleResolution|allowJs|module" "tsconfig*.json" "package.json"
Length of output: 139
Script:
#!/bin/bash
# Let's first find the TypeScript config files in the repository
fd "tsconfig.*json"
# Then check package.json for module-related configurations
cat package.json | jq '.type, .dependencies, .devDependencies'
# Check if there are any other .mjs files being used
fd ".+\.mjs$"
# Look for any ESM/CJS related configurations in any config files
rg -l "type.*=.*module|moduleResolution|allowJs|module"
Length of output: 8222
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
packages/web/components/earn/table/index.tsx (1)
42-64: Consider extracting magic numbers and improving maintainability.The virtual scrolling configuration uses some hard-coded values that could be extracted as constants for better maintainability:
- Row height estimation (80px)
- Overscan value (10)
Consider applying this refactor:
+const VIRTUAL_ROW_HEIGHT = 80; +const VIRTUAL_OVERSCAN = 10; + const rowVirtualizer = useWindowVirtualizer({ count: rows.length, - estimateSize: () => 80, - overscan: 10, + estimateSize: () => VIRTUAL_ROW_HEIGHT, + overscan: VIRTUAL_OVERSCAN, paddingStart: topOffset, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/web/components/earn/table/index.tsx(1 hunks)
🔇 Additional comments (3)
packages/web/components/earn/table/index.tsx (3)
24-32: LGTM: Clean component export with proper TypeScript typing.
The component is well-structured with appropriate props for handling loading, error states, and data. The MobX observer wrapper is correctly applied.
66-86: LGTM: Comprehensive state handling with clear user feedback.
The conditional rendering logic effectively handles all possible states (loading, error, empty, and filtered results) with appropriate user feedback and actions.
24-24: Verify the impact of component renaming.
The component has been renamed from _StrategiesTable to StrategiesTable. Let's verify all imports have been updated.
✅ Verification successful
Component rename verification complete
The component has been successfully renamed from _StrategiesTable to StrategiesTable. No references to the old name _StrategiesTable exist in the codebase, and the new name is properly imported and used in pages/earn/index.tsx. The change appears to be properly implemented across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to the old component name
rg "_StrategiesTable" --type ts --type tsx
# Search for imports of the new component name to verify correct usage
rg "import.*StrategiesTable.*from.*earn/table" --type ts --type tsx
Length of output: 168
Script:
#!/bin/bash
# Search for any remaining references to the old component name without file type restrictions
rg "_StrategiesTable"
# Search for imports of the new component name
rg "import.*StrategiesTable.*from"
# List all TypeScript/React files that might import this component
fd "\.(tsx?|jsx?)$"
Length of output: 99416
Script:
#!/bin/bash
# Search for imports of the old component name in TypeScript/React files
rg -t typescript -t typescriptreact "_StrategiesTable"
# Search for imports of the new component name in TypeScript/React files
rg -t typescript -t typescriptreact "import.*StrategiesTable.*from"
# Search for any usage of the old component name in TypeScript/React files
rg -t typescript -t typescriptreact "<_StrategiesTable"
Length of output: 287
Script:
#!/bin/bash
# Search for any references to the old component name
rg "_StrategiesTable" --type tsx --type ts || true
# Search for imports of the new component name
rg "import.*StrategiesTable.*from" --type tsx --type ts || true
# List all files in the earn directory to understand the context
fd "\.(tsx?|jsx?)$" packages/web/components/earn/
Length of output: 758
Script:
#!/bin/bash
# Search for any references to the old component name without file type restrictions
rg "_StrategiesTable"
# Search for imports and usage of the new component name
rg "StrategiesTable" --context 2
# Check the content of the table component file
cat packages/web/components/earn/table/index.tsx
Length of output: 10476
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
packages/web/localizations/scripts/remove-all-unused.mjs (1)
Line range hint
29-42: Consider streaming approach for large codebases.The current implementation loads all source files into memory simultaneously, which could be problematic for large codebases. Consider a streaming approach:
- Process source files in chunks using streams
- Use worker threads for parallel processing
- Implement progress indicators for long-running operations
Example implementation approach:
import { Worker, isMainThread, parentPort } from 'worker_threads'; import { createReadStream } from 'fs'; import readline from 'readline'; // In main thread function processSourceFiles(files) { const CHUNK_SIZE = 10; const chunks = Array.from({ length: Math.ceil(files.length / CHUNK_SIZE) }, (_, i) => files.slice(i * CHUNK_SIZE, (i + 1) * CHUNK_SIZE) ); return Promise.all(chunks.map(chunk => { return new Promise((resolve, reject) => { const worker = new Worker(__filename); worker.on('message', resolve); worker.on('error', reject); worker.postMessage(chunk); }); })); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (17)
packages/web/localizations/de.jsonis excluded by!**/*.jsonpackages/web/localizations/en.jsonis excluded by!**/*.jsonpackages/web/localizations/es.jsonis excluded by!**/*.jsonpackages/web/localizations/fa.jsonis excluded by!**/*.jsonpackages/web/localizations/fr.jsonis excluded by!**/*.jsonpackages/web/localizations/gu.jsonis excluded by!**/*.jsonpackages/web/localizations/hi.jsonis excluded by!**/*.jsonpackages/web/localizations/ja.jsonis excluded by!**/*.jsonpackages/web/localizations/ko.jsonis excluded by!**/*.jsonpackages/web/localizations/pl.jsonis excluded by!**/*.jsonpackages/web/localizations/pt-br.jsonis excluded by!**/*.jsonpackages/web/localizations/ro.jsonis excluded by!**/*.jsonpackages/web/localizations/ru.jsonis excluded by!**/*.jsonpackages/web/localizations/tr.jsonis excluded by!**/*.jsonpackages/web/localizations/zh-cn.jsonis excluded by!**/*.jsonpackages/web/localizations/zh-hk.jsonis excluded by!**/*.jsonpackages/web/localizations/zh-tw.jsonis excluded by!**/*.json
📒 Files selected for processing (3)
packages/web/hooks/use-disclosure.ts(1 hunks)packages/web/localizations/scripts/remove-all-unused.mjs(1 hunks)packages/web/localizations/scripts/remove-key.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web/hooks/use-disclosure.ts
🔇 Additional comments (2)
packages/web/localizations/scripts/remove-all-unused.mjs (2)
Line range hint 116-120: Previous review comment about error handling is still applicable.
The code still lacks error handling for prettier formatting and contains an unnecessary implementation comment.
Line range hint 7-7: Verify omitted-keys.mjs existence and contents.
Let's ensure the imported file exists and contains the expected export.
✅ Verification successful
Let me verify the contents of the omittedKeyPaths array to ensure it's properly defined.
Let me try a different approach to get the file contents.
Import verification successful
The omitted-keys.mjs file exists and properly exports the omittedKeyPaths array containing a list of key paths to be skipped in localization tests. The implementation matches the expected usage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify omitted-keys.mjs exists and exports omittedKeyPaths
# Test 1: Check if the file exists
if [ ! -f "packages/web/localizations/scripts/omitted-keys.mjs" ]; then
echo "Error: omitted-keys.mjs not found"
exit 1
fi
# Test 2: Verify the export
rg -U "export const omittedKeyPaths" "packages/web/localizations/scripts/omitted-keys.mjs"
Length of output: 192
Script:
#!/bin/bash
# Get the full content of omittedKeyPaths array
rg -U "export const omittedKeyPaths = \[.*?\]" packages/web/localizations/scripts/omitted-keys.mjs
Length of output: 100
Script:
#!/bin/bash
# Display the entire content of the file to see the array definition
cat packages/web/localizations/scripts/omitted-keys.mjs
Length of output: 251
| // Verify we're in the localizations directory | ||
| const currentDir = path.basename(process.cwd()); | ||
| if (currentDir !== "localizations") { | ||
| throw new Error("This script must be run from the localizations directory"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance directory verification robustness.
While the directory verification is a good safety measure, consider these improvements:
- Use
path.resolve()to handle path separators consistently across platforms - Add a non-zero exit code when throwing the error
// Verify we're in the localizations directory
-const currentDir = path.basename(process.cwd());
+const currentDir = path.basename(path.resolve(process.cwd()));
if (currentDir !== "localizations") {
- throw new Error("This script must be run from the localizations directory");
+ console.error("Error: This script must be run from the localizations directory");
+ process.exit(1);
}📝 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.
| // Verify we're in the localizations directory | |
| const currentDir = path.basename(process.cwd()); | |
| if (currentDir !== "localizations") { | |
| throw new Error("This script must be run from the localizations directory"); | |
| } | |
| // Verify we're in the localizations directory | |
| const currentDir = path.basename(path.resolve(process.cwd())); | |
| if (currentDir !== "localizations") { | |
| console.error("Error: This script must be run from the localizations directory"); | |
| process.exit(1); | |
| } |
| // Verify we're in the localizations directory | ||
| const currentDir = path.basename(process.cwd()); | ||
| if (currentDir !== "localizations") { | ||
| throw new Error("This script must be run from the localizations directory"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance directory verification robustness.
While verifying the directory name is good, consider making the verification more robust and user-friendly.
-// Verify we're in the localizations directory
-const currentDir = path.basename(process.cwd());
-if (currentDir !== "localizations") {
- throw new Error("This script must be run from the localizations directory");
-}
+// Verify we're in the localizations directory with required files
+const currentDir = path.basename(process.cwd());
+if (currentDir !== "localizations") {
+ throw new Error(
+ "This script must be run from the localizations directory.\n" +
+ `Current directory: ${process.cwd()}\n` +
+ "Please change to the localizations directory and try again."
+ );
+}
+
+// Verify required files exist
+const requiredFiles = ['en.json'];
+for (const file of requiredFiles) {
+ if (!fs.existsSync(path.join(process.cwd(), file))) {
+ throw new Error(`Required file ${file} not found in the localizations directory`);
+ }
+}Committable suggestion was skipped due to low confidence.
Knip fix