search

OfficeDev / office-js

A repo and NPM package for Office.js, corresponding to a copy of what gets published to the official "evergreen" Office.js CDN, at https://appsforoffice.microsoft.com/lib/1/hosted/office.js.
https://learn.microsoft.com/javascript/api/overview
Other
683 stars 95 forks source link

Support for non-optional parameters following optional in custom function #1749

Open vitalykuvaev opened 3 years ago

vitalykuvaev commented 3 years ago

Excel fails to load an add-in where any custom function has optional parameters preceding non-optional parameters in metadata.

Expected Behavior

Should be able to create optional and non-optional parameters in any order, as it is possible to do with Excel-DNA/UDF.

Current Behavior

Excel reports that it fails to load an add-in when any custom function has an non-optional parameter following an optional parameter.

Steps to Reproduce, or Live Example

  1. Create an add-in with manually managed JSON metadata and a custom function receiving 2 or more non-optional parameters. Make sure the custom function is available in Excel and is callable.
  2. Change only the first parameter in metadata as optional: true, rebuild abd reload the add-in. Excel 365 Web reports that the add-in can not be loaded. Desktop version just uses the previous version of the add-in.
  3. Try different combinations of parameters and values for optional. Observe how Excel 365 correctly loads the updated add-in only when all optional parameters follow non-optional or other optional parameters, while failing to load otherwise.

Context

This is working as expected in the current version of add-in developed with Excel DNA. We are currently rewriting the add-in using JS API and are trying to exactly match all function signatures. Some functions have parameters that can be omitted assuming default value, which are followed by non-optional parameters with no default value.

Your Environment

Excel 365 for OSX, Excel 365 Web. Have not been tested in Excel 365 for Windows. MacOS 10.15.7, Safari 13.1.3

ElizabethSamuel-MSFT commented 3 years ago

@vitalykuvaev Thanks for raising this issue.

@grangeryy Can you take a look, or assign to an appropriate person to investigate?

Thanks.

grangeryy commented 3 years ago

This is a new feature ask for Custom Function and I've open a backlog for it: 4991402. Will prioritize together with all exiting feature requirements. Thx for raising this!

wh1t3cAt1k commented 3 years ago

Hi @grangeryy , thank you for the response.

Technically nothing warns a user from attempting to repro those steps; the behaviour is highly cryptic, it took us a lot of time to figure out what was the root cause of the add-in not loading.

I agree that support for this functionality might be a feature request, but the way it breaks down when you try to use this functionality - is ugly 😬

Maybe, besides a backlog feature request, it deserves some separate measures to improve the feedback to the developer about why it's not working?

ElizabethSamuel-MSFT commented 3 years ago

Will it help to note this behavior in the documentation if it isn't already there?

grangeryy commented 3 years ago

Good suggestion @ElizabethSamuel-MSFT, and let me check if we could add some content to improve the documentation.

wh1t3cAt1k commented 3 years ago

Hi there! Just encountered this again with a cryptic error "Couldn't install add-in" (no further explanation given), so our engineers spent some time again figuring this out...

As a workaround, in cases when a required parameter follows an optional parameter, we have to mark it as optional, too. At the same time, we will check its presence in the application code and fail if it's an empty value.

This is, however, misleading for the users because the parameter is indicated in [SquareBrackets], but is in fact required. On our end, we have no choice but to misleadingly mark it as optional.

To give you a bit more background, we also have our legacy COM-based product, which we maintain function-level compatibility with, so we cannot reorder our arguments anymore; there are a lot of functions where an optional parameter precedes a required one, but it's supported in the legacy technology...

I would be immensely grateful if you guys could prioritise this a bit, especially if it's not a difficult fix.

wh1t3cAt1k commented 3 years ago

@grangeryy @keyur32 my apologies for nudging, I would like to ask for an update on this issue!

We have accumulated almost thirty (!) custom functions where we are forced to mark logically obligatory parameters as optional simply because they go after some optional ones.

(also we need to implement our own runtime checks that the arguments are in fact present)

We are getting unnecessary support requests because the [optional] syntax in the name misleads our customers.

Is it a lot of effort to address, and if not, could we hope for a fix soon?

