Azure / azure-sdk-for-js

This repository is for active development of the Azure SDK for JavaScript (NodeJS & Browser). For consumers of the SDK we recommend visiting our public developer docs at https://docs.microsoft.com/javascript/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-js.
MIT License
2.03k stars 1.19k forks source link

Types are too loosely defined to be useful #24613

Open FraserMuir opened 1 year ago

FraserMuir commented 1 year ago

Describe the bug When using the declared types within the @azure/arm-securityinsight every property is optional and hence could be undefined, even in cases where it is obvious the underlying service (Sentinel in this case) will definitely return required properties (IDs for example)

To Reproduce Steps to reproduce the behavior:

  1. Get a typed response from any method within the Security Insights client.
  2. Access a properly that surely must be required in Sentinel (the id of the incident)
  3. Try to do anything with the id, and you must first check that it is defined, since it's type is string | undefined

Expected behavior Properties like IDs (and many others) will obviously always be present of their respective entities. These should not be optional properties on their declared types.

Screenshots image

image

Additional context I don't believe it is acceptable to expect the end user of the types exported from this package to conditionally access all properties on Incident objects (also applied to most others such as Entity, Alert, Comment) in cases where these properties must surely not actually be optional in practise.

xirzec commented 1 year ago

@qiaozha looks like a bad swagger input, maybe we can reach out to the service team or do a few transforms?

qiaozha commented 1 year ago

I will send an email to service team about this.

FraserMuir commented 1 year ago

any update on this?

qiaozha commented 1 year ago

I think this is a limitation of the current design. I will update this issue when we have a better design to resolve this issue. Please note that, it will probably take a long time to support it.