dfinke / ImportExcel

PowerShell module to import/export Excel spreadsheets, without Excel
https://www.powershellgallery.com/packages/ImportExcel/
Apache License 2.0
2.47k stars 400 forks source link

Getting error when worksheet is named "Package" #1064

Closed jdenicola-pa closed 3 years ago

jdenicola-pa commented 3 years ago

In this example code:

$xlSourcefile = "c:\TEMP\ImportExcelExample.xlsx"
Remove-Item $xlSourcefile -ErrorAction Ignore

1..10 | Export-Excel $xlSourcefile -WorkSheetName "Package"
11..20 | Export-Excel $xlSourcefile -WorkSheetName "Package" -StartRow 15

$excel = Open-ExcelPackage $xlSourcefile 
Close-ExcelPackage -ExcelPackage $excel -Show

When I run this, it populates the spreadsheet with the data, but it gives me an error:

Add-Member : Cannot add a member with the name "Package" because a member with that name already exists. To overwrite the member anyway, add the Force parameter to your command. At C:\Program Files\WindowsPowerShell\Modules\ImportExcel\7.2.2\Public\Open-ExcelPackage.ps1:38 char:17

  • ... Add-Member -InputObject $pkgobj -MemberType ScriptPropert ...
  • 
    + CategoryInfo          : InvalidOperation: (OfficeOpenXml.ExcelPackage:PSObject) [Add-Member], InvalidOperationEx
    ception
    + FullyQualifiedErrorId : MemberAlreadyExists,Microsoft.PowerShell.Commands.AddMemberCommand

If I change the WorkSheetName to something else - let's say "PackageX", it works without any errors.

Why does it care if the worksheet is named "Package"?

stahler commented 3 years ago

Debugging your example, I see where "Package" is a property of the OfficeOpenXml.ExcelPackage object, thus the collision.

image

jhoneill commented 3 years ago

This is something I put in a long time ago so that we refer to $excel.worksheetname as a shorthand - I didn't trap a worksheet name being the same as a property or method. It looks like you error is in public\Open-ExcelPackage.ps1 at line 38. So if you edit that and make the add-member xxxx command at line 38 try {add-member xxx} catch {} that will give you a short term fix. When I get a chance I'll it properly for the next release

dfinke commented 3 years ago

@jhoneill took a quick look, started to write a test to prove the error. Off the top of my head, looks like preventing naming a worksheet based on a get-member list of the OfficeOpenXml.ExcelPackage is the way to go.

jhoneill commented 3 years ago

@jhoneill took a quick look, started to write a test to prove the error. Off the top of my head, looks like preventing naming a worksheet based on a get-member list of the OfficeOpenXml.ExcelPackage is the way to go.

That solves it when we create the file, but if someone creates a workbook in excel with a bad name we have a problem as well. I think the simplest thing to do is to wrap the add-member in a try catch so if it doesn't like it for whatever reason we don't get the extra property but things go on as normal.

dfinke commented 3 years ago

Yep. Thought about it a bit more, needs to be solved for read and write.

dfinke commented 3 years ago

@jdenicola-pa, new version published with the fix, it now does a try/catch and issues a warning about the name if the sheet. Please try, and post any issues. Thanks @stahler and @jhoneill.

jdenicola-pa commented 3 years ago

@dfinke When I run my example above with the new version, it emits this warning:

WARNING: Could not add sheet Package as 'short cut', you need to access it via $wb.Worksheets['Package']

What would cause that?

jhoneill commented 3 years ago

@jdenicola-pa what happens is this

The workbook is an object provided by the EPPLus library we use if you send it to Get-Member in PowerShell it looks like this

Name                 MemberType
----                 ----------
Dispose                  Method
Equals                   Method
GetAsByteArray           Method
GetHashCode              Method
GetType                  Method
Load                     Method
Save                     Method
SaveAs                   Method
ToString                 Method
Compatibility          Property
Compression            Property
DoAdjustDrawings       Property
Encryption             Property
File                   Property
Package                Property
Stream                 Property
Workbook               Property

I added code so that when the workbook was opened or a sheet was added , sheet names became script-properties - they just return $this.workbook.Worksheets["Sheet1"] allowing me (or anyone) to use $excel.sheet1 instead of typing $excel.Workbook.Worksheets["sheet1"]

If there is a sheet named "Save" or "File" or "Package" this is going cause a conflict. I'm really surprised that we've gone this long and either no-one has hit the problem or everyone who did changed the name and said nothing.

Someone who expects that their new sheet "Package" will be be accessible as $excel.package should get a warning that it's not going to work - especially if they have a $sheetname variable and later try to refer to $excel.$sheetname because that's going to break. What that message is saying is "We'd prefer you not to use package as a sheet name, if you need to do that AND you need to access it from PowerShell, use workbook.worksheets['package']"

If you have this in a script and you know the same warning will appear every time, you can hide it with -warningAction Silentlycontinue.

jdenicola-pa commented 3 years ago

@jhoneill I get that, but in my initial example code, I am not referencing $excel.package or $excel.$sheetname. It's using the -WorkSheetName parameter only. So perhaps there is something in one or more of the methods that is using that instead of workbook.worksheets[]?

dfinke commented 3 years ago

@jdenicola-pa when using Open-ExcelPackage , it, under the covers, does the add-member to the object, that is where it was failing. Now it catches the error, doesn't fail, and instead shows a warning.

jdenicola-pa commented 3 years ago

@dfinke @jhoneill

Thanks for you updates and responses. Sorry, just getting back to this...

Would I then be correct in saying that this code:

    $excel = Open-ExcelPackage $xlFile
    $ws = $excel.Workbook.Worksheets["Package"]

will always produce a warning, and there is no way to suppress it? (I haven't been able to.)

Is there any way to get a reference to the worksheet with the name "Package" into a variable ($ws) without a warning?

stahler commented 3 years ago

Try $excel = Open-ExcelPackage C:\TEMP\test.xlsx -WarningAction Ignore