fix: enable auto-complete for methods overriding

[m8524769]

No description provided.

[CLAassistant]

All committers have signed the CLA.

[bd82]

Hello @m8524769

Can you explain how this change helps support "auto-complete for methods overriding"?
Preferably with a snippet of a scenario that does not work now but would work after your change...

[m8524769]

Hi @bd82

Thanks for your reviewing.
It mainly works on hooks overriding such as onInit(), onBeforeRendering() and so on, here is a scenario:

And the current version doesn't support it, hope this helps :)

[bd82]

This GIF was very helpful, now I understand.

By add Partial<${FQN}> all the methods from the base type would be offered as optional
auto-complete properties.

Now the only questions remaining is in regards to UI5 semantics.

• Could this cause properties to be suggested that are completely wrong/invalid.
• Could this cause too many things to be suggested, e.g only 5% of possible properties can ever be overridden (spam).
• Could this cause errors, e.g: overriding property XYZ would cause UI5 runtime to explode?

@codeworrior @petermuessig : Thoughts? Feedback?

[m8524769]

I think most of the public methods in UI5 classes are allow you to override them, although it is dangerous to do so, and for some important methods, they will be marked as "final": true in the source code, see Controller.js

In the other hand, this behavior is quite similar with ES6 class using extends syntax:

BTW, I think documentation is more important and useful than auto-complete.

Anyway, this is just a small suggestion :)

[codeworrior]

@bd82 there's no simple answer to your questions.

In custom control development, it is common that subclasses override some method from their base class. Very heavy controls might have methods that they don't want to be overridden (e.g. the template method in a template/hook pattern), but that's not the 100% case.

But in application development (when subclassing Component, UIComponent, Controller or ControllerExtension), the UI5 framework only wants some explicitly documented hooks to be overridden. Overriding other methods has a high risk of causing issues - both in future versions or in unexpected contexts - and therefore should be avoided.

Unfortunately, neither JavaScript nor TypeScript provide a simple mean to technically prevent this (there's no final, see also the discussion in microsoft/TypeScript#8306). Private methods are not a full solution as there's a big difference between private ("don't use") and final ("use, but do not override"). There are some suggestions to workaround this using readonly, but as far as I understand, those workarounds can be bypassed easily.

That being said, I would love to prevent code completion for non-hooks. But a) there's no easy way to make this separation available in a generic way and b) the analogy to ES6 classes is so appealing that I have to surrender :-)

@petermuessig or @akudev: do you want to be the last man standing?

[bd82]

That being said, I would love to prevent code completion for non-hooks. But a) there's no easy way to make this separation available in a generic way and b) the analogy to ES6 classes is so appealing that I have to surrender :-)

okay I will merge this next week if there are no objections.

[bd82]

Unfortunately, neither JavaScript nor TypeScript provide a simple mean to technically prevent this

Btw, to me this sounds like the "right tool" for the job here may be custom ESLint validations

[codeworrior]

Not sure if eslint rules really work out in that context. To my understanding, one basic design rule for eslint rules is that they operate locally (on a single file) only. The checks we discuss here require knowledge about the inheritance tree, which might be spread across multiple sources / projects. At least not a "perfect fit" for eslint rules, IMO.

[bd82]

To my understanding, one basic design rule for eslint rules is that they operate locally (on a single file) only.

The eslint typescript plugin has support for more context, So perhaps it is possible in an advanced ESLint plugin.

• https://github.com/typescript-eslint/typescript-eslint#can-we-write-rules-which-leverage-type-information

[petermuessig]

@codeworrior : as a last man standing comment I can confirm, that I do not see an issue with exposing those functions in the code completion.

[bd82]

This was released in:

• https://www.npmjs.com/package/@ui5/dts-generator/v/2.2.1

Now it is up for UI5 team (@codeworrior @petermuessig ) to update the version used in the UI5 build process.