XrmDefinitelyTyped icon indicating copy to clipboard operation
XrmDefinitelyTyped copied to clipboard

Modified getControl, getAttribute and get to return the basic type instead of undefined

Open mathiasbl opened this issue 11 months ago • 6 comments

My team start adopting strictest for TypeScript. https://www.npmjs.com/package/@tsconfig/strictest It's doesn't like undefined alot as return type.

The getAttribute and getControl have been changed to return the basic type matching the method.

mathiasbl avatar Aug 24 '23 17:08 mathiasbl

Hi Mathias

I'm not familiar with strictest, so it is not obvious to me, what issue you are experiencing when using strictest with XDT. Is your issue with the XDT source code? Or with the generated typings?

We currently generate the typing getAttribute(attributeName: string): undefined as we believe it gives the best developer experience. Changing it to getAttribute(attributeName: string): any changes the compile-time error to a run-time error when using undefined fields. This means that removing a field (used in code) from a form no longer causes compile-time errors (this example does require re-generating the types, which we recommend doing that as part of your CI/CD pipeline).

Return type 'undefined' gives compile-time error

Return type 'any' gives run-time error

skovlund avatar Nov 15 '23 12:11 skovlund

Agreed with @skovlund, we should not return a valid type for invalid method calls.

Would it be a better solution to let them return the never type?

mkholt avatar Nov 15 '23 14:11 mkholt

@mkholt According to the MS documentation, getControl returns an Object Collection, Object or null. Wouldn't it make sense to stick to the underlying SDK definition and at least return an XrmBaseControl, XrmBaseControl[] or null?

@jbrueschweileravantic

sideshowbob avatar May 02 '24 07:05 sideshowbob

@sideshowbob, it seems both getControl and getAttribute return null if the control / attribute is not on the entity. So we could return null as a fallback value as well.

My point in using never was that we want to avoid you ever calling using an identifier we know doesn't exist. Specifically, we should return something that will result in compile-time errors.

mkholt avatar May 02 '24 07:05 mkholt

@mkholt I see your point for resulting compile-time errors. But there are circumstances where you may explicitly want to get a control based on, say a variable that is populate at run-time. From my point of view, this prevents some generic development approaches and does not correspond with the SDK, the Types are made for.

sideshowbob avatar May 02 '24 07:05 sideshowbob

@sideshowbob, part of the intention behind the framework is also to provide compile-time safety.

I believe you would be able to do a cast if you wish to do something unsafe, which you would then show your intention of doing by doing the cast, e.g. getControl('some_control') as unknown as Xrm.CustomControl

mkholt avatar May 02 '24 07:05 mkholt