Documentation / Feature request: How to use controller extensions in TS?

[vonBrax]

Documentation / Feature request: How to use controller extensions in TS?

[codeworrior]

Our thinking goes into the same direction. Either we introduce a marker method on Controller.prototype, something like

   myExtension = this.registerExtension(MyExtensionClass)

that makes the babel-plugin-transform-modules-ui5 move the field into the extend call.

Or we use a stage-3 decorator for that purpose, which looks a bit nicer in the code. Only drawback is that we need a new delivery for TypeScript content (the decorator), which we don't have yet.

That's something we still have to decide on, as @akudev mentioned.

[akudev]

Hi @vonBrax, this topic has come up recently and we are actively looking into it. Nevertheless thanks for reporting it in detail with your current workarounds. This gives additional insight, confirms the need, and finally makes the problem listed here. The difficulty is that those extensions are not "some specific class properties", but could have any name, hence we need something more explicit.

[vonBrax]

Hey @akudev, thanks for replying. The way I fixed this problem before was creating a generic "preserve" (or whatever name) tag and decorator in the babel plugin, to let it know that these class members should not be moved around and instead should go directly to the extends object. The advantages are that the logic wouldn't depend on the name of the class property itself and that you could use this tag to flag any other property that you want to be left untouched. The disadvantage is that we would need to remember to "mark" these properties and it may not be immediately clear why the controller extensions are not working. But maybe that could be a relatively easy fix?

[akudev]

@vonBrax When you extended that transformer plugin to keep things outside the constructor which were annotated with this specific JSDoc tag you invented, how did you deal with the fact that from TypeScript perspective the extension class was assigned, not an instance?

You wrote:

/**
 * @keepThisOutOfTheConstructor
 */
this.routing = Routing;

someMethod() {
   this.routing.navigate(...); // TS would complain here, as it thinks "routing" is the class, not an instance
}

EDIT: oh, you mentioned a decorator as well, then maybe this one has created what was needed for TS.

[vonBrax]

That's actually a good point, I forgot we would still need to type cast it. The decorators we were using were just empty decorators, since the plugin support for decorators is very limited. I did work on a complete rewrite of the plugin, with better integration with other babel plugins and decorators support, but I never published it, I gave up when I saw that it was being actively developed again by the ui5-community 🤷‍♂️

Anyways, we were just type casting it, we were mainly focused on having a working code and there were only a few places where we had TS controllers with extensions. I think an ideal solution could be what @codeworrior mentioned, but I'm assuming it would take some time before it lands. For a short term fix, I'm imagining something like:

/**
 * @controllerextension
 */
this.routing = new Routing();

someMethod() {
  this.routing.navigate(...); // OK, TS doesn't complain
}

Then in the plugin, for properties marked with the @controllerextension tag, convert the babel NewExpression to an Identifier, so we would end up with:

....extend('com.example.controller', {
  ...,
  routing: Routing,
  ...
});

But I imagine some people woudln't agree with that, since we are changing the meaning of the code with the babel plugin.

I still think it would be a good idea to have a generic tag to preserve some properties, though. We were using it both for the controller extensions and inside the controller extension for the override property, and maybe there could be even more use cases where it would be helpful.

[dfenerski]

It has always worked for me, am I missing something?

.babelrc.json:

    "presets": [
        [
            "transform-ui5",
            {
                "onlyMoveClassPropsUsingThis": true
            }
        ],
        ["@babel/preset-typescript", {}]
    ]

Stuff.extension.ts:

/**
 * @namespace my.ns.foo.bar
 */
class StuffExtension
    extends ControllerExtension{
// this works
public static metadata = { ... };
// this works too
public static override = { ... };
// add any methods as normal
public foo(){
    throw 'bar';
}

// anything else

}
// Lie to the type system because ui5 will instantiate the extension at runtime
export default StuffExtension as unknown as UI5ControllerExtension<
    typeof StuffExtension
>;

UI5ControllerExtension.ts:

export type UI5ControllerExtension<T extends new (...args: any) => any> =
    InstanceType<T> & {
        override(...args: any[]): InstanceType<T> & { override(): never };
    };

Demo.controller.ts:

    public readonly StuffExtension = StuffExtension.override({
        foo: function () { doWork() },
    });
   // just `public readonly StuffExtension = StuffExtension` works ofc too

The only problem I have with it does not relate to TS - ControllerExtension does not allow subclassing. This hinders reusal. AFAIK it has something to do with the fact it inherits from BaseObject which is the earlies object in the main hierarchy but can't really say as I haven't done the time to research this more thoroughly.

[akudev]

@dfenerski Well, the "Lie to the type system" part is something usually not found in the Controller Extensions provided by Fiori Elements. That's what I meant in my comment elsewhere. The rest does work when you use onlyMoveClassPropsUsingThis (which, as I stated there as well, is probably not meant to be how to best use the transformer, as this is only a compatibility switch to restore the legacy behavior). So yes, with this type hack and the compatibility switch I guess it works.

The extension issue you also reported sounds strange. Even sap.ui.base.Object, from which it inherits, has the extend method and I don't see any attempt in its code to prevent this for subclasses.

[akudev]

While that other change provided support for the main use-case, the solution is not completely covering extended use-cases and people are still looking into comprehensive support and documentation, so I'll re-open this.