Add daily sales summary cron job #265
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -66,6 +66,7 @@ | |||
| "class-variance-authority": "^0.7.0", | |||
| "clsx": "2.1.1", | |||
| "cmdk": "^1.0.0", | |||
| "csv-writer": "1.6.0", | |||
There was a problem hiding this comment.
Please let me know why we need to csv-writer.
I believe that the following code could be used as an alternative, and if a package is used, I would like to know the reasons for the selection compared to the alternative package.
interface ObjectToCsvOptions {
headerLabels?: Record<string, string>; // Mapping of keys to custom header labels
}
/**
* Converts an array of objects to a CSV formatted string
* @param objects Array of objects to convert
* @param options Conversion options
* @returns CSV formatted string
*/
function objectToCsv(
objects: Record<string, any>[],
options: ObjectToCsvOptions = {}
): string {
if (!Array.isArray(objects) || objects.length === 0) {
return '';
}
const { headerLabels = {} } = options;
// Get column names from the first object
const headers = Object.keys(objects[0]);
// Format value for CSV
const formatValue = (value: any): string => {
if (value === null || value === undefined) {
return '""';
}
return `"${String(value).replace(/"/g, '""')}"`;
};
// Generate header row with custom labels
const headerRow = headers
.map(header => formatValue(headerLabels[header] || header))
.join(',') + '\n';
// Generate data rows
const rows = objects.map(obj => {
return headers
.map(header => formatValue(obj[header]))
.join(',');
});
return headerRow + rows.join('\n');
}
// Usage example
const data = [
{ id: 1, name: 'John "Johnny" Doe', age: 30, city: 'New York, NY' },
{ id: 2, name: 'Jane Doe', age: 25, city: 'Los Angeles' },
{ id: 3, name: 'Bob Smith', age: null, city: 'Chicago' }
];
// With custom header labels
console.log(objectToCsv(data, {
headerLabels: {
id: 'ID Number',
name: 'Full Name',
age: 'Age',
city: 'City'
}
}));There was a problem hiding this comment.
I chose to use a dedicated CSV library (csv-writer) instead of built-in string manipulation or JSON conversion for several key reasons:
Reliable Data Escaping
Properly handles special characters like commas, double quotes, and line breaks
Critical for accounting data where malformed or corrupted formats are not acceptable
Improved Maintainability
CSV formatting logic is cleanly separated, making specification changes easier
Enhanced code readability
When compared to other libraries (e.g., fast-csv), csv-writer was chosen because it:
- Has minimal dependencies and is lightweight
- Provides good TypeScript support
- Offers a simple and intuitive API
These characteristics made it the optimal choice for our use case where we need to generate reliable CSV files for accounting purposes.
There was a problem hiding this comment.
Your code might work, but I don't know every CSV specifications, and it would take more time to investigate and verify.
Using a library would be more appropriate in this case.
There was a problem hiding this comment.
This data will be used for accounting tasks, so I strongly prefer not to implement CSV manually.
There was a problem hiding this comment.
📝
If reliability is important, I would consider using these libraries:
These libraries have many more stars on GitHub and are actively maintained.
(However, they might be a bit over-engineered for our current needs.)
There was a problem hiding this comment.
How about using only @fast-csv/format as a minimal solution while ensuring the reliability of the additional library?
https://c2fo.github.io/fast-csv/docs/formatting/getting-started/
There was a problem hiding this comment.
In what aspects does the use of packages increase reliability compared to manual implementation?
The CSV file created by this Pull request seems to be only strings, dates and numbers. I think reliability can be guaranteed by testing these expectations and outputs.
Also, if the perspective is that using a well-used package solves the known problems, then I think fast-csv should be used rather than csv-writer.
https://npmtrends.com/csv-writer-vs-fast-csv
There was a problem hiding this comment.
@toyamarinyon cc/ @shige
The CSV file created by this Pull request seems to be only strings, dates and numbers. I think reliability can be guaranteed by testing these expectations and outputs.
The string values could contain various characters that might break CSV formatting.
Additionally, since team names and agent names come from user input, we would need to handle encoding, commas, quotes, and other special characters ourselves.
I don't think it's a good use of our time to implement CSV conversion logic from scratch.
Also, if the perspective is that using a well-used package solves the known problems, then I think fast-csv should be used rather than csv-writer.
Regarding the suggestion that using a well-established package would solve known issues, I agree that fast-csv would be a better choice than csv-writer. I support using @fast-csv/format instead of csv-writer.
There was a problem hiding this comment.
I don't think it's a good use of our time to implement CSV conversion logic from scratch.
I feel the same way. I don't want to understand and implement RFC CSV myself, and if someone had implemented it independently, I would probably request changes to switch to using a library instead.
Overview
Key Changes
Technical Details
Future Tasks
TODO
CRON_SECRETto Vercel: Preview, DevelopmentCRON_SECRETto Vercel as a Sensitive value: Production