Mbostock/xlsx tweaks (#254)

* Update xlsx.js

* Update xlsx.js

* Use Object.create(null)

* Prefer public ExcelJS APIs (#255)

* Use Object.create(null)

* Prefer public ExcelJS APIs

* Use latest tap API

* Coerce header row values to strings before fallback check

* Update src/xlsx.js

Co-authored-by: Mike Bostock <[email protected]>

* Public API

Co-authored-by: Visnu Pitiyanuvath <[email protected]>

Author: mbostock

package.json

@@ -32,6 +32,7 @@
   },
   "devDependencies": {
     "eslint": "^7.18.0",
+    "exceljs": "^4.3.0",
     "husky": "^4.3.8",
     "node-fetch": "^2.6.1",
     "rollup": "^2.37.1",

src/xlsx.js

@@ -21,26 +21,25 @@ export class Workbook {
   }
 }
 
-function extract(sheet, {range, headers = false} = {}) {
+function extract(sheet, {range, headers} = {}) {
   let [[c0, r0], [c1, r1]] = parseRange(range, sheet);
-  const headerRow = headers && sheet._rows[r0++];
+  const headerRow = headers ? sheet._rows[r0++] : null;
   let names = new Set(["#"]);
   for (let n = c0; n <= c1; n++) {
-    let name = (headerRow ? valueOf(headerRow._cells[n]) : null) || toColumn(n);
+    const value = headerRow ? valueOf(headerRow.findCell(n + 1)) : null;
+    let name = (value && value + "") || toColumn(n);
     while (names.has(name)) name += "_";
     names.add(name);
   }
   names = new Array(c0).concat(Array.from(names));
 
   const output = new Array(r1 - r0 + 1);
   for (let r = r0; r <= r1; r++) {
-    const row = (output[r - r0] = Object.defineProperty({}, "#", {
-      value: r + 1,
-    }));
-    const _row = sheet._rows[r];
-    if (_row && _row.hasValues)
+    const row = (output[r - r0] = Object.create(null, {"#": {value: r + 1}}));
+    const _row = sheet.getRow(r + 1);
+    if (_row.hasValues)
       for (let c = c0; c <= c1; c++) {
-        const value = valueOf(_row._cells[c]);
+        const value = valueOf(_row.findCell(c + 1));
         if (value != null) row[names[c + 1]] = value;
       }
   }
@@ -52,14 +51,16 @@ function extract(sheet, {range, headers = false} = {}) {
 function valueOf(cell) {
   if (!cell) return;
   const {value} = cell;
-  if (value && value instanceof Date) return value;
-  if (value && typeof value === "object") {
-    if (value.formula || value.sharedFormula)
+  if (value && typeof value === "object" && !(value instanceof Date)) {
+    if (value.formula || value.sharedFormula) {
       return value.result && value.result.error ? NaN : value.result;
-    if (value.richText) return value.richText.map((d) => d.text).join("");
+    }
+    if (value.richText) {
+      return richText(value);
+    }
     if (value.text) {
       let {text} = value;
-      if (text.richText) text = text.richText.map((d) => d.text).join("");
+      if (text.richText) text = richText(text);
       return value.hyperlink && value.hyperlink !== text
         ? `${value.hyperlink} ${text}`
         : text;
@@ -69,6 +70,10 @@ function valueOf(cell) {
   return value;
 }
 
+function richText(value) {
+  return value.richText.map((d) => d.text).join("");
+}
+
 function parseRange(specifier = ":", {columnCount, rowCount}) {
   specifier += "";
   if (!specifier.match(/^[A-Z]*\d*:[A-Z]*\d*$/))

test/xlsx-test.js

@@ -1,41 +1,31 @@
 import {test} from "tap";
 import {Workbook} from "../src/xlsx.js";
+import ExcelJS from "exceljs";
 
-function mockWorkbook(contents, overrides = {}) {
-  return {
-    worksheets: Object.keys(contents).map((name) => ({name})),
-    getWorksheet(name) {
-      const _rows = contents[name];
-      return Object.assign(
-        {
-          _rows: _rows.map((row) => ({
-            _cells: row.map((cell) => ({value: cell})),
-            hasValues: !!row.length,
-          })),
-          rowCount: _rows.length,
-          columnCount: Math.max(..._rows.map((r) => r.length)),
-        },
-        overrides
-      );
-    },
-  };
+function exceljs(contents) {
+  const workbook = new ExcelJS.Workbook();
+  for (const [sheet, rows] of Object.entries(contents)) {
+    const ws = workbook.addWorksheet(sheet);
+    for (const row of rows) ws.addRow(row);
+  }
+  return workbook;
 }
 
 test("FileAttachment.xlsx reads sheet names", (t) => {
-  const workbook = new Workbook(mockWorkbook({Sheet1: []}));
+  const workbook = new Workbook(exceljs({Sheet1: []}));
   t.same(workbook.sheetNames, ["Sheet1"]);
   t.end();
 });
 
 test("FileAttachment.xlsx sheet(name) throws on unknown sheet name", (t) => {
-  const workbook = new Workbook(mockWorkbook({Sheet1: []}));
+  const workbook = new Workbook(exceljs({Sheet1: []}));
   t.throws(() => workbook.sheet("bad"));
   t.end();
 });
 
 test("FileAttachment.xlsx reads sheets", (t) => {
   const workbook = new Workbook(
-    mockWorkbook({
+    exceljs({
       Sheet1: [
         ["one", "two", "three"],
         [1, 2, 3],
@@ -50,13 +40,15 @@ test("FileAttachment.xlsx reads sheets", (t) => {
     {A: "one", B: "two", C: "three"},
     {A: 1, B: 2, C: 3},
   ]);
+  t.equal(workbook.sheet(0)[0]["#"], 1);
+  t.equal(workbook.sheet(0)[1]["#"], 2);
   t.end();
 });
 
 test("FileAttachment.xlsx reads sheets with different types", (t) => {
   t.same(
     new Workbook(
-      mockWorkbook({
+      exceljs({
         Sheet1: [
           [],
           [null, undefined],
@@ -79,7 +71,7 @@ test("FileAttachment.xlsx reads sheets with different types", (t) => {
   );
   t.same(
     new Workbook(
-      mockWorkbook({
+      exceljs({
         Sheet1: [
           [
             {richText: [{text: "two"}, {text: "three"}]}, // A
@@ -112,7 +104,7 @@ test("FileAttachment.xlsx reads sheets with different types", (t) => {
   );
   t.same(
     new Workbook(
-      mockWorkbook({
+      exceljs({
         Sheet1: [
           [
             {formula: "=B2*5", result: 10},
@@ -131,7 +123,7 @@ test("FileAttachment.xlsx reads sheets with different types", (t) => {
 
 test("FileAttachment.xlsx reads sheets with headers", (t) => {
   const workbook = new Workbook(
-    mockWorkbook({
+    exceljs({
       Sheet1: [
         [null, "one", "one", "two", "A", "0"],
         [1, null, 3, 4, 5, "zero"],
@@ -156,9 +148,10 @@ test("FileAttachment.xlsx reads sheets with headers", (t) => {
 });
 
 test("FileAttachment.xlsx throws on invalid ranges", (t) => {
-  const workbook = new Workbook(mockWorkbook({Sheet1: []}));
+  const workbook = new Workbook(exceljs({Sheet1: []}));
   const malformed = new Error("Malformed range specifier");
 
+  t.throws(() => t.same(workbook.sheet(0, {range: 0})), malformed);
   t.throws(() => t.same(workbook.sheet(0, {range: ""})), malformed);
   t.throws(() => t.same(workbook.sheet(0, {range: "-:"})), malformed);
   t.throws(() => t.same(workbook.sheet(0, {range: " :"})), malformed);
@@ -174,7 +167,7 @@ test("FileAttachment.xlsx throws on invalid ranges", (t) => {
 
 test("FileAttachment.xlsx reads sheet ranges", (t) => {
   const workbook = new Workbook(
-    mockWorkbook({
+    exceljs({
       Sheet1: [
         [0, 1, 2, 3, 4, 5, 6, 7, 8, 9],
         [10, 11, 12, 13, 14, 15, 16, 17, 18, 19],
@@ -248,31 +241,22 @@ test("FileAttachment.xlsx reads sheet ranges", (t) => {
   t.end();
 });
 
-test("FileAttachment.xlsx throws on unknown range specifier", (t) => {
-  const workbook = new Workbook(mockWorkbook({Sheet1: []}));
-  t.throws(() => workbook.sheet(0, {range: 0}));
-  t.end();
-});
-
 test("FileAttachment.xlsx derives column names such as A AA AAA…", (t) => {
-  const l0 = 26 * 26 * 26 + 26 * 26 + 26;
+  const l0 = 26 * 26 * 23;
   const workbook = new Workbook(
-    mockWorkbook({
+    exceljs({
       Sheet1: [Array.from({length: l0}).fill(1)],
     })
   );
   t.same(
-    workbook.sheet(0, {headers: false}).columns.filter((d) => d.match(/^A*$/)),
+    workbook.sheet(0).columns.filter((d) => d.match(/^A+$/)),
     ["A", "AA", "AAA"]
   );
-  const workbook1 = new Workbook(
-    mockWorkbook({
-      Sheet1: [Array.from({length: l0 + 1}).fill(1)],
-    })
-  );
-  t.same(
-    workbook1.sheet(0, {headers: false}).columns.filter((d) => d.match(/^A*$/)),
-    ["A", "AA", "AAA", "AAAA"]
-  );
+  t.end();
+});
+
+test("FileAttachment.sheet headers protects __proto__ of row objects", (t) => {
+  const workbook = new Workbook(exceljs({Sheet1: [["__proto__"], [{a: 1}]]}));
+  t.not(workbook.sheet(0, {headers: true})[0].a, 1);
   t.end();
 });