@BridgeAR thanks for suggestion!

while it seems reasonable to index from 0, i fear such a change of interface would break a lot of dependent code since it is possible to access sheets using their generated id:

workbook.addWorksheet('first')workbook.getWorksheet(0) //results in undefinedworkbook.getWorksheet(1) //results in 'first' worksheet

there seem to be however more places where the loose array poses a problem - i will look into it deeper next week

ps: you are 100% right filtering everything is an overkill - simply slicing the first item would be enough

@pookong yes, this is a minimal behavior change but at least right now I do not see a better way to do this.

It would be nice if semver would be used but this is not the case, so breaking changes might be in any update but I might be wrong.

And this is a very trivial bug fix that fixes lots of side effects opposed to quite a few difficulties to work around this and keep the current behavior.

@guyonroche could you give us a brief update on your view to this issue? :-)

@pookong,@BridgeAR
Welcome to the wonders that is xlsx file format :-)
Many of the indexes in the document are indeed 1 based - which is why nextId starts at one and why workbook.getWorksheet(0) will always be undefined.

I've added unit-tests to cover the cannot read property 'name' of undefined issue

I'll investigate the effects of this PR this weekend

I think filtering is fine - after all, the arrays of worksheets will typically be small.
I would prefer not to change the behaviour of workbook.getWorksheet(0) as that would mean a breaking change. From what I can see, this PR doesn't make any interface changes and it does fix the vector length issue.

@guyonroche well in that case I recommend to change thegetWorksheet method.

There does not have to be a breaking change by just fixing the getter in the calling method. But using sparse arrays is not a good idea and will likely have more side effects than here visible. And I guess you do not want to special handle zero everytime.

So I'd say a better solution would be something like:

classWorkbook{// ...getWorksheet(index){if(typeofindex==='number'){if(index<=0){thrownewTypeError('The index has to be 1 or above')}index--// Do original stuff}else ...}}