csv2json on command line, with no output file, prints corrupt JSON and limits to 100 items

[mikehardy]

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Okay those first two lines are boilerplate from patch-package but seriously, thanks for the project. It's quite useful.

I discovered two things interesting though, for my use case:

1. I am streaming to a file from stdout, I am not specifying a file, and in this case csv2json was printing the raw javascript object via the node console printer, which is not the same as printing JSON. So it needs to be JSON.stringified I think

2. console.log has an implementation-specific limit on the number of lines of raw data it will print, with 100 as a suggestion. Node v16 at least sticks to that 100 limit, so if you have 101 CSV lines you will get 100 objects in your array then '... and 1 more item', instead of all your items. To fix this you can either use utils.inspect (I tested it, it works) or - the same JSON.stringify from 1 above fixes it.

I think if a tool is called csv2json it should print valid JSON in all cases, so I chose solution in 1 for a fix. It's possibly a breaking change though? Depending on how you feel about that, you might like option 2. On the breaking change philosophy front I maintain a lot of packages and at least for me, I'd just bump that major and in the changelog say "csv2json with no output file now prints valid JSON as output. If you relied on the old behavior, adapt to the new valid JSON" or something, and get on with my life. But we're adults and adults can disagree and that's fine :-)

I'll post 2 PRs so you can see both solutions and can decide for yourself - merge one, close the other

Cheers!

Here is the diff that solved my problem printing valid JSON:

diff --git a/node_modules/@mrodrig/json-2-csv-cli/bin/utils/utils.js b/node_modules/@mrodrig/json-2-csv-cli/bin/utils/utils.js
index abe837b..3fc9527 100644
--- a/node_modules/@mrodrig/json-2-csv-cli/bin/utils/utils.js
+++ b/node_modules/@mrodrig/json-2-csv-cli/bin/utils/utils.js
@@ -78,8 +78,8 @@ function processOutput(params) {
         // Pretty print the output when converting from CSV to JSON
         return writeToFile(params.output, JSON.stringify(params.outputData, null, 4));
     }
-    // Otherwise, no output was specified so just send it to stdout via the console
-    console.log(params.outputData); // eslint-disable-line no-console
+    // Otherwise, no output was specified so send JSON to stdout via the console
+    console.log(JSON.stringify(params.outputData, null, 2)); // eslint-disable-line no-console
 }
 
 function constructKeysList(key, keys) {

This issue body was partially generated by patch-package.

[mrodrig]

Hi @mikehardy. Thanks for reporting this and for both pull requests you opened! I'm honestly surprised I didn't catch this when testing this functionality 😅, but you're right. I completely agree that it should be printing JSON for the output, rather than the JS representation of the data, so I think option 1 is my preference too. I'd love to keep the version number on this package roughly synced with the json-2-csv module, but I agree that this could potentially be a breaking change, so I'm in agreement that this would be good to release as a major version change. 🙂

I'll get #9 merged in now and published out in 4.0.0. Thanks again for finding this, reporting it, and the pull requests to address it!