Skip to content

Commit

Permalink
fix(formatter,#503): Do not ignore rows when headers is false
Browse files Browse the repository at this point in the history
* Fixes issue where row contents were ignored if the first row is empty and headers is false
  • Loading branch information
doug-martin committed Nov 3, 2020
1 parent e3165c7 commit 1560564
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 7 deletions.
6 changes: 3 additions & 3 deletions documentation/docs/formatting/options.md
Expand Up @@ -58,9 +58,9 @@ Set to `true` if you want the first character written to the stream to be a utf-
**Type**: `null|boolean|string[]` **Default**: `null`

If `true` then the headers will be auto detected from the first row. [Example](./examples.mdx#auto-discovery)
* If the row is a one-dimensional array then headers is a no-op
* If the row is an object then the keys will be used.
* If the row is an array of two element arrays (`[ ['header', 'column'], ['header2', 'column2'] ]`) then the first element in each array will be used.
* If the row is a one-dimensional array then the `headers` will be the first row processed
* If the row is an object then the keys will be used.
* If the row is an array of two element arrays (`[ ['header', 'column'], ['header2', 'column2'] ]`) then the first element in each array will be used.

If there is not a headers row and you want to provide one then set to a `string[]`. [Example](./examples.mdx#provided-headers)

Expand Down
6 changes: 6 additions & 0 deletions packages/format/__tests__/formatter/RowFormatter.spec.ts
Expand Up @@ -93,6 +93,12 @@ describe('RowFormatter', () => {
const formatter = createFormatter({ headers: false });
await expect(formatRow(headerRow, formatter)).resolves.toEqual([headerRow.join(',')]);
});

it('should still format all rows without headers', async () => {
const formatter = createFormatter({ headers: false });
await expect(formatRow([], formatter)).resolves.toEqual(['']);
await expect(formatRow(headerRow, formatter)).resolves.toEqual([`\n${headerRow.join(',')}`]);
});
});

describe('with headers=true', () => {
Expand Down
34 changes: 34 additions & 0 deletions packages/format/__tests__/issues/issue503.spec.ts
@@ -0,0 +1,34 @@
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).toEqual(['\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).toEqual(['1', '\n', '\n1,2,3']);
res();
});
});
});
});
16 changes: 12 additions & 4 deletions packages/format/src/formatter/RowFormatter.ts
Expand Up @@ -9,16 +9,20 @@ type RowFormatterTransform<I extends Row, O extends Row> = (row: I, cb: RowTrans
type RowFormatterCallback = (error: Error | null, data?: RowArray) => void;

export class RowFormatter<I extends Row, O extends Row> {
private static isHashArray(row: Row): row is RowHashArray {
private static isRowHashArray(row: Row): row is RowHashArray {
if (Array.isArray(row)) {
return Array.isArray(row[0]) && row[0].length === 2;
}
return false;
}

private static isRowArray(row: Row): row is RowArray {
return Array.isArray(row) && !this.isRowHashArray(row);
}

// get headers from a row item
private static gatherHeaders(row: Row): string[] {
if (RowFormatter.isHashArray(row)) {
if (RowFormatter.isRowHashArray(row)) {
// lets assume a multi-dimesional array with item 0 being the header
return row.map((it): string => it[0]);
}
Expand Down Expand Up @@ -130,7 +134,6 @@ export class RowFormatter<I extends Row, O extends Row> {
// either the headers were provided by the user or we have already gathered them.
return { shouldFormatColumns: true, headers: this.headers };
}

const headers = RowFormatter.gatherHeaders(row);
this.headers = headers;
this.fieldFormatter.headers = headers;
Expand All @@ -151,7 +154,7 @@ export class RowFormatter<I extends Row, O extends Row> {
if (!Array.isArray(row)) {
return this.headers.map((header): string => row[header] as string);
}
if (RowFormatter.isHashArray(row)) {
if (RowFormatter.isRowHashArray(row)) {
return this.headers.map((header, i): string => {
const col = (row[i] as unknown) as string;
if (col) {
Expand All @@ -160,6 +163,11 @@ export class RowFormatter<I extends Row, O extends Row> {
return '';
});
}
// if its a one dimensional array and headers were not provided
// then just return the row
if (RowFormatter.isRowArray(row) && !this.shouldWriteHeaders) {
return row;
}
return this.headers.map((header, i): string => row[i]);
}

Expand Down

0 comments on commit 1560564

Please sign in to comment.