SAP / ui5-typescript

Tooling to enable TypeScript support in SAPUI5/OpenUI5 projects
https://sap.github.io/ui5-typescript
Apache License 2.0
201 stars 28 forks source link

Inconsistencies between documentation and TS types #329

Open iljapostnovs opened 2 years ago

iljapostnovs commented 2 years ago

This issue is regarding fire<EventName> and similar methods, which are marked as protected in official documentation, however they are public in TS types. Let's take an example: sap.m.ComboBox event: change method: fireChange API: image

TS: image

So it's either wrong in TS, or in official API. Please let me know if it is official API issue, in such case I will create issue there.

codeworrior commented 2 years ago

No, it's not an official API issue. The TypeScript generation doesn't distinguish between protected and public.

protected is a bit special in UI5 APIs. When we started using it, "protected" had no meaning in JavaScript. We started to use it in control development, where the standard meaning (limited to the inheritance tree) did fit quite well. But over time, the meaning in UI5 evolved to "intended for control development, but not for applications". This no longer fitted the standard meaning 100%. There's an additional developer guide page underway (since months, I have to check what the status is) which should explain this better (kind of legend for the API reference).

I think the dts-generator ignored the @protected flag due to this mismatch.

iljapostnovs commented 2 years ago

Clear, thanks for the response. Would be nice to have proper access level modifiers in TS, it's pretty necessary thing in OOP world ;)

iljapostnovs commented 2 years ago

No, it's not an official API issue. The TypeScript generation doesn't distinguish between protected and public.

protected is a bit special in UI5 APIs. When we started using it, "protected" had no meaning in JavaScript. We started to use it in control development, where the standard meaning (limited to the inheritance tree) did fit quite well. But over time, the meaning in UI5 evolved to "intended for control development, but not for applications". This no longer fitted the standard meaning 100%. There's an additional developer guide page underway (since months, I have to check what the status is) which should explain this better (kind of legend for the API reference).

I think the dts-generator ignored the @protected flag due to this mismatch.

Well, access level modifiers have universal meaning in OOP world, you will not find different public, private and protected implementation in different languages. E.g. if you take protected in C#, Java, Scala or any other language, they all are the same. That means, that visibility in UI5 was misused from the very beginning. As for now, I see that either official API should be adjusted correctly, or TS.

codeworrior commented 2 years ago

Not sure if is worth to start this discussion again. But there is not a single, universal meaning for access modifiers. Different languages differ in detail, especially for protected.

If you want, what we do in UI5 is quite similar to what C++ does by allowing friends to access protected APIs. But without having a @friends construct, there's currently no easy way to document this properly in the UI5 API reference.

Anyhow, if you stick to the universal meaning, you shouldn't do any mistakes.

iljapostnovs commented 2 years ago

Not sure if is worth to start this discussion again. But there is not a single, universal meaning for access modifiers. Different languages differ in detail, especially for protected.

If you want, what we do in UI5 is quite similar to what C++ does by allowing friends to access protected APIs. But without having a @friends construct, there's currently no easy way to document this properly in the UI5 API reference.

Anyhow, if you stick to the universal meaning, you shouldn't do any mistakes.

Agreed :) Indeed, they differ a bit, but the common part is that protected is meant for derived classes everywhere. image

C++ does have "friends", you are correct on that. However typescript doesn't have such features (or I don't know about them). What would be your next steps regarding this? If we stick to C++ definition of protected, we loose TS support for it. Are there any plans to adjust API closer to TS? Or there will be a separate "UI5 Protected" term, which will not be controlled by TS?

Edit: Is TS used internally for developing UI5? If no, I don't see what issue there would be to add members as protected for public usage.