ui5-typescript icon indicating copy to clipboard operation
ui5-typescript copied to clipboard

[Discussion] Support for control development with new "ts-types-esm" type definitions

Open akudev opened this issue 3 years ago • 6 comments

This is the place to discuss control development support with the new ES module version of the UI5 type definitions, the one recommended to be used going forward.

This is also a continuation of https://github.com/r-murphy/babel-plugin-transform-modules-ui5/pull/66, which turned off-topic for the location where it was discussed.

Problem

Accessor methods for the properties, aggregations etc. (functions like getMyProperty() and addMyAggregation() and attachSomeEvent()) are in general not explicitly written in the control code, but generated by UI5 at runtime. This means they are not known to TypeScript at development and build time.

But even the control's own renderer will have to do things like

oRenderManager.text(oMyControlInstance.getAdditionalText());

As the getAdditionalText() getter for the "additionalText" property is not known to TypeScript, the method call will be marked as error. In addition, TypeScript also doesn't know that new MyControl({additionalText: "hi"}) is valid, but new MyControl({thisDoesntExist: "hi"}) or new MyControl({additionalText: {someProp: 3}})is not.

So the control developers themselves, but also application developers using the control, will get TypeScript errors (and no code assist) for all accessor method calls and for the cosntructor settings object.

One could argue that defining the property getter manually is not a big deal, but once you think about application developers using a control developed by others, all potentially used methods need to be declared. For properties this is relatively simple (although e.g. things like bindXY()for bindable properties could be easily forgotten). But for one single aggregation, it's already quite a list of methods that need to be declared correctly:

        // aggregation: content
        getContent(): Control[];
        addContent(content: Control): this;
        insertContent(content: Control, index: number): this;
        removeContent(content: number | string | Control): this;
        removeAllContent(): Control[];
        indexOfContent(content: Control): number;
        destroyContent(): this;
        bindContent(bindingInfo: AggregationBindingInfo): this;
        unbindContent(): this;

Solution

According to current thinking, the best solution is generating a TypeScript interface, which declares the missing methods and the constructor settings object type. This generation cannot happen in the TypeScript transpilation, but needs to happen in parallel to development: not only the build result needs to contain the additional interfaces, but even the sources - otherwise the solution does not work for the control developer.

While the interface content itself is straightforward, it is more tricky to make TypeScript recognize that it extends the developed control class. Generating the interface right into the control file would work, but was discarded as this can interfere with the manual coding that is happening in parallel. The currently planned solution makes use of TypeScript's module augmentation and interface merging features to accomplish the enrichment from a stand-alone generated file next to the developed control file.

A generated file looks like this right now:

import { $ControlSettings } from "sap/ui/core/Control";

declare module "./MyControl" {
    /**
     * Interface defining the settings object used in constructor calls
     */
    interface $MyControlSettings extends $ControlSettings {
        text?: string;
    }

    interface MyControl {
        // property: text
        getText(): string;
        setText(text: string): this;
    }
}

Caveats

As interfaces cannot define constructors, the different constructor signatures still have to be defined within the original control class. But at least they don't need to be changed after creation, as they are independent from the control API definition.

To make the interface merging work, the original control module needs to export the control class not only as default export (which is needed for regular usage of the control in the same way the standard controls of UI5 work), but also as named export in addition. This is a bit hacky, as discussed in https://github.com/r-murphy/babel-plugin-transform-modules-ui5/pull/66. An alternative might be to require an explicit import of the interfaces (more manual one-time effort; current proposal does not seem to work).

Discussion

This issue is meant to bundle discussions and feedback and pull them away from other places, where it would be off-topic. Suggestions how to work around the caveats, or alternative ideas are welcome!

The code generating the current version of the interfaces is right now not available on GitHub, but the plan is to offer it, once a home is found.

akudev avatar Oct 26 '21 10:10 akudev

Hello @akudev,

why not just make the following change in the generated .ts-file:

    export default interface MyControl {
        // property: text
        getText(): string;
        setText(text: string): this;
    }

By adding the export default clause in the generated interface you also do not need the named export in the main control file. I am doing the same thing to augment types from the UI5 library - albeit in .d.ts files, but it also works with .ts files (see sample here Enhanced sap.ui.comp types).

Best Regards, Ludwig

stockbal avatar Nov 19 '21 17:11 stockbal

Hi @stockbal,

Ouch! Of course, that's the solution! :-) Sometimes you don't see the forest due to all the trees...

Thanks a lot! Andreas

akudev avatar Nov 25 '21 16:11 akudev

As general remark: the interface generator tool is now available via npm as "@ui5/ts-interface-generator"

akudev avatar Nov 25 '21 16:11 akudev

@stockbal, I have applied the export default in the tool generating the interfaces and released version 0.2.0.

Unfortunately the change was not so straightforward: if I only add the "export default", TypeScript complains about not being able to find the name MyControl in those places where it is used within the definitions of the getters and setters and within the constructor settings object. It doesn't seem to make a lot of sense, but that's the error I get: image

It does work to define the interface twice (once without export, once as default export), so that's what the tool generates now until we figure out a better way. Better have a weird generated file than something weird to add to the manually coded file... image

akudev avatar Nov 28 '21 21:11 akudev

Hi @akudev,

I am not sure if this is a bug of typescript but the reason you are getting the syntax error is because you splitted the class definition and the export clause in file MyControl.ts.

If you write it like that:

/**
 * @namespace ui5.typescript.helloworld.control
 */
export default class MyControl extends Control {
...

the syntax error should dissappear and the second module declaration in the generated file is no longer needed.

Best regards, Ludwig

stockbal avatar Nov 28 '21 23:11 stockbal

@stockbal Cool, thanks again! It's indeed surprising that it makes a difference whether a class is exported immediately or separately, but anyway: it works. I'll adapt the generator. If you have more good ideas, keep them coming! ;-)

Regards Andreas

akudev avatar Dec 03 '21 18:12 akudev

Discussion closed.

akudev avatar May 26 '23 14:05 akudev