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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Incorrect Worksheet Name on Streaming XLSX Reader #1364

Closed
whizdummy opened this issue Jul 1, 2020 · 4 comments
Closed

[BUG] Incorrect Worksheet Name on Streaming XLSX Reader #1364

whizdummy opened this issue Jul 1, 2020 · 4 comments
Labels

Comments

@whizdummy
Copy link

馃悰 Bug Report

I am having problems getting correct worksheet name when using streaming xlsx reader (async iteration). The first worksheet always defaults to Sheet1 regardless of name.

Lib version: 4.1.0

excel
sheet-name

Steps To Reproduce

Sample Template.xlsx

const workbook = new stream.xlsx.WorkbookReader('path/to/Sample Template.xlsx', {});

for await (const worksheetReader of workbook) {
    console.log('sheet name:', worksheetReader.name); // It always outputs Sheet1
}

The expected behaviour:

Worksheet name should be Sum Worksheet instead of Sheet1.

@Alanscut
Copy link
Member

Alanscut commented Jul 2, 2020

@whizdummy I found that there is indeed something unreasonable in the XLSX reader.
@alubbe Do you have time to answer these questions?

Source Code

The function sheet-xform.js should be used to resolve worksheet names.

// sheet-xform.js
 parseOpen(node) {
    if (node.name === 'sheet') {
      this.model = {
        name: utils.xmlDecode(node.attributes.name),
        id: parseInt(node.attributes.sheetId, 10),
        state: node.attributes.state,
        rId: node.attributes['r:id'],
      };
      return true;
    }
    return false;
  }

The following code will build an instance of worksheet-reader, including the name of the worksheet, such as Sheet1 and Sheet2, etc.

// workbook-reader.js
  *_parseWorksheet(iterator, sheetNo) {
    this._emitEntry({type: 'worksheet', id: sheetNo});
    const worksheetReader = new WorksheetReader({workbook: this, id: sheetNo, iterator, options: this.options});
    if (this.options.worksheets === 'emit') {
      yield {eventType: 'worksheet', value: worksheetReader};
    }
  }

I think that workbook-xform.js should be used to parse the workbook.xml file.

// const WorkbookPropertiesManager = require('../../xlsx/xform/book/workbook-properties-xform');
const WorkbookManager = require('../../xlsx/xform/book/workbook-xform');

@Alanscut Alanscut added the bug label Jul 2, 2020
@alubbe
Copy link
Member

alubbe commented Jul 2, 2020

@Alanscut that looks reasonable. Could you put together a PR with a potential fix (or an attempt of it) and with a test using the provided xlsx?

@colinroemer
Copy link

Do you plan on addressing this any time soon?

@vksonagara1996
Copy link

Is there a workaround other than not using the stream interface? @Alanscut @alubbe

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

No branches or pull requests

5 participants