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.

Sign up for GitHub

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

Jump to bottom

[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
bug

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 •
edited
Loading

@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

@antimatter15 antimatter15 mentioned this issue Sep 29, 2020
Merged
@alubbe alubbe closed this as completed in 15b46a1 Oct 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@alubbe @colinroemer @whizdummy @Alanscut @vksonagara1996