-
Notifications
You must be signed in to change notification settings - Fork 339
Fix oxlint warnings across UI and scripts #2443
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Clean up unused variables, hook dependencies, and Next.js head usage so lint passes without disabling rules.
📝 WalkthroughWalkthroughThis PR performs extensive code cleanup and optimization across the application, including removal of unused environment variables and dead code, addition of optional callback props to chart components for lifecycle management, dependency array refinements in effects and memos, and renaming of unused parameters with underscore prefixes to comply with linting standards. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes This PR introduces heterogeneous changes across 80+ files with multiple distinct patterns: dependency array refinements, prop lifecycle callbacks, unused parameter removals, logic simplifications, and refactoring. While many changes are routine cleanup, several require careful review including the destructuring issue in Bridges/columns.tsx, the extensive ProDashboard table refactoring with visibility key tracking, and the removal of features (breakdown charts, csvData memo). The high file diversity and mixed change types elevate cognitive load despite repetitive patterns in some cohorts. Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/components/Table/Bridges/Bridges/columns.tsx (2)
258-276: Runtime error:symbolis undefined but referenced in JSX.The destructuring was changed from
[symbol, token]to[, token]to fix a linter warning, butsymbolis still used at line 271 (<span>{symbol}</span>). This will throw aReferenceErrorat runtime.The linter incorrectly flagged
symbolas unused. Restore the original destructuring:🐛 Proposed fix
- const [, token] = splitValue + const [symbol, token] = splitValue
322-336: Runtime error:symbolis undefined but referenced in JSX.Same issue as
largeTxsColumn: the destructuring ignoressymbol, but it's still rendered at line 331. This will crash at runtime.🐛 Proposed fix
- const [, token] = splitValue + const [symbol, token] = splitValuesrc/containers/ProDashboard/components/datasets/FeesDataset/index.tsx (1)
58-73: Addinginstanceto the dependency array may cause excessive effect executions.
useReactTablereturns a new object reference on every render. Includinginstancein the dependency array will cause this effect to run on every render, continuously resetting column sizing and ordering. This defeats the purpose of only resetting on window resize.If the linter is complaining about the missing dependency, consider suppressing it with an eslint-disable comment, or use a ref to track initialization:
Suggested fix using initialization ref
+ const initializedRef = React.useRef(false) + React.useEffect(() => { + if (initializedRef.current && !windowSize) return + initializedRef.current = true + const defaultOrder = instance.getAllLeafColumns().map((d) => d.id) const defaultSizing = { name: 280, category: 120, total24h: 145, total7d: 120, total30d: 120, change_1d: 100, change_7d: 100, chains: 200 } instance.setColumnSizing(defaultSizing) instance.setColumnOrder(defaultOrder) - }, [instance, windowSize]) + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [windowSize])src/containers/ProDashboard/components/datasets/RevenueDataset/index.tsx (1)
170-185: Same issue:instancein dependency array causes excessive effect executions.This has the same problem as
FeesDataset—useReactTablereturns a new object reference on every render, so includinginstancein the dependency array will cause this effect to run continuously.Consider applying the same fix pattern suggested for
FeesDataset/index.tsx— either suppress the lint rule or use an initialization ref to prevent repeated resets.src/containers/Subscribtion/components/AccountStatus.tsx (1)
41-55: WrapgetPortalSessionUrlwithuseCallbackinuseSubscribehook to prevent unnecessary effect re-runs.The dependency array in AccountStatus.tsx is correct per exhaustive-deps, but
getPortalSessionUrlinsrc/containers/Subscribtion/useSubscribe.tsx(line 376) is not memoized. Since a new function reference is created on every hook render, this causes the useEffect to re-run on every parent re-render, triggering redundant API calls viacreatePortalSessionMutation.mutateAsync(). Wrap the function withuseCallbackto preserve the reference across renders.src/containers/ProDashboard/components/datasets/ChainsDataset/index.tsx (1)
276-327: The dependency oncolumnVisibilitydoesn't cause infinite loops, but it triggers unnecessary re-runs.The conditional guards at lines 310-326 prevent the effect from calling
setColumnVisibilitymore than once:
- First run:
columnVisibilityis empty, the condition at line 313 passes, and the effect sets it- Subsequent runs (triggered by
columnVisibilitychanges): The condition at line 313 evaluates tofalsebecausecolumnVisibilityis no longer empty, sosetColumnVisibilityis skippedHowever, the effect still re-executes on every
columnVisibilitychange, which is wasteful. Consider separating initialization logic from synchronization logic using a ref to track first-time setup, or restructure dependencies to exclude state values that the effect itself updates.Note: This same pattern appears in
useYieldsTable.ts(line 209), suggesting it may be intentional, though it remains suboptimal.src/components/ECharts/AreaChart/index.tsx (1)
402-419: RemoveonReadyfrom the second effect's dependency array or memoize withuseCallback.The second cleanup-only effect (lines 402–419) includes
onReadyin its dependency array, causing it to re-run wheneveronReadychanges. If the parent component passes an inline callback, the effect cleanup runs on every render, triggering unnecessarychartInstance.dispose()calls that destabilize the chart.Additionally, both effects call
chartInstance.dispose()(lines 386 and 408), creating redundant disposal attempts when dependencies change.Recommend either:
- Remove
onReadyfrom the second effect's dependency array if it's only intended to clean up on unmount, or- Require consumers to memoize
onReadywithuseCallbackand document this requirement, or- Use a ref to store
onReadyinternally to break the dependency chain
🤖 Fix all issues with AI agents
In `@src/components/Unlocks/PastUnlockPriceImpact.tsx`:
- Around line 23-24: The regex constant DESCRIPTION_REGEX has inconsistent
escaping for the closing bracket in the fourth alternation: change the token
pattern in the fourth group from \{tokens\[1]\} to \{tokens\[1\]\} so it matches
the same escaped form used in the second group; update the pattern in the const
DESCRIPTION_REGEX in PastUnlockPriceImpact (the const name IDENTIFIER) to use
the properly escaped closing bracket for consistency and clarity.
🧹 Nitpick comments (21)
src/containers/ProDashboard/components/MetricCard.tsx (1)
72-72: Consider removing the unused variable instead of aliasing.Rather than aliasing
lastUpdatedTsto_lastUpdatedTsto silence the linter, consider omitting it from the destructuring entirely since it's not used in this component:♻️ Suggested refactor
- const { value, delta, deltaPct, sparklineData, lastUpdatedTs: _lastUpdatedTs, isLoading, isError } = useMetricData(metric) + const { value, delta, deltaPct, sparklineData, isLoading, isError } = useMetricData(metric)src/containers/LlamaAI/hooks/useStreamNotification.ts (1)
48-52: Consider usingvoid new Notification(...)directly.The current approach works, but you can eliminate the intermediate variable by applying
voiddirectly to the constructor call:const showNotification = useCallback(() => { - const notification = new Notification('LlamaAI', { body: 'Llama has answered your question!', icon: '/favicon-badge.png' }) - void notification + void new Notification('LlamaAI', { body: 'Llama has answered your question!', icon: '/favicon-badge.png' }) playSound() }, [playSound])This expresses the intent more clearly (side effect with intentionally discarded result) and satisfies the same lint rules.
src/containers/ProDashboard/components/AddChartModal/MetricSentenceBuilder.tsx (1)
186-186: Consider not destructuring the unused value instead of prefixing with underscore.The
protocolFamilySetis computed inside theuseMemobut never used in the component. While the underscore prefix suppresses the lint warning, a cleaner approach is to simply not destructure unused values.♻️ Cleaner approach
-const { protocolOptions, protocolFamilySet: _protocolFamilySet } = useMemo(() => { +const { protocolOptions } = useMemo(() => {If
protocolFamilySetis truly unnecessary, you might also consider removing its computation from theuseMemoentirely (lines 203, 209, and the return value at line 225) to eliminate dead code.src/containers/ChainOverview/CustomColumnModal.tsx (1)
28-33: Consider adding a type annotation for thewordparameter.The removal of the unused
beforeCursorparameter is correct. However, since this is a TypeScript file, adding an explicit type improves clarity and type safety.♻️ Suggested improvement
-function getFilteredSuggestions(word) { +function getFilteredSuggestions(word: string) {src/pages/yields/strategy/[strat].js (1)
172-172: Consider removing redundant dependency.Both
configDataandconfigsMapare in the dependency array, butconfigsMapis derived fromconfigData(lines 34-36). SinceconfigsMapwill change wheneverconfigDatachanges, including both is redundant and could theoretically cause the memo to run twice perconfigDataupdate.You could remove
configDatafrom the dependency array since the null check at line 59 will still work correctly when the memo re-runs due toconfigsMapchanging.Suggested change
- }, [lendHistory, borrowHistory, farmHistory, configData, configsMap, lendToken, borrowToken, lendProjectCategory]) + }, [lendHistory, borrowHistory, farmHistory, configsMap, lendToken, borrowToken, lendProjectCategory])src/game/index.js (1)
6-14: Pre-existing security concern:evalusage.While not introduced by this PR,
window.eval(data)on line 10 is a security risk. Even though it fetches from the same origin (/game/index.js),evalcan be dangerous if the endpoint is ever compromised or serves unexpected content.Consider refactoring this in a future PR to use dynamic script injection or a safer loading mechanism.
src/pages/yields/pool/[pool].tsx (1)
224-230: Good fix for the null/undefined handling.The change correctly addresses the issue where
-el.apyBaseBorrow?.toFixed(2) ?? nullwould produceNaNwhenapyBaseBorrowis null/undefined (since-undefinedisNaN, and nullish coalescing doesn't trigger onNaN).Consider applying the same fix to lines 227-229, which still uses strict
=== nulland could produceNaNifapyBaseBorrowisundefined:el.apyBaseBorrow === null && el.apyRewardBorrow === null ? null : ((-el.apyBaseBorrow + el.apyRewardBorrow).toFixed(2) ?? null)♻️ Suggested consistency fix for lines 227-229
- el.apyBaseBorrow === null && el.apyRewardBorrow === null - ? null - : ((-el.apyBaseBorrow + el.apyRewardBorrow).toFixed(2) ?? null) + el.apyBaseBorrow == null || el.apyRewardBorrow == null + ? null + : (-el.apyBaseBorrow + el.apyRewardBorrow).toFixed(2)src/containers/ProDashboard/components/datasets/ChainsDataset/ChainsTableHeader.tsx (1)
8-8: ThecolumnPresetsprop is unused in this component, but removing it requires updating callers.The prop is declared in the interface and passed by callers in
index.tsx(lines 395, 416, 435), but never used withinChainsTableHeader. While renaming to_columnPresetssuppresses the lint warning, a cleaner approach would be to remove it from both the interface and the three call sites in the parent component if it's truly not needed.Alternatively, if this prop is being held for future use or API compatibility, the underscore prefix is a reasonable interim solution—but that intent should be clarified.
src/containers/LlamaAI/components/ResearchLimitModal.tsx (1)
13-13: The lint fix is correct, butresetTimeis a required prop that's never used.The underscore prefix appropriately silences the warning. However,
resetTimeis passed from the parent component but never consumed—line 41 hardcodes "Resets at midnight UTC" instead of displaying the actual reset time.Consider either removing
resetTimefrom the interface if it's truly unnecessary, or updating the component to display the dynamic reset time.src/containers/LlamaAI/components/ChartRenderer.tsx (1)
532-532: Lint fix looks good, but consider removing the unused prop entirely.The underscore prefix correctly silences the unused variable warning. However,
expectedChartCountis defined inChartRendererProps(line 32) but never used in the component. If this prop serves no purpose, consider removing it from the interface to reduce API surface area. Note that it's also being passed fromPromptResponse.tsx(lines 238, 290), so removal would require cleaning up those call sites as well.src/containers/Bridges/BridgesOverviewByChain.tsx (3)
33-39: RedundantuseEffectand unused type value.Two issues here:
- The
useEffecton lines 37-39 setschartViewto'netflow', but the state is already initialized to'netflow'on line 33—this effect is a no-op.- The type union includes
'default'but this value is never used in the component.Proposed cleanup
- const [chartView, setChartView] = React.useState<'default' | 'netflow' | 'volume'>('netflow') + const [chartView, setChartView] = React.useState<'netflow' | 'volume'>('netflow') const [activeTab, setActiveTab] = React.useState<'bridges' | 'messaging' | 'largeTxs'>('bridges') const [searchValue, setSearchValue] = React.useState('') - - useEffect(() => { - setChartView('netflow') - }, [])
2-2:useEffectimport may become unused.If the redundant
useEffect(lines 37-39) is removed as suggested above, this import will become unused and should be removed as well.
25-25: Remove thebridgeNamesprop or apply it if breakdown-chart logic is planned.The prop is currently unused (prefixed with
_to suppress linter warnings), but both callers (src/pages/bridges.tsxandsrc/pages/bridges/[...chain].tsx) actively pass it. If the breakdown-chart feature will not return, either remove it from the component signature and both callers together, or keep the prop if the feature may be re-added. The underscore prefix is acceptable for intentionally unused but stable-API props, but clarify the intent.src/containers/Stablecoins/StablecoinOverview.tsx (1)
162-166: Verify type safety for chart type lookup.The lookup
CHART_TYPE_TO_API_TYPE[chartType]could returnundefinedifchartTypedoesn't match a key, since the mapping usesRecord<string, ...>. Currently safe becausechartTypeis constrained by theTagGroupvalues, but consider using a stricter key type for the mapping to catch mismatches at compile time.💡 Optional: Stricter typing for the mapping
type ChartTypeLabel = 'Total Circ' | 'Pie' | 'Dominance' | 'Area' const CHART_TYPE_TO_API_TYPE: Record<ChartTypeLabel, StablecoinAssetChartType> = { 'Total Circ': 'totalCirc', Pie: 'chainPie', Dominance: 'chainDominance', Area: 'chainMcaps' }Then update the state type:
const [chartType, setChartType] = React.useState<ChartTypeLabel>('Pie')src/containers/ProDashboard/components/ProTable/useProTable.tsx (2)
408-408: Potential excessive effect re-runs due to object reference in dependency array.Adding
columnVisibility(an object) directly to the dependency array may cause this effect to run more frequently than intended, since objects are compared by reference. IfcolumnVisibilityis recreated on each render, this effect will fire repeatedly even when the actual visibility values haven't changed.Consider using a stable representation (e.g.,
Object.keys(columnVisibility).lengthor a JSON string) similar to thevisibleLeafColumnsKeypattern used elsewhere in this file.♻️ Suggested fix
- }, [customViews, options?.initialActiveViewId, columnOrder.length, columnVisibility]) + }, [customViews, options?.initialActiveViewId, columnOrder.length, Object.keys(columnVisibility).length])
1061-1079: Duplicate memoized key computation.
leafVisibilityKey(lines 1061-1066) is computed identically tovisibleLeafColumnsKey(lines 722-727). Both derive the same value fromtable.getAllLeafColumns().map(col => col.getIsVisible()).join(',').Consider reusing
visibleLeafColumnsKeyin thecurrentColumnsuseMemo instead of introducing a duplicate variable.♻️ Suggested fix
- const leafVisibilityKey = table - ? table - .getAllLeafColumns() - .map((col) => col.getIsVisible()) - .join(',') - : '' - // Get current column visibility state const currentColumns = React.useMemo(() => { if (!table) return {} - void leafVisibilityKey + void visibleLeafColumnsKey return table.getAllLeafColumns().reduce( (acc, col) => { acc[col.id] = col.getIsVisible() return acc }, {} as Record<string, boolean> ) - }, [table, leafVisibilityKey]) + }, [table, visibleLeafColumnsKey])src/containers/ProtocolOverview/index.tsx (1)
2097-2170: Consider removing dead code instead of renaming with underscore prefix.The
_Unlocksand_Governancefunctions are renamed with underscore prefixes to silence unused variable warnings, but the actual component usages are commented out (lines 1845-1846 and 1859-1860). This leaves ~70 lines of dead code in the file.If these features are temporarily disabled, consider adding a TODO comment explaining when they'll be re-enabled. If they're permanently removed, delete the function definitions entirely to reduce maintenance burden.
Is this a temporary disable or permanent removal? If temporary, a TODO comment would help track this:
// TODO: Re-enable Unlocks and Governance components when backend data is available // See: [ticket/issue link]If permanent, consider removing lines 2097-2170 entirely.
src/containers/ProDashboard/components/ProTable/index.tsx (1)
35-35: Ineffective memoization - useMemo returns the same reference.This
useMemodoesn't provide any memoization benefit:const memoizedChains = useMemo(() => chains, [chains])When
chainsreference changes, the dependency[chains]triggers a recomputation, returning the newchainsreference. Whenchainsstays the same,memoizedChainsis also the same. The memo adds overhead without stabilizing the reference.If the goal is to stabilize the array when its contents are equivalent but the reference differs (e.g.,
['All']recreated each render), use a content-based dependency:♻️ Suggested fix for content-based memoization
- const memoizedChains = useMemo(() => chains, [chains]) + const memoizedChains = useMemo(() => chains, [chains.join(',')])Alternatively, if the parent component already provides a stable reference, remove this memoization entirely.
src/components/ECharts/MultiSeriesChart/index.tsx (1)
44-44: Unused prophideDownloadButtonshould be removed from destructuring.The prop is destructured and aliased to
_hideDownloadButtonbut is never used in the component. While the underscore prefix suppresses lint warnings, this is effectively dead code. If the prop isn't needed, consider removing it from both the interface (line 29) and the destructuring.Suggested fix
- hideDownloadButton: _hideDownloadButton = false,And remove from interface:
- hideDownloadButton?: booleansrc/containers/ProDashboard/components/UnifiedTable/index.tsx (2)
234-244: Adding fullconfigobject to dependencies may cause excessive effect runs.The dependency arrays now include both
configand specific properties likeconfig.columnOrder. If the parent component doesn't memoize theconfigobject, any unrelated config change (or even the same config as a new reference) will trigger these effects, resetting column order and visibility state.If
getDefaultColumnOrderandgetDefaultColumnVisibilitydepend on additional config fields (likecustomColumns), consider listing those specific fields instead:Alternative approach
}, [config, config.columnOrder, previewMode]) + // Consider: [config.columnOrder, config.customColumns, previewMode]
409-412: Consider memoizinghandleCsvClickfor consistency.
handleExportClickis now memoized, buthandleCsvClickwraps it without memoization and is passed toUnifiedTableHeader(line 477). This creates a new function reference every render, potentially causing unnecessary child re-renders.Suggested fix
- const handleCsvClick = () => { - if (!unifiedTable.leafRows.length) return - handleExportClick() - } + const handleCsvClick = useCallback(() => { + if (!unifiedTable.leafRows.length) return + handleExportClick() + }, [handleExportClick, unifiedTable.leafRows.length])
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (91)
scripts/build-msg.jsscripts/pullMetadata.jssrc/components/ECharts/AreaChart/index.tsxsrc/components/ECharts/BarChart/index.tsxsrc/components/ECharts/MultiSeriesChart/index.tsxsrc/components/ECharts/MultiSeriesChart2/index.tsxsrc/components/ECharts/PieChart/index.tsxsrc/components/ECharts/SingleSeriesChart/index.tsxsrc/components/ECharts/TreemapChart/index.tsxsrc/components/IconsRow.tsxsrc/components/SEO/index.tsxsrc/components/Table/Bridges/Bridges/columns.tsxsrc/components/Table/Table.tsxsrc/components/Table/TableWithSearch.tsxsrc/components/Unlocks/CalendarView.tsxsrc/components/Unlocks/PastUnlockPriceImpact.tsxsrc/components/Unlocks/TopUnlocks.tsxsrc/containers/Bridges/BridgeProtocolOverview.tsxsrc/containers/Bridges/BridgesOverviewByChain.tsxsrc/containers/Bridges/queries.server.tsxsrc/containers/ChainOverview/Chart.tsxsrc/containers/ChainOverview/CustomColumnModal.tsxsrc/containers/ChainOverview/SmolCharts.tsxsrc/containers/ChainOverview/queries.server.tsxsrc/containers/CompareTokens/index.tsxsrc/containers/LlamaAI/components/ChartRenderer.tsxsrc/containers/LlamaAI/components/RecommendedPrompts.tsxsrc/containers/LlamaAI/components/ResearchLimitModal.tsxsrc/containers/LlamaAI/hooks/useStreamNotification.tssrc/containers/LlamaAI/index.tsxsrc/containers/Oracles/index.tsxsrc/containers/ProDashboard/components/AddChartModal/ChartBuilderTab.tsxsrc/containers/ProDashboard/components/AddChartModal/MetricSentenceBuilder.tsxsrc/containers/ProDashboard/components/AddChartModal/SubjectMultiPanel.tsxsrc/containers/ProDashboard/components/AddChartModal/UnifiedTableTab/index.tsxsrc/containers/ProDashboard/components/AriakitVirtualizedSelect.tsxsrc/containers/ProDashboard/components/ChartBuilderCard.tsxsrc/containers/ProDashboard/components/ChartCard.tsxsrc/containers/ProDashboard/components/ChartGrid.tsxsrc/containers/ProDashboard/components/ComparisonWizard/index.tsxsrc/containers/ProDashboard/components/DashboardSearch.tsxsrc/containers/ProDashboard/components/MetricCard.tsxsrc/containers/ProDashboard/components/ProTable/ColumnManagementPanel.tsxsrc/containers/ProDashboard/components/ProTable/CustomColumnPanel.tsxsrc/containers/ProDashboard/components/ProTable/ProtocolFilterModal.tsxsrc/containers/ProDashboard/components/ProTable/ReorderableHeader.tsxsrc/containers/ProDashboard/components/ProTable/TableBody.tsxsrc/containers/ProDashboard/components/ProTable/index.tsxsrc/containers/ProDashboard/components/ProTable/useProTable.tsxsrc/containers/ProDashboard/components/UnifiedTable/config/ColumnRegistry.tsxsrc/containers/ProDashboard/components/UnifiedTable/core/useUnifiedTable.tssrc/containers/ProDashboard/components/UnifiedTable/index.tsxsrc/containers/ProDashboard/components/datasets/AggregatorsDataset/index.tsxsrc/containers/ProDashboard/components/datasets/BridgeAggregatorsDataset/index.tsxsrc/containers/ProDashboard/components/datasets/CexDataset/columns.tsxsrc/containers/ProDashboard/components/datasets/CexDataset/index.tsxsrc/containers/ProDashboard/components/datasets/ChainsDataset/ChainsTableHeader.tsxsrc/containers/ProDashboard/components/datasets/ChainsDataset/index.tsxsrc/containers/ProDashboard/components/datasets/EarningsDataset/index.tsxsrc/containers/ProDashboard/components/datasets/FeesDataset/index.tsxsrc/containers/ProDashboard/components/datasets/HoldersRevenueDataset/columns.tsxsrc/containers/ProDashboard/components/datasets/HoldersRevenueDataset/index.tsxsrc/containers/ProDashboard/components/datasets/RevenueDataset/index.tsxsrc/containers/ProDashboard/components/datasets/StablecoinsDataset/index.tsxsrc/containers/ProDashboard/components/datasets/TokenUsageDataset/index.tsxsrc/containers/ProDashboard/components/useYieldChartTransformations.tssrc/containers/ProDashboard/services/ProtocolCharts.tssrc/containers/ProtocolOverview/Chart/Chart.tsxsrc/containers/ProtocolOverview/Emissions/UpcomingEvent.tsxsrc/containers/ProtocolOverview/Governance.tsxsrc/containers/ProtocolOverview/Layout.tsxsrc/containers/ProtocolOverview/index.tsxsrc/containers/ProtocolOverview/queries.tsxsrc/containers/ProtocolOverview/utils.tsxsrc/containers/ProtocolsByCategoryOrTag/index.tsxsrc/containers/ProtocolsByCategoryOrTag/queries.tsxsrc/containers/Stablecoins/StablecoinOverview.tsxsrc/containers/Subscribtion/Home.tsxsrc/containers/Subscribtion/components/AccountStatus.tsxsrc/game/index.jssrc/pages/chart/chain/[...chain].tsxsrc/pages/digital-asset-treasury/[...company].tsxsrc/pages/pitch.tsxsrc/pages/pro/[dashboardId].tsxsrc/pages/protocol/fees/[...protocol].tsxsrc/pages/protocol/options/[...protocol].tsxsrc/pages/protocol/unlocks/[...protocol].tsxsrc/pages/yields/pool/[pool].tsxsrc/pages/yields/strategy/[strat].jssrc/server/protocolSplit/protocolChainService.tssrc/utils/cache-client.ts
💤 Files with no reviewable changes (9)
- src/containers/ProDashboard/components/useYieldChartTransformations.ts
- src/containers/ProDashboard/components/ProTable/TableBody.tsx
- src/containers/ProDashboard/components/ProTable/ReorderableHeader.tsx
- src/components/IconsRow.tsx
- src/containers/ProDashboard/components/datasets/HoldersRevenueDataset/columns.tsx
- src/containers/Subscribtion/Home.tsx
- src/components/ECharts/SingleSeriesChart/index.tsx
- src/containers/ProDashboard/services/ProtocolCharts.ts
- src/containers/ProDashboard/components/datasets/BridgeAggregatorsDataset/index.tsx
🧰 Additional context used
🧬 Code graph analysis (12)
src/containers/ProDashboard/components/UnifiedTable/core/useUnifiedTable.ts (1)
src/utils/index.js (1)
rows(767-774)
src/components/Table/TableWithSearch.tsx (1)
src/components/Table/Defi/Protocols/columns.tsx (2)
columnOrders(1497-1540)columnSizes(1542-1579)
src/pages/protocol/unlocks/[...protocol].tsx (2)
src/pages/protocol/dex-aggregators/[...protocol].tsx (1)
Protocols(106-204)src/pages/protocol/governance/[...protocol].tsx (1)
Protocols(71-84)
src/containers/ProDashboard/components/AddChartModal/UnifiedTableTab/index.tsx (1)
src/containers/ProDashboard/types.ts (1)
TableFilters(228-341)
src/containers/ProDashboard/components/datasets/AggregatorsDataset/index.tsx (1)
src/containers/ProDashboard/components/datasets/AggregatorsDataset/useAggregatorsData.ts (1)
useAggregatorsData(5-18)
src/containers/ProDashboard/components/MetricCard.tsx (1)
src/containers/ProDashboard/hooks/useMetricData.ts (1)
useMetricData(163-260)
src/containers/ProtocolOverview/Layout.tsx (1)
src/utils/index.js (2)
slug(468-468)slug(468-468)
src/containers/ChainOverview/SmolCharts.tsx (2)
scripts/build-msg.js (1)
name(41-41)src/pages/yields/strategy/[strat].js (1)
tokens(23-23)
src/containers/ProtocolOverview/index.tsx (1)
src/containers/ProtocolOverview/types.ts (1)
IProtocolOverviewPageData(136-274)
src/containers/ProDashboard/components/UnifiedTable/index.tsx (1)
src/containers/ProDashboard/components/UnifiedTable/utils/configHelpers.ts (1)
getDefaultColumnVisibility(44-74)
src/containers/Stablecoins/StablecoinOverview.tsx (2)
src/containers/ProDashboard/types.ts (1)
StablecoinAssetChartType(146-146)src/utils/index.js (2)
slug(468-468)slug(468-468)
src/containers/ChainOverview/queries.server.tsx (1)
src/utils/index.js (5)
date(821-821)date(835-835)date(840-840)date(847-847)date(941-941)
🔇 Additional comments (86)
src/pages/digital-asset-treasury/[...company].tsx (1)
163-174: LGTM!The underscore prefix on
_mcap_realizedcorrectly signals that this variable is intentionally unused, consistent with the existing_mcap_realisticand_mcap_maxon lines 169-170. This is a standard convention for suppressing linter warnings while maintaining positional destructuring from the stats array.src/containers/ProtocolOverview/utils.tsx (1)
718-737: LGTM!The simplification from
addlProtocolData ? true : falsetoBoolean(addlProtocolData)is semantically equivalent and more idiomatic. This is a clean lint fix that improves readability without changing behavior.src/containers/Oracles/index.tsx (1)
147-149: LGTM!The unused
rowparameter is correctly removed from the cell renderer. The implementation only usesgetValue(), so this cleanup is appropriate and aligns with the PR's lint fix objectives.src/components/Unlocks/TopUnlocks.tsx (1)
69-82: LGTM!Removing the unused index parameter
iis a clean fix for the oxlint warning. The key correctly usesunlock.namewhich is unique per protocol, so the index was unnecessary.src/containers/ProtocolOverview/queries.tsx (1)
862-862: LGTM!The destructuring pattern
[, value]correctly ignores the unuseddateelement while preserving the original logic. This is the idiomatic way to silence unused variable warnings in array destructuring.src/containers/ProDashboard/components/datasets/AggregatorsDataset/index.tsx (1)
41-41: LGTM!Removing the unused
refetchfrom the destructuring is a clean fix for the linter warning. The component doesn't expose any manual refresh functionality, so the variable was indeed dead code.src/utils/cache-client.ts (1)
12-27: LGTM!The added
if (redisUrl)guard is a sensible defensive check that prevents attempting to dynamically importioredisand establish a connection when no Redis URL is configured. This correctly handles the case whereEXT_REDIS_URLis undefined in non-runtime environments.src/containers/ChainOverview/queries.server.tsx (1)
1171-1180: LGTM!The underscore prefix correctly marks
_net,_inflow, and_outflowas intentionally unused while preserving the tuple destructuring order. This is the idiomatic way to satisfy linters when positional destructuring requires intermediate elements that aren't needed.src/containers/CompareTokens/index.tsx (1)
16-31: LGTM! Correct dependency fix.Adding
coinsDatato the dependency array is the right fix. The memo usescoinsData.find()on line 27 to resolve coin objects, so it must recompute whencoinsDatachanges to avoid returning staleselectedCoins.src/server/protocolSplit/protocolChainService.ts (1)
224-224: LGTM! Correct linting fix for unused variable.Prefixing the unused destructured variable with an underscore is the correct convention and aligns with the PR's goal of cleaning up oxlint warnings. The variable is not referenced anywhere in the loop body (lines 225-258), so this change is appropriate.
src/containers/ProDashboard/components/AriakitVirtualizedSelect.tsx (1)
59-66: LGTM!Adding
virtualizerto the dependency array is correct. The effect usesvirtualizer.measure()andvirtualizer.scrollToIndex(), so it should be listed as a dependency. TanStack Virtual'suseVirtualizerreturns a stable reference, so this won't cause unnecessary re-executions. The early return guard onisPopoverOpenensures the effect logic only runs when the popover is open.src/containers/ProDashboard/components/DashboardSearch.tsx (1)
34-34: LGTM!The destructuring correctly excludes
pagefrom the query when updating search results, ensuring users start from page 1 on a new search. The_pagealias appropriately signals an intentionally unused variable to satisfy the linter.src/components/Unlocks/CalendarView.tsx (1)
271-275: LGTM!Removing the unused index parameter is a valid lint fix. The Week view correctly uses
day.date!for the key sincegenerateWeekDaysalways produces entries with valid dates, unlike the Month view which needs the index fallback for potential empty calendar slots.src/containers/ProDashboard/components/datasets/CexDataset/columns.tsx (1)
5-5: LGTM!Correctly removes the unused
TokenLogoandformattedPercentimports while retainingformattedNum, which is used throughout the column definitions. Clean oxlint fix.src/containers/ProDashboard/components/ChartBuilderCard.tsx (2)
484-484: LGTM! Redundant ternary removed.The comparison
value === '% Percentage'already returns a boolean, making the? true : falseternary unnecessary. This simplification improves readability without changing behavior.
521-521: LGTM! Inverted ternary simplified.Replacing
value === 'All' ? false : truewithvalue !== 'All'is logically equivalent and more concise. This improves readability while maintaining the same behavior.src/containers/ProtocolsByCategoryOrTag/queries.tsx (2)
44-45: LGTM!The
Boolean()wrapper is a clean simplification that maintains equivalent semantics.
526-526: LGTM!Correctly uses the comma placeholder to skip the unused
dateelement in the destructuring pattern.src/containers/ProtocolsByCategoryOrTag/index.tsx (1)
46-48: LGTM!The unused
keyparameter is correctly removed from the filter callback while the subsequent.map()still extracts the keys as needed.src/containers/ProtocolOverview/Emissions/UpcomingEvent.tsx (2)
181-182: LGTM!The underscore prefix correctly marks the destructured
timestampas intentionally unused within this callback scope, preventing shadowing of the outertimestampprop while silencing the linter warning.
326-327: LGTM!Consistent with the earlier change at line 182—same pattern applied to the second map callback to mark the unused inner
timestampappropriately.src/containers/Bridges/queries.server.tsx (3)
463-469: LGTM!Using the leading comma in destructuring
[, volumeData]is the correct pattern to indicate the first element (symbol/key) is intentionally unused while filtering based only onvolumeData.volume. This properly addresses the oxlint warning.
543-549: LGTM!Consistent with the earlier change at line 464 — using
[, addressData]correctly signals that the address key is intentionally unused in this filter callback.
314-314: AI summary references non-existent function.The AI-generated summary mentions changes to a
getDATInflowsfunction with destructuring updates like[date, _net, _inflow, _outflow, purchasePrice, usdValueOfPurchase], but this function does not appear in the provided code. The actual changes in this file are withingetBridgePageDatanewat lines 464 and 544. This may indicate the summary was generated from a different file or an earlier version.src/containers/LlamaAI/components/RecommendedPrompts.tsx (2)
96-108: LGTM!Adding
storeto the dependency array correctly satisfies the exhaustive-deps rule. Since Ariakit'suseTabStorereturns a stable reference, this won't cause unnecessary re-subscriptions while ensuring the effect would correctly re-attach if the store ever changed.
110-133: LGTM!Same rationale as above—adding
storeto the dependency array correctly addresses the lint warning without introducing any runtime issues.src/containers/ChainOverview/CustomColumnModal.tsx (2)
42-42: LGTM!The underscore prefix correctly signals that
displayAsis intentionally unused, satisfying the linter while maintaining API compatibility.
83-83: LGTM!The call site correctly matches the updated function signature after removing the unused parameter.
src/containers/ChainOverview/SmolCharts.tsx (2)
348-348: LGTM!Adding
groupByto the dependency array is correct—it's used in the tooltip formatter (line 306), so the effect should re-run when this prop changes.
489-489: LGTM!Adding
tokensto the dependency array is correct—it's directly referenced in the tooltip formatter (line 447). Whileseriesalready depends ontokensvia useMemo, the lint rule requires all direct references within the effect to be declared.src/pages/yields/strategy/[strat].js (1)
118-119: LGTM!Good defensive fix using nullish coalescing to prevent
NaNresults whentotalSupplyUsdortotalBorrowUsdis undefined.src/containers/ProDashboard/components/ComparisonWizard/index.tsx (1)
35-41: LGTM! Cleaner dependency pattern.Extracting
resetand using it directly in the dependency array is the correct pattern to satisfy the exhaustive-deps rule. This assumesresetis a stable reference (memoized inComparisonWizardContext). Ifresetwere recreated on each render, this cleanup would fire on every re-render rather than only on unmount.src/components/Table/Bridges/Bridges/columns.tsx (1)
342-348: LGTM!This change is correct—only
tokenis needed to derivechainName, so ignoringsymbolis appropriate here.scripts/build-msg.js (1)
63-69: LGTM!The refactoring cleanly consolidates the
buildSummaryandcommitSummaryconstruction. Using a ternary for the initial outcome and single template literals improves readability while preserving the original logic.src/components/SEO/index.tsx (1)
1-1: ESLint disable is reasonable for this use case.The
LinkPreviewCardandSEOcomponents are designed to be composed together on pages, each contributing different meta tags to<Head>. The disable directive is appropriate here.However, consider migrating to Next.js 15+ Metadata API (
generateMetadataormetadataexport) in a future refactor, which handles deduplication automatically and is the recommended approach for Next.js 15+.src/pages/pro/[dashboardId].tsx (1)
1-1: ESLint disable is appropriate.The page uses
<Head>in multiple locations to dynamically set the title based on dashboard state (lines 110-112 and 182-184), while theLayoutcomponent handles base SEO. This is a valid Next.js pattern where multiple<Head>components merge their contents.src/game/index.js (1)
1-1: ESLint disable is acceptable for this game component.The game loads external CSS assets via a
<link>tag (line 19), which is a reasonable pattern for isolated game functionality that may have its own styling requirements separate from the Next.js build pipeline.src/components/Unlocks/PastUnlockPriceImpact.tsx (2)
26-29: LGTM!The helper function cleanly extracts the first non-empty capture group with proper optional chaining and fallback handling.
31-31: LGTM!Good refactoring to place the helper function before the main component for better readability.
src/containers/ProDashboard/components/datasets/TokenUsageDataset/index.tsx (1)
87-87: LGTM!Good use of
useMemoto stabilize thetokenSymbolsreference. Previously,config.tokenSymbols ?? []would create a new empty array on every render whenconfig.tokenSymbolsis undefined, causing downstream hooks (useTokenUsageData,getColumns,tokenStats,protocolOverlap) to recompute unnecessarily. This memoization correctly addresses that.src/containers/ProDashboard/components/ChartCard.tsx (1)
212-214: LGTM! Clean boolean simplification.The comparison
value === 'Cumulative'already evaluates to a boolean, making the ternary redundant. This is a good lint fix.src/containers/ProDashboard/components/datasets/StablecoinsDataset/index.tsx (1)
62-76: LGTM! Correct dependency array fix.Adding
instanceto the dependency array satisfies the exhaustive-deps rule since the effect usesinstance.getAllLeafColumns(),instance.setColumnSizing(), andinstance.setColumnOrder(). TheuseReactTablehook returns a stable reference when its own dependencies haven't changed, so this won't cause unnecessary re-runs.src/containers/ProDashboard/components/AddChartModal/SubjectMultiPanel.tsx (1)
106-119: LGTM! Proper dependency tracking for virtualizers.The effect invokes
chainVirtualizer.measure(),protocolVirtualizer.measure(), and theirscrollToIndex()methods, so including them in the dependency array is correct. TheuseVirtualizerhook maintains stable references, making this a safe addition.src/containers/ProDashboard/components/datasets/CexDataset/index.tsx (1)
62-78: LGTM! Consistent with the broader dataset component pattern.This change mirrors the same fix applied across other dataset components (
StablecoinsDataset,EarningsDataset, etc.), ensuring the effect dependencies includeinstancesince it's used within the effect body.src/containers/ProDashboard/components/datasets/EarningsDataset/index.tsx (1)
170-185: LGTM! Correct exhaustive-deps fix.The effect references
instancefor column operations, so adding it to the dependency array is the right fix. This aligns with the consistent pattern applied across the ProDashboard dataset components in this PR.scripts/pullMetadata.js (1)
23-29: LGTM!Removing the unused
errorparameter from the catch clause is a valid cleanup. The optional catch binding syntax (catch { ... }) is supported in ES2019+ and appropriately addresses the oxlint warning.src/components/Table/TableWithSearch.tsx (1)
97-107: AddingcolumnOrdersandcolumnSizesto dependencies is correct.These props control the column configuration, so the effect should re-run when they change. This is a valid fix for the exhaustive-deps rule.
Note: The
instancedependency has the same caveat as other files (new reference on each render), but that appears to be a pre-existing pattern in this codebase.src/containers/ProtocolOverview/Layout.tsx (1)
207-207: LGTM!Clean conversion from string concatenation to template literal. The resulting key value is identical, and template literals are more readable for string interpolation.
src/containers/ProDashboard/components/datasets/HoldersRevenueDataset/index.tsx (1)
169-184: LGTM - addinginstanceto dependencies is correct.The effect uses
instance.getAllLeafColumns(),instance.setColumnSizing(), andinstance.setColumnOrder(), so includinginstancein the dependency array aligns with React's exhaustive-deps rule.Note: This effect re-applies default sizing/order whenever
instanceorwindowSizechanges. If preserving user column customizations across data refreshes is desired, consider tracking initialization state separately.src/containers/LlamaAI/index.tsx (1)
214-287: LGTM!Adding
updateSessionTitleto the dependency array is correct - the callback invokes it on line 260 when processing'title'events from the stream. This aligns with React's exhaustive-deps rule foruseCallback.src/containers/ProtocolOverview/Governance.tsx (1)
300-306: LGTM!Clean simplification from
info.getValue() === 0 ? false : truetoinfo.getValue() !== 0. The logic is preserved and the expression is more idiomatic.src/containers/ProDashboard/components/ChartGrid.tsx (1)
323-329: LGTM! Clean lint fix for dependency array.Extracting
currentRatingSession?.sessionIdto a local variable avoids the optional chaining expression in the dependency array, which is the correct pattern for satisfying lint rules. The guard condition change fromif (currentRatingSession)toif (currentSessionId)is semantically appropriate since the effect's purpose is tied to having a valid session ID.src/containers/ProDashboard/components/AddChartModal/UnifiedTableTab/index.tsx (1)
342-364: LGTM! Correct dependency additions for useCallback hooks.Adding
setFiltersto the dependency arrays is the proper fix for exhaustive-deps lint warnings. Both callbacks directly referencesetFilters, so they should re-memoize if that reference changes.Note: The underscore prefix on
_handleRemoveArrayFilterValuefollows the convention for intentionally unused variables to satisfy no-unused-vars lint rules.src/components/ECharts/PieChart/index.tsx (2)
68-68: LGTM! Boolean expression simplification.
!showLegendis cleaner and semantically identical toshowLegend ? false : true.
77-83: LGTM! Removed unused parameter.Removing the unused
idxparameter from the map callback satisfies the no-unused-vars lint rule.src/containers/Stablecoins/StablecoinOverview.tsx (1)
37-42: Good refactor - hoisting constant to module level.Moving the mapping outside the component avoids recreating the object on each render. The keys align correctly with the
TagGroupvalues used in the UI.src/containers/Bridges/BridgeProtocolOverview.tsx (1)
95-98: Lint-compliant but introduces extra effect executions.Adding
chartTypeto the dependency array satisfies exhaustive-deps, but now the effect runs on everychartTypechange (including manual selections like 'Tokens To' or 'Tokens From'), even when no state change occurs. This is a minor performance consideration.The effect is still safe—it won't cause infinite loops since the conditions are mutually exclusive with the state changes. If the extra effect executions become noticeable, consider refactoring to only run when
isAllChainschanges:💡 Alternative: Track previous isAllChains value
+const prevIsAllChains = React.useRef(isAllChains) + React.useEffect(() => { + if (prevIsAllChains.current === isAllChains) return + prevIsAllChains.current = isAllChains if (isAllChains && chartType === 'Inflows') setChartType('Volume') if (!isAllChains && chartType === 'Volume') setChartType('Inflows') }, [chartType, isAllChains])src/components/ECharts/TreemapChart/index.tsx (1)
27-32: LGTM!The refactor from inline logical-AND assignments to explicit
ifblocks improves readability and satisfies linting rules about assignments in expressions. Functionality is preserved.src/containers/ProDashboard/components/ProTable/ColumnManagementPanel.tsx (2)
178-196: LGTM!Removing the
indexprop fromColumnButtonand computing position internally viavisibleColumnsInOrder.indexOf()simplifies the component API. The trade-off is a slight recomputation on each render, but this is negligible for typical column counts.
389-395: Cleaner mapping without external index.The simplified mapping correctly removes the now-unused index parameter, aligning with the internal position computation in
ColumnButton.src/containers/ProDashboard/components/AddChartModal/ChartBuilderTab.tsx (4)
221-221: LGTM!Memoizing
seriesColorsprevents creating a new empty object{}on every render whenchartBuilder.seriesColorsis undefined. This stabilizes downstream dependencies.
244-253: LGTM!Wrapping
resolveSeriesColorinuseCallbackwith[seriesColors]as the dependency is correct and stabilizes the function reference for dependent memos liketreemapData.
273-273: Dependency updated correctly.
treemapDatanow depends onresolveSeriesColorinstead of the rawseriesColorsobject, which aligns with its usage inside the memo.
379-388: TheonChartBuilderChangecallback is already properly memoized. InuseModalActions.ts(lines 283-288),updateChartBuilderis wrapped inuseCallbackwith dependency array[actions], and theactionsobject is returned from auseMemohook. This ensures the callback is stable across renders and will not cause unnecessary effect re-runs.src/pages/pitch.tsx (2)
92-92: Unused prop aliased for lint compliance.
lastRoundsis received as a prop but never used in the component body. The aliasing to_lastRoundssatisfies the linter. Note that this prop is still being passed fromgetStaticProps(line 86) — consider removing it from both the props object and the interface if it's no longer needed.
279-283: LGTM!Using an empty catch block (
catch {}) is valid ES2019+ syntax for optional catch binding. This is appropriate here sinceshowPicker()may throw on browsers that don't support it, and the error is intentionally ignored for progressive enhancement.src/containers/ProDashboard/components/ProTable/CustomColumnPanel.tsx (1)
25-26: Unused prop aliased to suppress linter.The
onUpdateCustomColumnprop is destructured but never used in this component. While aliasing it to_onUpdateCustomColumncorrectly suppresses the lint warning, consider whether this prop should be removed from the interface entirely if updating custom columns is not a planned feature. The component currently supports only creating and deleting columns; existing columns are displayed read-only.src/containers/ProtocolOverview/Chart/Chart.tsx (1)
556-558: LGTM!The dependency array now correctly includes
onReadyandhideDataZoom, ensuring the effect re-runs when these props change. This aligns with React's exhaustive-deps rule and ensures the chart properly updates whenhideDataZoomaffects the grid bottom calculation (line 522) or when a newonReadycallback is provided.src/containers/ProDashboard/components/ProTable/ProtocolFilterModal.tsx (2)
14-16: LGTM!The removal of
getValuefrom the props destructuring is correct - the virtualized menu list directly accesseschildren[virtualRow.index]rather than usinggetValue, making this prop unused.
87-88: Remove unusedportalTargetprop from interface and call site.The
portalTargetprop is defined in the interface and passed from the parent component but is never used in the ProtocolFilterModal implementation. It's aliased to_portalTargetto suppress linter warnings. TheAriakit.Dialogcomponent is configured withportal={true}(a boolean flag) and does not have aportalElementor similar prop to consume the target. Remove the prop entirely from both the interface definition and the parent component's call site.Likely an incorrect or invalid review comment.
src/pages/protocol/fees/[...protocol].tsx (1)
345-345: LGTM!The template string consolidation is cleaner. The trailing space ensures consistent layout regardless of whether the deprecated tag is present.
src/pages/protocol/options/[...protocol].tsx (1)
198-198: LGTM!Consistent with the fees page refactor — template string is cleaner and maintains the same output behavior.
src/containers/ProDashboard/components/UnifiedTable/core/useUnifiedTable.ts (1)
110-110: LGTM!Good memoization addition. This stabilizes the
rowsreference, preventing unnecessary recomputations of downstream memos likefilteredRowswhen the query data hasn't actually changed.src/containers/ChainOverview/Chart.tsx (1)
437-437: Dependency array expansion looks correct.Adding
isThemeDark,chartData, andhideDataZoomensures the effect re-runs when these values change:
chartDatais used directly for axis color determination (lines 181, 260-266)hideDataZoomaffects grid layout (line 412)isThemeDarkinfluences axis styling fallbacks (line 264-266)This properly satisfies the exhaustive-deps lint rule.
src/components/Table/Table.tsx (1)
24-30: LGTM!The underscore prefix (
_TData,_TValue) correctly signals that these type parameters are required by the interface signature but intentionally unused in the metadata properties. This satisfies the lint rule for unused type parameters.src/pages/chart/chain/[...chain].tsx (1)
61-104: LGTM! Correct dependency array expansion.The added dependencies (
props.charts,props.chainTokenInfo?.gecko_id,selectedChain) are all referenced within theuseMemocallback and were previously missing. This fix ensures the memoized values recompute correctly when these props change.src/pages/protocol/unlocks/[...protocol].tsx (1)
53-67: LGTM! Consistent with other protocol pages.The simplified signature removes unused destructured parameters (
clientSide,protocolData) that weren't part of the props returned bygetStaticProps. This aligns with the pattern used ingovernance/[...protocol].tsxanddex-aggregators/[...protocol].tsx.src/components/ECharts/MultiSeriesChart2/index.tsx (1)
321-336: LGTM! Dependencies correctly added.Both
selectedChartsandonReadyare used within the effect body:
selectedChartsaffects the dataset dimensions (line 281)onReadyis invoked when the chart instance is created (lines 168-170)This follows the same pattern applied to
AreaChartandBarChartin this PR.src/components/ECharts/BarChart/index.tsx (2)
219-230: LGTM! Dependencies correctly added.Both
orientation(used at line 178) andonReady(invoked at lines 158-160) are properly tracked in the dependency array.
251-251: LGTM! Cleaner boolean expression.Using
Boolean()is more explicit and idiomatic than a ternary for boolean coercion.src/containers/ProDashboard/components/UnifiedTable/config/ColumnRegistry.tsx (2)
13-13: LGTM! Unused import removed.The
getChainNameForRowimport was removed as it's not used in this file.
19-24: LGTM! Correct TypeScript convention for unused type parameters.The underscore prefix (
_TData,_TValue) correctly indicates these type parameters are required by the@tanstack/table-coremodule augmentation signature but are not referenced within theColumnMetainterface body. This suppresses linter warnings about unused generics.src/components/ECharts/AreaChart/index.tsx (1)
398-400: Addition ofonReadyto dependencies looks correct.The
onReadycallback is invoked within the effect body (lines 328-330), so including it in the dependency array ensures the effect re-runs if the callback reference changes.src/components/ECharts/MultiSeriesChart/index.tsx (1)
273-275: Dependency additions are correct.These values (
showAggregateInTooltip,valueSymbol,yAxisSymbols) are all used within the effect body:
showAggregateInTooltipat lines 146 and 197valueSymbolat line 174yAxisSymbolsat line 174Including them ensures the chart re-renders when these formatting options change.
src/containers/ProDashboard/components/UnifiedTable/index.tsx (2)
376-407: Memoization ofhandleExportClickis appropriate.Wrapping this in
useCallbackwith dependencies onconfig.customColumnsandunifiedTable.tableensures the callback is stable across renders when these values don't change, preventing unnecessary re-renders of components that receive this callback.
414-437: Dependency array update looks correct.Including
handleExportClick,unifiedTable.table, andunifiedTable.leafRows.lengthensures the callback stays synchronized with the latest state. SincehandleExportClickis now memoized, this won't cause excessive re-renders.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const DESCRIPTION_REGEX = | ||
| /(?:of (.+?) tokens (?:will be|were) unlocked)|(?:will (?:increase|decrease) from \{tokens\[0\]\} to \{tokens\[1\]\} tokens per week from (.+?) on {timestamp})|(?:from (.+?) on {timestamp})|(?:was (?:increased|decreased) from \{tokens\[0\]\} to \{tokens\[1]\} tokens per week from (.+?) on {timestamp})/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent bracket escaping in regex pattern.
The fourth alternation group has \{tokens\[1]\} while the second group has \{tokens\[1\]\}. The closing bracket escape \] is missing in the fourth group. While unescaped ] works in regex outside character classes, this inconsistency suggests a copy-paste error and should be fixed for clarity and maintainability.
Proposed fix
const DESCRIPTION_REGEX =
- /(?:of (.+?) tokens (?:will be|were) unlocked)|(?:will (?:increase|decrease) from \{tokens\[0\]\} to \{tokens\[1\]\} tokens per week from (.+?) on {timestamp})|(?:from (.+?) on {timestamp})|(?:was (?:increased|decreased) from \{tokens\[0\]\} to \{tokens\[1]\} tokens per week from (.+?) on {timestamp})/
+ /(?:of (.+?) tokens (?:will be|were) unlocked)|(?:will (?:increase|decrease) from \{tokens\[0\]\} to \{tokens\[1\]\} tokens per week from (.+?) on {timestamp})|(?:from (.+?) on {timestamp})|(?:was (?:increased|decreased) from \{tokens\[0\]\} to \{tokens\[1\]\} tokens per week from (.+?) on {timestamp})/📝 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 DESCRIPTION_REGEX = | |
| /(?:of (.+?) tokens (?:will be|were) unlocked)|(?:will (?:increase|decrease) from \{tokens\[0\]\} to \{tokens\[1\]\} tokens per week from (.+?) on {timestamp})|(?:from (.+?) on {timestamp})|(?:was (?:increased|decreased) from \{tokens\[0\]\} to \{tokens\[1]\} tokens per week from (.+?) on {timestamp})/ | |
| const DESCRIPTION_REGEX = | |
| /(?:of (.+?) tokens (?:will be|were) unlocked)|(?:will (?:increase|decrease) from \{tokens\[0\]\} to \{tokens\[1\]\} tokens per week from (.+?) on {timestamp})|(?:from (.+?) on {timestamp})|(?:was (?:increased|decreased) from \{tokens\[0\]\} to \{tokens\[1\]\} tokens per week from (.+?) on {timestamp})/ |
🤖 Prompt for AI Agents
In `@src/components/Unlocks/PastUnlockPriceImpact.tsx` around lines 23 - 24, The
regex constant DESCRIPTION_REGEX has inconsistent escaping for the closing
bracket in the fourth alternation: change the token pattern in the fourth group
from \{tokens\[1]\} to \{tokens\[1\]\} so it matches the same escaped form used
in the second group; update the pattern in the const DESCRIPTION_REGEX in
PastUnlockPriceImpact (the const name IDENTIFIER) to use the properly escaped
closing bracket for consistency and clarity.
Summary
Test plan
Summary by CodeRabbit
Release Notes
New Features
onReadycallback support to chart components for lifecycle tracking.Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.