advisr-io / excel4node

Node module to allow for easy Excel file creation
MIT License
124 stars 18 forks source link

Long worksheet names interpreted as corrupted file (MS Excel) #84

Open jeff-r-koyaltech opened 9 months ago

jeff-r-koyaltech commented 9 months ago

Describe the bug When opening spreadsheets generated by this library in Microsoft Excel, if any worksheet has a name longer than 31 characters, there is a warning message that appears, and asks the user if they want to repair the file. (This doesn't seem to happen in other products, such as Libre Office). This happens to me on both Mac and Windows machines in Microsoft Excel.

To Reproduce Add link to gist that will replicate issue here https://gist.github.com/jeff-r-koyaltech/ca06f246726040220150f19554f2d10d

Expected behavior A clear and concise description of what you expected to happen.

Environment (please complete the following information):

Additional context

The message says "We found a problem with some content in 'temp.xlsx'. Do you want us to try to recover as much as we can? If you trust the source of this workbook, click Yes."

If you view the repairs, it opens a Word file with the following contents: "Repair Result to temp0.xml Errors were detected in file '/Users/.../temp.xlsx'

Repaired Records: Worksheet properties from /xl/workbook.xml part (Workbook) "

Googling that last line, I was able to find this snippet of info online: https://stackoverflow.com/questions/48643025/sheetjs-repaired-records-worksheet-properties-from-xl-workbook-xml-part-work

And that led me to the workaround of truncating characters to be less than 31 characters. I don't know what a proper fix is for this "feature" or "bug". I could understand not wanting to code against these kinds of specifics in your library. But perhaps it should be a known issue at minimum. A console warning about long worksheet names might be another nice way to resolve this, so that users are aware they're generating something that may throw up a nasty message for their users? I'm flexible on how it's fixed, because in any case, it's quite trivial to workaround once you know about it.

I'm also willing to submit a PR to fix it as part of giving back to this community. Just let me know how you would want me to go about the fix/solution. Thanks!

TRONJon commented 7 months ago

Thank you Jeff for raising this one!

This issue has taken my team a while to understand. Microsoft's cryptic "Do you want us to recover as much as we can?" left me trying all kinds of tests until I realised it was related to Worksheet name length, which lead me to this issue. I'm glad it wasn't just us running into this.

I hope a warning or fix can be rolled out, or at least the admin lets a PR go in for it - would save other developers a lot of debugging time.

Jon