sheet_add_json with origin: -1 leaves gap from bottom of worksheet #1364

New Issue

2018-11-24T12:30:00Z

dandv commented

2018-11-24 12:30:00 +00:00

(Migrated from github.com)

I'm seeing strange behavior with the following code:

const XLSX = require('xlsx');

const wb = XLSX.utils.book_new();

const ws = XLSX.utils.json_to_sheet([
  { Product: 'p2', Qty: 2 },
]);

XLSX.utils.sheet_add_json(ws,
  [
    { Product: 'p3', Qty: 3 },
    { Product: 'p4', Qty: 4 },
    { Product: 'p5', Qty: 5 },
    { Product: 'p6', Qty: 6 },
    { Product: 'p7', Qty: 7 },
    { Product: 'p8', Qty: 8 },
  ],
  {
    header: ['Product', 'Qty'],
    skipHeader: true,
    origin: -1,  // append to bottom of worksheet starting on first column
  }
);


XLSX.utils.book_append_sheet(wb, ws, 'Bug sheet');

XLSX.writeFile(wb, 'bug.ods');

I expected p3...p8 to be placed on rows 3 ... 8, but they ended up on rows 6 ... 11 (in LibreCalc at least).

I'm on 0.14.1. What might be going on?

I'm seeing strange behavior with the following code: ```js const XLSX = require('xlsx'); const wb = XLSX.utils.book_new(); const ws = XLSX.utils.json_to_sheet([ { Product: 'p2', Qty: 2 }, ]); XLSX.utils.sheet_add_json(ws, [ { Product: 'p3', Qty: 3 }, { Product: 'p4', Qty: 4 }, { Product: 'p5', Qty: 5 }, { Product: 'p6', Qty: 6 }, { Product: 'p7', Qty: 7 }, { Product: 'p8', Qty: 8 }, ], { header: ['Product', 'Qty'], skipHeader: true, origin: -1, // append to bottom of worksheet starting on first column } ); XLSX.utils.book_append_sheet(wb, ws, 'Bug sheet'); XLSX.writeFile(wb, 'bug.ods'); ``` I expected `p3`...`p8` to be placed on rows 3 ... 8, but they ended up on rows 6 ... 11 (in LibreCalc at least). ![image](https://user-images.githubusercontent.com/33569/48968235-6353a780-efa1-11e8-98f0-3b5ed52b8eea.png) I'm on 0.14.1. What might be going on?

👍 1

dandv commented

2018-11-29 05:07:21 +00:00

(Migrated from github.com)

Did some more digging. Looks like if the JSON array has N >= 4 elements, the library inserts N - 3 empty rows, if the existing worksheet has 2 rows.

SheetJSDev commented

2018-11-29 05:16:40 +00:00

(Migrated from github.com)

The logical error is in https://github.com/SheetJS/js-xlsx/blob/master/bits/90_utils.js#L193-L199 : when origin: -1 is specified and the number of rows inserted exceeds the number of rows in the file, it is added twice.

It would probably be simpler to proactively wraparound by checking if _R < 0 in https://github.com/SheetJS/js-xlsx/blob/master/bits/90_utils.js#L186 and just adding the number of rows in the worksheet. This would also handle cases like -2 for the penultimate row.

The logical error is in https://github.com/SheetJS/js-xlsx/blob/master/bits/90_utils.js#L193-L199 : when `origin: -1` is specified and the number of rows inserted exceeds the number of rows in the file, it is added twice. It would probably be simpler to proactively wraparound by checking if `_R < 0` in https://github.com/SheetJS/js-xlsx/blob/master/bits/90_utils.js#L186 and just adding the number of rows in the worksheet. This would also handle cases like -2 for the penultimate row.

dandv commented

2018-11-29 05:30:27 +00:00

(Migrated from github.com)

Thanks for confirming and looking forward to the fix!

dandv commented

2019-03-21 02:20:09 +00:00

(Migrated from github.com)

[![Bountysource](https://api.bountysource.com/badge/issue?issue_id=66589915)](https://www.bountysource.com/issues/66589915-sheet_add_json-with-origin-1-leaves-gap-from-bottom-of-worksheet?utm_medium=shield&utm_campaign=ISSUE_BADGE)

dongido001 commented

2019-03-21 14:26:32 +00:00

(Migrated from github.com)

Made a pull request that solves the issue #1463 @dandv

👍 1

dandv commented

2019-03-21 19:52:15 +00:00

(Migrated from github.com)

Yay! First time I'm about to award a bounty, after years of using BountySource :)

@SheetJSDev, can you please test and merge if everything checks out?

Yay! First time I'm about to award a bounty, after years of using BountySource :) @SheetJSDev, can you please test and merge if everything checks out?

👍 1

dandv commented

2019-05-18 02:45:03 +00:00

(Migrated from github.com)

Hey @SheetJSDev, ping :)

👍 2

dandv commented

2020-01-06 05:07:23 +00:00

(Migrated from github.com)

Here's a 2020 ping!

😆 3

fabiooshiro commented

2020-01-12 11:03:11 +00:00

(Migrated from github.com)

workaround with the advantage of writing new columns header if your object has new properties

let arrayObjects = XLSX.utils.sheet_to_json(dataSheet);
arrayObjects.push(newObject);
XLSX.utils.sheet_add_json(dataSheet, arrayObjects);

workaround with the advantage of writing new columns header if your object has new properties ``` let arrayObjects = XLSX.utils.sheet_to_json(dataSheet); arrayObjects.push(newObject); XLSX.utils.sheet_add_json(dataSheet, arrayObjects); ```

👍 1

dandv commented

2021-01-24 07:14:28 +00:00

(Migrated from github.com)

Greetings from 2021 ❗

Greetings from 2021 :exclamation:

👍 3 😆 5

SheetJSDev commented

2021-09-13 06:23:05 +00:00

(Migrated from github.com)

This was fixed in a slightly different way (by updating the anchor row) in v0.16.0:

-		if(_R == -1) { _R = range.e.r + 1; range.e.r = _R + js.length - 1 + offset; }
+		if(_R == -1) { _R = _range.e.r + 1; range.e.r = _R + js.length - 1 + offset; }

That said, the bounty should be awarded to @dongido001 in recognition of the effort.

This was fixed in a slightly different way (by updating the anchor row) in v0.16.0: ```diff - if(_R == -1) { _R = range.e.r + 1; range.e.r = _R + js.length - 1 + offset; } + if(_R == -1) { _R = _range.e.r + 1; range.e.r = _R + js.length - 1 + offset; } ``` That said, the bounty should be awarded to @dongido001 in recognition of the effort.

Sign in to join this conversation.