Conversation
When we use that let then we get this error "'allRows' is never reassigned. Use 'const' instead.",
|
@AsimSafarli is attempting to deploy a commit to the Dodo Payments Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughA variable declaration in the usage table component was changed from mutable ( Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/registry/billingsdk/usage-table.tsx (2)
72-78:formatNumber/formatCurrencyinuseCallbackdeps cause the callback to be recreated on every render.Both helpers are defined as fresh function references on every render. Because they are included in the
useCallbackdependency array (lines 176–177),exportToCsvitself is also recreated on every render, defeating the purpose of memoization. Since neither function closes over any state or props, the simplest fix is to hoist them outside the component.♻️ Proposed refactor — hoist pure helpers outside the component
+"use client"; +import React, { useCallback } from "react"; +// ... + +// Hoisted outside the component — no dependency on state/props +const formatNumber = (num: number) => new Intl.NumberFormat().format(num); +const formatCurrency = (amount: number) => `$${amount.toFixed(2)}`; + export function UsageTable({ ... }: UsageTableProps) { - const formatNumber = (num: number) => { - return new Intl.NumberFormat().format(num); - }; - - const formatCurrency = (amount: number) => { - return `$${amount.toFixed(2)}`; - }; // ...Then remove them from the
useCallbackdependency array:}, [ usageHistory, totalRow, showTotal, hasApiCost, hasCostToYou, - formatNumber, - formatCurrency, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/registry/billingsdk/usage-table.tsx` around lines 72 - 78, formatNumber and formatCurrency are recreated each render and included in the useCallback deps, causing exportToCsv to be recreated every render; hoist both pure helper functions (formatNumber, formatCurrency) out of the component scope so they are stable top-level functions and then remove them from the exportToCsv useCallback dependency array (leaving only real dependencies) so exportToCsv can be memoized properly.
97-107:exportColumnsis used insideuseCallbackbut absent from the dependency array.
exportColumnsis declared inside the component (line 86) and captured in theexportToCsvclosure, but it's not listed in theuseCallbackdeps (lines 170–178). Thereact-hooks/exhaustive-depsrule would flag this. It's functionally harmless today since the array is static, but it's an inconsistency worth resolving — either moveexportColumnsoutside the component alongsideformatNumber/formatCurrency, or add it to the dep array.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/registry/billingsdk/usage-table.tsx` around lines 97 - 107, The exportToCsv useCallback closes over exportColumns but exportColumns is not in its dependency array; either add exportColumns to the useCallback deps or move the exportColumns declaration out of the component (like formatNumber/formatCurrency) so it's truly static; update the dependency array on exportToCsv (the useCallback that defines exportToCsv) to include exportColumns if you keep it inside the component, or hoist exportColumns above the component and keep the current deps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/registry/billingsdk/usage-table.tsx`:
- Around line 72-78: formatNumber and formatCurrency are recreated each render
and included in the useCallback deps, causing exportToCsv to be recreated every
render; hoist both pure helper functions (formatNumber, formatCurrency) out of
the component scope so they are stable top-level functions and then remove them
from the exportToCsv useCallback dependency array (leaving only real
dependencies) so exportToCsv can be memoized properly.
- Around line 97-107: The exportToCsv useCallback closes over exportColumns but
exportColumns is not in its dependency array; either add exportColumns to the
useCallback deps or move the exportColumns declaration out of the component
(like formatNumber/formatCurrency) so it's truly static; update the dependency
array on exportToCsv (the useCallback that defines exportToCsv) to include
exportColumns if you keep it inside the component, or hoist exportColumns above
the component and keep the current deps.
When we use that let then we get this error "'allRows' is never reassigned. Use 'const' instead.",
Summary
Describe the purpose of this PR.
Changes
Screenshots/Recordings (if UI)
Add before/after or short clip if relevant.
How to Test
Steps to validate locally:
Checklist
Summary by CodeRabbit