SANC-56-export-to-csv-button-functionality #41
Conversation
jason-duong4509
requested review from
JustinTan-1,
Lujarios,
Yemyam,
burtonjong,
promatty,
tanvimahal,
themaxboucher and
wesleylui
as code owners
📝 WalkthroughWalkthroughA new CSV export feature is introduced with helper functions for generating and downloading CSV files. The implementation supports both direct data export and TRPC-backed queries with refetch and error handling capabilities, paired with a reusable React button component. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Button as ExportToCSVButton
participant Helper as downloadCSVWithEndpoint
participant TRPC as TRPC Query
participant CSV as downloadCSVWithoutEndpoint
participant Browser
User->>Button: Click Export
alt With Endpoint
Button->>Helper: Call with query result
Helper->>TRPC: Refetch data
TRPC-->>Helper: Data or Error
alt Success
Helper->>CSV: Pass table data
else Error
Helper->>User: Notify "Query failed!"
end
else Without Endpoint
Button->>CSV: Call with local data
end
CSV->>CSV: Extract headers from keys
CSV->>CSV: Build CSV rows
CSV->>CSV: Create blob
CSV->>Browser: Download via anchor element
Browser->>Browser: Cleanup anchor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
src/app/_components/common/exportToCSVButton/ExportToCSVButton.tsx (5)
15-15: Consider simplifying the return type.The returned function accepts
...args: any[]but doesn't use them. Consider using() => voidfor clarity, or if you need to accept event arguments, use(event?: React.MouseEvent) => void.🔎 Proposed fix
-export function downloadCSVWithoutEndpoint(tableData: JSONArray): (...args: any[]) => void { +export function downloadCSVWithoutEndpoint(tableData: JSONArray): () => void { return () => {
66-68: Simplify unnecessary function indirection.Creating a function and immediately calling it adds unnecessary indirection. Call
downloadCSVWithoutEndpoint(jsonArray)()directly.🔎 Proposed fix
const jsonArray = result.data as JSONArray; //Assert that this is a JSONArray - const downloadCSVFunc = downloadCSVWithoutEndpoint(jsonArray); //Call other helper to get downloader function - - downloadCSVFunc(); //Run the downloader function + downloadCSVWithoutEndpoint(jsonArray)(); //Generate and run the downloader function
76-78: Consider making downloadFunction required and matching actual signature.The
downloadFunctionprop is optional (?) but the component doesn't handle the undefined case. Additionally, the type should match the actual function signatures which don't use the args parameter.🔎 Proposed fix
interface ExportToCSVButtonProps { - downloadFunction?: (...args: any[]) => void; //Variable is of type function + downloadFunction: () => void | (() => Promise<void>); }If you want to keep it optional, add a default handler:
-export default function ExportToCSVButton({ downloadFunction }: ExportToCSVButtonProps) { +export default function ExportToCSVButton({ + downloadFunction = () => notify.warning("No download function provided") +}: ExportToCSVButtonProps) {
80-89: Consider adding loading state for async operations.When
downloadCSVWithEndpointis used, the download is async and may take time. Consider adding a loading state to provide user feedback during the operation.🔎 Example implementation with loading state
export default function ExportToCSVButton({ downloadFunction }: ExportToCSVButtonProps) { const [loading, setLoading] = React.useState(false); const handleClick = async () => { if (!downloadFunction) return; setLoading(true); try { await downloadFunction(); } finally { setLoading(false); } }; return ( <Button text="Export to CSV File" variant="secondary" icon={<Grid />} onClick={handleClick} loading={loading} /> ); }Note: This assumes
downloadFunctioncan be async. You may need to adjust based on whether the function is sync or async.
15-45: Consider using a well-established CSV library.Rolling a custom CSV implementation requires careful handling of edge cases (escaping, null values, encoding, different data types). Libraries like
papaparseorcsv-stringifyare battle-tested and handle these cases correctly.This would simplify the implementation significantly:
import Papa from 'papaparse'; export function downloadCSVWithoutEndpoint(tableData: JSONArray): () => void { return () => { if (tableData.length === 0) { notify.error("No data to export"); return; } const csv = Papa.unparse(tableData); const blob = new Blob([csv], { type: "text/csv;charset=utf-8;" }); const downloadURL = URL.createObjectURL(blob); const downloadElement = document.createElement("a"); downloadElement.href = downloadURL; downloadElement.download = `export-${new Date().toISOString().slice(0, 10)}.csv`; document.body.appendChild(downloadElement); downloadElement.click(); URL.revokeObjectURL(downloadURL); document.body.removeChild(downloadElement); }; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/_components/common/exportToCSVButton/ExportToCSVButton.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/app/_components/common/exportToCSVButton/ExportToCSVButton.tsx (2)
src/lib/notifications.ts (1)
notify(13-58)src/app/_components/common/button/Button.tsx (1)
Button(41-80)
| import type { UseTRPCQueryResult } from "node_modules/@trpc/react-query/dist/getQueryKey.d-CruH3ncI.mjs"; | ||
| import type { JSONArray, JSONObject } from "node_modules/superjson/dist/types"; |
There was a problem hiding this comment.
Fix import paths to use package names directly.
Importing from specific distribution paths within node_modules is fragile and will break when package versions change.
🔎 Proposed fix
-import type { UseTRPCQueryResult } from "node_modules/@trpc/react-query/dist/getQueryKey.d-CruH3ncI.mjs";
-import type { JSONArray, JSONObject } from "node_modules/superjson/dist/types";
+import type { UseTRPCQueryResult } from "@trpc/react-query";
+import type { JSONArray, JSONObject } from "superjson";📝 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.
| import type { UseTRPCQueryResult } from "node_modules/@trpc/react-query/dist/getQueryKey.d-CruH3ncI.mjs"; | |
| import type { JSONArray, JSONObject } from "node_modules/superjson/dist/types"; | |
| import type { UseTRPCQueryResult } from "@trpc/react-query"; | |
| import type { JSONArray, JSONObject } from "superjson"; |
🤖 Prompt for AI Agents
In src/app/_components/common/exportToCSVButton/ExportToCSVButton.tsx around
lines 3 to 4, the imports reference deep node_modules distribution paths which
are brittle; replace them with direct package imports (e.g., import type {
UseTRPCQueryResult } from "@trpc/react-query" or the correct @trpc package
exported type, and import type { JSONArray, JSONObject } from "superjson"),
update any named type names to match the package exports, save and run
TypeScript/type-check to confirm the correct module names and adjust if the
types live under a different exported entrypoint.
| for (const key in tableData[0] as JSONObject) { | ||
| //Keys in first row (JSON) become file column headers | ||
| csvFileString = csvFileString + key + ","; | ||
| } | ||
|
|
||
| csvFileString = csvFileString.slice(0, -1); //Remove the last , | ||
| csvFileString = csvFileString + "\n"; //Add a newline | ||
|
|
||
| for (let jsonObject of tableData) { | ||
| jsonObject = jsonObject as JSONObject; //Assert its type for TS | ||
| for (const key in jsonObject) { | ||
| csvFileString = csvFileString + jsonObject[key] + ","; | ||
| } | ||
| csvFileString = csvFileString.slice(0, -1); //Remove the last , | ||
| csvFileString = csvFileString + "\n"; //Add a newline | ||
| } |
There was a problem hiding this comment.
Escape CSV special characters to prevent malformed output.
Values containing commas, quotes, or newlines will break the CSV format. Each cell value must be properly escaped according to RFC 4180 (wrap in quotes if it contains comma/quote/newline, and escape quotes by doubling them).
🔎 Proposed fix with CSV escaping helper
Add this helper function before downloadCSVWithoutEndpoint:
function escapeCSVValue(value: unknown): string {
if (value == null) return "";
const str = String(value);
// If value contains comma, quote, or newline, wrap in quotes and escape internal quotes
if (str.includes(",") || str.includes('"') || str.includes("\n")) {
return `"${str.replace(/"/g, '""')}"`;
}
return str;
}Then update the CSV generation logic:
for (const key in tableData[0] as JSONObject) {
//Keys in first row (JSON) become file column headers
- csvFileString = csvFileString + key + ",";
+ csvFileString = csvFileString + escapeCSVValue(key) + ",";
}
csvFileString = csvFileString.slice(0, -1); //Remove the last ,
csvFileString = csvFileString + "\n"; //Add a newline
for (let jsonObject of tableData) {
jsonObject = jsonObject as JSONObject; //Assert its type for TS
for (const key in jsonObject) {
- csvFileString = csvFileString + jsonObject[key] + ",";
+ csvFileString = csvFileString + escapeCSVValue(jsonObject[key]) + ",";
}
csvFileString = csvFileString.slice(0, -1); //Remove the last ,
csvFileString = csvFileString + "\n"; //Add a newline
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/app/_components/common/exportToCSVButton/ExportToCSVButton.tsx around
lines 19 to 34, CSV output is not escaping values so commas, quotes, or newlines
will corrupt the file; add a helper (e.g., escapeCSVValue) that returns an empty
string for null/undefined, converts the value to string, doubles internal quotes
and wraps the string in quotes if it contains a comma, quote, or newline, then
use this helper when appending both headers and each cell value instead of
concatenating raw keys/values and continue trimming trailing commas and adding
newlines as before.
| return () => { | ||
| let csvFileString = ""; //Will hold the CSV file contents as a string to convert to blob later | ||
|
|
||
| for (const key in tableData[0] as JSONObject) { |
There was a problem hiding this comment.
Add validation for empty array.
Accessing tableData[0] without checking if the array is non-empty will cause a runtime error when exporting empty data.
🔎 Proposed fix
export function downloadCSVWithoutEndpoint(tableData: JSONArray): (...args: any[]) => void {
return () => {
+ if (tableData.length === 0) {
+ notify.error("No data to export");
+ return;
+ }
+
let csvFileString = ""; //Will hold the CSV file contents as a string to convert to blob later🤖 Prompt for AI Agents
In src/app/_components/common/exportToCSVButton/ExportToCSVButton.tsx around
line 19, the code accesses tableData[0] without checking if tableData is
non-empty which will throw for empty arrays; add a guard that checks if
tableData is an array and has length > 0 before iterating its keys (return
early, disable export, or show a suitable message/error when empty), and ensure
subsequent logic handles the empty case safely.
| const downloadElement = document.createElement("a"); | ||
| downloadElement.href = downloadURL; | ||
| document.body.appendChild(downloadElement); | ||
| downloadElement.click(); //Force the element to click, causing the file download |
There was a problem hiding this comment.
Add filename to the download anchor element.
Without a download attribute, the browser will generate a generic filename. Specify a meaningful default filename for better UX.
🔎 Proposed fix
const blob = new Blob([csvFileString], { type: "text/csv" }); //Turn CSV string into blob
const downloadURL = URL.createObjectURL(blob); //Turn blob into a URL
const downloadElement = document.createElement("a");
downloadElement.href = downloadURL;
+ downloadElement.download = `export-${new Date().toISOString().slice(0, 10)}.csv`;
document.body.appendChild(downloadElement);📝 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 downloadElement = document.createElement("a"); | |
| downloadElement.href = downloadURL; | |
| document.body.appendChild(downloadElement); | |
| downloadElement.click(); //Force the element to click, causing the file download | |
| const downloadElement = document.createElement("a"); | |
| downloadElement.href = downloadURL; | |
| downloadElement.download = `export-${new Date().toISOString().slice(0, 10)}.csv`; | |
| document.body.appendChild(downloadElement); | |
| downloadElement.click(); //Force the element to click, causing the file download |
🤖 Prompt for AI Agents
In src/app/_components/common/exportToCSVButton/ExportToCSVButton.tsx around
lines 38 to 41, the created anchor lacks a download attribute so the browser
chooses a generic filename; set a meaningful default filename (e.g., from props,
fallback to `export.csv` or include a timestamp) by assigning
downloadElement.download = filename before calling click(), then proceed to
click, remove the element and revoke the object URL if used to ensure cleanup.
| if (result.error) { | ||
| //There's an error | ||
| notify.error("Query failed!"); | ||
| } else if (Array.isArray(result.data) && typeof result.data[0] === "object") { |
There was a problem hiding this comment.
Add explicit empty array validation.
While typeof result.data[0] === "object" will be false for empty arrays (since typeof undefined is "undefined"), explicitly checking array length improves code clarity and prevents potential issues.
🔎 Proposed fix
- } else if (Array.isArray(result.data) && typeof result.data[0] === "object") {
+ } else if (Array.isArray(result.data) && result.data.length > 0 && typeof result.data[0] === "object") {
//result.data is an array of objects
const jsonArray = result.data as JSONArray; //Assert that this is a JSONArrayOr handle empty arrays explicitly:
- } else if (Array.isArray(result.data) && typeof result.data[0] === "object") {
+ } else if (Array.isArray(result.data) && result.data.length > 0 && typeof result.data[0] === "object") {
//result.data is an array of objects
const jsonArray = result.data as JSONArray; //Assert that this is a JSONArray
const downloadCSVFunc = downloadCSVWithoutEndpoint(jsonArray); //Call other helper to get downloader function
downloadCSVFunc(); //Run the downloader function
+ } else if (Array.isArray(result.data) && result.data.length === 0) {
+ notify.warning("No data to export");
} else {📝 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.
| } else if (Array.isArray(result.data) && typeof result.data[0] === "object") { | |
| } else if (Array.isArray(result.data) && result.data.length > 0 && typeof result.data[0] === "object") { |
🤖 Prompt for AI Agents
In src/app/_components/common/exportToCSVButton/ExportToCSVButton.tsx around
line 63, the branch uses Array.isArray(result.data) && typeof result.data[0] ===
"object" but doesn't explicitly guard against empty arrays; change the condition
to check length (e.g., Array.isArray(result.data) && result.data.length > 0 &&
typeof result.data[0] === "object") and add an explicit handler for empty arrays
(result.data.length === 0) to return/skip CSV generation or show an appropriate
no-data response so empty arrays are handled clearly and safely.
There was a problem hiding this comment.
https://www.ag-grid.com/javascript-data-grid/csv-export/
@jason-duong4509, couldn't the already existing CSV export module of ag grid be used so we don't have to implement our own functionality?
I haven't read through all the docs so maybe not, let me know. It is possible that the export to csv button might not be available on the calendar view, but maybe we can do something "hacky" to get it working.
Added functionality for an export to CSV button. The video below shows how someone could implement this button in their code
Video.mp4
As shown in the video, there are two helper functions,
downloadCSVWithoutEndpointanddownloadCSVWithEndpoint. They are both similar but the latter will first grab table data from a given endpoint and pass it to the formerThe intended use of this button is to always use either two helper functions.
downloadCSVWithEndpointcan be used in most cases, or if table data does not need to be specially altered beyond whatever a query endpoint returns.downloadCSVWithoutEndpointis used in all other cases, where data a query returns is not sufficient and needs to be filtered firstSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.