keyur32 commented 3 years ago

@grangeryy - can you please help? thanks!

[MS Logo] Keyur Patel PM, Office Platform @.***

From: Pavel Kabir @.> Sent: Thursday, October 28, 2021 12:10 PM To: OfficeDev/office-js @.> Cc: Keyur Patel @.>; Mention @.> Subject: Re: [OfficeDev/office-js] Support for non-optional parameters following optional in custom function (#1749)

@grangeryyhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fgrangeryy&data=04%7C01%7Ckepatel%40microsoft.com%7Cbbc3b2ea5f054f53b17c08d99a469038%7C72f988bf86f141af91ab2d7cd011db47%7C0%7C0%7C637710450165691827%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=8Vpa46BZTDyg7Sn0uY2RngYDNr4rYyhu1%2FbDiC49Ado%3D&reserved=0 @keyur32https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fkeyur32&data=04%7C01%7Ckepatel%40microsoft.com%7Cbbc3b2ea5f054f53b17c08d99a469038%7C72f988bf86f141af91ab2d7cd011db47%7C0%7C0%7C637710450165701785%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=8S5g9JAxTlj584p0eLRlIbsiU7yBK8qQro%2BwSD6RCTg%3D&reserved=0 my apologies for nudging, I would like to ask for an update on this issue!

We have accumulated almost thirty (!) custom functions where we are forced to mark logically obligatory parameters as optional simply because they go after some optional ones.

We are getting unnecessary support requests because the [optional] syntax in the name misleads our customers.

Is it a lot of effort to address, and if not, could we hope for a fix soon?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FOfficeDev%2Foffice-js%2Fissues%2F1749%23issuecomment-954124485&data=04%7C01%7Ckepatel%40microsoft.com%7Cbbc3b2ea5f054f53b17c08d99a469038%7C72f988bf86f141af91ab2d7cd011db47%7C0%7C0%7C637710450165711735%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=484sSCgdoToZ0ejRUtipOgZkNtMdclxgh1ukcdOlg8o%3D&reserved=0, or unsubscribehttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAA2QU546ZYB5FS25FYLHBE3UJGNZFANCNFSM4ZZZGIPA&data=04%7C01%7Ckepatel%40microsoft.com%7Cbbc3b2ea5f054f53b17c08d99a469038%7C72f988bf86f141af91ab2d7cd011db47%7C0%7C0%7C637710450165721698%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=7B0WCeht%2BjKdZZgNnEdDio9dG3Xt8JVOWRwH2dy4plA%3D&reserved=0.

grangeryy commented 3 years ago

@wh1t3cAt1k , really sorry to hear the issues here. Unfortunately, we don't have a quick fix solution for this, which means to support the requirement, it will take some time. Trying to understand more about the impact here, how many of the customers are actually impacted by the 30 functions? Except for the existing 30 custom functions, how many remaining functions also need to make changes? Thx!

wh1t3cAt1k commented 3 years ago

@grangeryy we have several hundred clients on our XLL add-in with many of those expressing interest to work on Mac and Online, for which we direct them towards the new OfficeJS based add-in we're developing.

We don't have proper usage statistics yet but I think a sizeable portion of them are already using, or tried out, or plans to use, the cross-platform add-in.

The problem with misleading "optional" parameters (as I mentioned we have to make them optional to work around the limitation mentioned in this thread) exists in our core functions for retrieving an ERP account balance, which nearly 100% of our clients use.

Altogether the problem exists in over a third of our functions, we now have about 80 in total AFAIR.

So while we worked around this by making run-time checks that these parameters are required, I'd be glad if we could properly mark them as required in the metadata.

wh1t3cAt1k commented 2 years ago

@yaweizhu-henson @JinghuiMS we are now at the level of around 1000 clients and 100 functions, and would like to intensify migration to the office.js based solution.

Unfortunately, this issue is a support workload for our small company - since it's more than 1.5 years old, is there any chance it will be addressed in the near future?

wh1t3cAt1k commented 10 months ago

This issue will soon celebrate its third birthday! 🎂 can it be fixed anytime soon?

We are at a point where we're considering to mark all our parameters as optional and take all validation to userland...