fix(stream): read xlsx via central directory so streaming WorkbookReader stops losing the workbook entry#86
Open
amir-arad-lmnd wants to merge 1 commit into
Conversation
…der stops losing the workbook entry stream.xlsx.WorkbookReader intermittently threw "Cannot read properties of undefined (reading 'sheets')" in _parseWorksheet on Node >= 18 (~90% of reads on Node 22). parse() read the archive from a single streaming unzip pass in stored order; because xl/workbook.xml is written last, that pass often did not deliver it to _parseWorkbook before the worksheets were parsed, leaving this.model undefined. Read the archive through its central directory (unzipper.Open) instead: parse rels/workbook/sharedStrings/styles before any worksheet, then emit worksheets in order, still consuming each row-by-row so the workbook is never materialized in memory. Removes the temp-file worksheet spooling and the tmp dependency usage. Fixes exceljs#3064.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the intermittent streaming-reader crash (upstream exceljs#3064):
Cannot read properties of undefined (reading 'sheets')on Node >= 18.Root cause
The old
parse()used a single streamingunzip.Parsepass, emitting entries in stored order. Becausexl/workbook.xmlis typically written last, worksheets can be parsed beforethis.modelis set. The guard only checkedthis.sharedStrings && this.workbookRels, notthis.model, sothis.model.sheetsthrew intermittently depending on ZIP entry ordering + async-iteration timing on Node >= 18.Fix
Read the archive through its central directory (
unzip.Openvia_openZip) so every part is addressable. Dependency entries (rels, workbook.xml, sharedStrings, styles) are always parsed before any worksheet, guaranteeingthis.modelis present. Worksheets are still consumed lazily viaiterateStream, so the full workbook is never held in memory.Notes
spec/integration/issues/issue-3064-streaming-workbook-model.spec.js.protobi/master(conflict inparse()resolved in favor of the central-directory implementation).