Keyang / node-csvtojson

Blazing fast and Comprehensive CSV Parser for Node.JS / Browser / Command Line.
MIT License
2.02k stars 271 forks source link

header names aren't taken from the first row emitted when using csv().fromStream #134

Closed paddymahoney closed 7 years ago

paddymahoney commented 7 years ago
import * as request from "request";

const parseReport = (reportFileUrl, response) => {
    csv({flatKeys: true})
        .fromStream(request.get(reportFileUrl))
        .pipe(response);
};

When I switched this code to using fromStream(it was using fromString), it resulted in generated field names being used in the resulting json, instead of the first item emitted from the request.get readable stream.

Is this a known problem? would a fleshed out repro project/example help? I may also be using something wrong as well.

Thank you for this library @Keyang !

Keyang commented 7 years ago

Hi, Thanks for the good feedback. If you could create a public repo project/example reproducing this issue, it would be great helpful. I have tests around using fromStream which passing ok. I assume it is the remote csv resource may contain whitespaces at the beginning. if so, there may be a bug in parser.

Regards, Keyang

On 18 January 2017 at 17:18, Patrick Mahoney notifications@github.com wrote:

`import * as request from "request";

const parseReport = (reportFileUrl, response) => { csv({flatKeys: true}) .fromStream(request.get(reportFileUrl)) .pipe(response); };`

When I switched this code to using fromStream(it was using fromString), it resulted in generated field names being used in the resulting json, instead of the first item emitted from the request.get readable stream.

Is this a known problem? would a fleshed out repro project/example help? I may also be using something wrong as well.

Thank you for this library @Keyang https://github.com/Keyang !

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Keyang/node-csvtojson/issues/134, or mute the thread https://github.com/notifications/unsubscribe-auth/ABCs98IqK5LGMKxmLP9IeuHCpLWEUIMUks5rTklfgaJpZM4LnJ2f .

paddymahoney commented 7 years ago

@Keyang thanks for responding - I'm going to attempt a repro as soon as possible and will link here.

Keyang commented 7 years ago

Thanks for the feedback. I can see the issue. This is caused by the long header which exceeded the highWaterMark of readStream.

Will fix this in 1.1.3 soon.

~Keyang

On 23 January 2017 at 20:18, Patrick Mahoney notifications@github.com wrote:

Hi @Keyang https://github.com/Keyang , I still intend to do a more complete repro project, on the way to the whole thing, here is an example file: https://raw.githubusercontent.com/paddymahoney/node- csvtojson/master/scrubbed.csv

To test this, I added a new route (in order to stream to an express response stream):

app.get("/reports/test2", (request, response) => { parseReport("https://raw.githubusercontent.com/paddymahoney/node-csvtojson/master/scrubbed.csv", response); });

Then, parseReport is defined like:

const parseReport = (reportFileUrl, response) => { csv({flatKeys: true}) .fromStream(request.get(reportFileUrl)) .pipe(response); };

So I should be able to reproduce this with just csvtojson, express and request. The file should be a valid example though.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Keyang/node-csvtojson/issues/134#issuecomment-274604562, or mute the thread https://github.com/notifications/unsubscribe-auth/ABCs96w-HnltrRxuVSCX63QVWjVxyEczks5rVQsYgaJpZM4LnJ2f .

Keyang commented 7 years ago

Hi, The issue has been fixed in 1.1.3

Thanks, Keyang

paddymahoney commented 7 years ago

Truly awesome @Keyang - that is a tricky solve.