Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] writeToStream / writeToPath emits empty output following empty rows #503

Closed
1 task
robations opened this issue Oct 23, 2020 · 4 comments
Closed
1 task
Assignees

Comments

@robations
Copy link

Describe the bug
When writing to a stream with an empty row and one or more rows with data, instead of writing the rows provided, writeToStream and writeToPath emits empty rows for each line. Not sure if intended but this seems to be a change in behaviour from v2 and feels like an unexpected outcome.

It might be slightly non-standard, but I'm working with a CSV file where the number of columns may change throughout the file.

Parsing or Formatting?

  • [ x] Formatting
  • Parsing

To Reproduce
Steps to reproduce the behavior:

// test.ts
import {writeToPath} from "fast-csv";

writeToPath("space-oddity.csv", [[], ["this", "should", "be", "written?"]], {headers: false})
    .on("finish", () => {
        console.log("done");
        process.exit();
    });
ts-node test.ts && cat space-oddity.csv

The same thing happens with writeToStream.

Expected behavior
Expected output:


this,should,be,written?

Screenshots
n/a

Desktop (please complete the following information):

  • OS: MacOS
  • OS Version 10.15.6
  • Node Version 12.18.3

Additional context
Let me know if you need any more info. I tried to step through the source code with the debugger, but the line numbers were pretty mangled so it wasn't clear what was happening when.

@robations
Copy link
Author

Looking at this a bit more, I suspect the issue might be something related to the headers = false setting getting normalised to null in FormatterOptions.ts line 71. Just a hunch.

When we get to RowFormatter.ts, it is changing this.headers to a value inside the method checkHeaders() (I would have expected in the case of headers = false we take no heed of headers at all?).

I also tried settings writeHeaders = false in case this changes the behaviour, but makes no difference. I'm still assuming this is a bug not a feature :)

Test case:

// packages/format/__tests__/issues/issue503.spec.ts
import { RecordingStream } from '../__fixtures__';
import { RowArray, write } from '../../src';

describe('Issue #503 - https://github.com/C2FO/fast-csv/issues/503', () => {
    it('should emit all columns after an empty row', () => {
        return new Promise((res, rej) => {
            const rs = new RecordingStream();
            const data: RowArray[] = [[], ['something']];

            write(data, { quote: false, headers: false, writeHeaders: false })
                .pipe(rs)
                .on('error', rej)
                .on('finish', () => {
                    expect(rs.data.join('')).toBe('\nsomething');
                    res();
                });
        });
    });

    it('should not assume first row is a header if header = false', () => {
        return new Promise((res, rej) => {
            const rs = new RecordingStream();
            const data: RowArray[] = [['1'], [], ['1', '2', '3']];

            write(data, { quote: false, headers: false, writeHeaders: false })
                .pipe(rs)
                .on('error', rej)
                .on('finish', () => {
                    expect(rs.data.join('')).toBe('1\n\n1,2,3');
                    res();
                });
        });
    });
});

@doug-martin
Copy link
Contributor

@robations I think I narrowed it down to https://github.com/C2FO/fast-csv/blob/master/packages/format/src/formatter/RowFormatter.ts#L135 I'm going to play with this since I'll have to handle the case where a row is an array of objects, hash arrays or a one dimensional array.

Thank you for the test case that is really useful!

@robations
Copy link
Author

@doug-martin no probs, and thanks for checking this out.

doug-martin added a commit that referenced this issue Nov 3, 2020
* Fixes issue where row contents were ignored if the first row is empty and headers is false
doug-martin added a commit that referenced this issue Nov 3, 2020
* Fixes issue where row contents were ignored if the first row is empty and headers is false
@doug-martin
Copy link
Contributor

@robations fixed and published under v4.3.4. Thanks again for the bug report!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants