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

Controller extension issues when using typescript #332

Closed heimwege closed 1 year ago

heimwege commented 2 years ago

I'm trying to switch a fiori elements odata v4 app to typescript and am facing the issue that the override objects need to be brought into the extend call, just like the metadata object for a control. However even if that was ok, there is also a static function called override on the ControllerExtension type which will contradict with this definition and create type conflict that can only be resolved through a ts-ignore.

Do you have any guidance for this ?

sufiyan63 commented 2 years ago

Hi, We are also facing the same issue. Could you please provide any update on this.

akudev commented 2 years ago

Hi, this is currently being discussed. Bringing the override object into the extend call is a simple change in the transformer, but yeah, then there is the name clash.

For this clash we currently see 3 potential solutions:

  1. renaming/establishing a different name than "override" in UI5
  2. establishing a different name in TypeScript and convert the name to "override" in the mentioned transformer
  3. introducing decorators in TS

the pros and cons:

  1. Not nice to have a second name, might create confusion. We’re in favor of the name "overrides" for the metadata and "override" for the static API method. But we have no information, which variant of "override" is currently more in use outside. It depends a bit on that whether we prefer to rename the metadata part or the API part.
  2. Adding a 2nd name to the transpilation looks only like a temporary solution as it also will cause confusion and create a documentation problem when the documentation depends on the used programming language. Ideally, APIs should be consistent between JS runtime and TS world.
  3. As long as decorators are not settled in TC39, we tend not to adopt them as a public offering of UI5.

The UI5 core team is going to work on documenting extensibility in UI5 better. Insights from that work might help to decide between the options.

Regards Andreas

heimwege commented 2 years ago

Hi @akudev ,

any updates on this one?

Best Regards Dominik

akudev commented 2 years ago

None that I heard of. :-( I'll inquire.

akudev commented 2 years ago

Basically this is waiting on backlog items around the "controller extensions" topic in the core team. It is unclear when they will be addressed.

heimwege commented 2 years ago

Hi @nlunets was the reference to pull request 653 a mistake because it was deleted from the original comment or does 653 help with this issue as well?

nlunets commented 2 years ago

It doesn't help as far as i know more than it confirm there is a problem :)

akudev commented 1 year ago

Option 1 (renaming with "overrides" for the metadata and "override" for the static API method) is the way to go. The necessary change in the transformer is on its way and the UI5 core team will work on allowing the new name "overrides" for the metadata.

akudev commented 1 year ago

As already indicated by the changes above, this issue is now solved.

In short, you can use the term overrides instead of override for the object containing methods to override:

  export default class AppExtension extends ControllerExtension {
    static readonly overrides = {
      onInit: function() {
        // ...
      }
    }
  }

This works with

The first solution is the recommended one going forward, the second renames overrides to override during transformation and is only a compatibility workaround for older UI5 versions.

ravish-garg commented 1 year ago

Is there a published example on how to do controller extensions for Fiori Elements using ts, and utilizing the extensionAPI?

akudev commented 1 year ago

@ravish-garg Unfortunately not. It's on the backlog, but the responsible team has been busy with UI5 2.0 tasks for a long time.

heimwege commented 1 year ago

UPDATE: code snippet slightly adjusted

@ravish-garg I can give a brief description of the setup I'm quite happy with. Actually it's not that different to a JS based Fiori Elements controller extension.

Assumptions:

Additionally:

{
  "sourceMaps": true,
  "ignore": [
    "**/*.d.ts"
  ],
  "presets": [
    ["@babel/preset-env", {
      "targets": "defaults"
    }],
    ["transform-ui5", {
      "overridesToOverride": true
    }],
    "@babel/preset-typescript"
  ]
}

Notes:

With this setup your Fiori Elements controller extension looks as follows:

import ControllerExtension from "sap/ui/core/mvc/ControllerExtension";
import type PageController from "sap/fe/core/PageController";

/**
 * @namespace the.id.of.my.app.and.the.path.to.the.controller.extension.folder
 */
export default class MySpecificExtension extends ControllerExtension {
     /**
     * Workaround for missing the correct controller extension type
     * @protected
     */
    protected base!: PageController;

    static overrides = {
        onInit: function (this: MySpecificExtension) {
            this.foo();
        }
    };

    /**
     * Some method
     */
    foo () {
        // imagine some meaningful code here and then calling securedExecution of the extensionAPI
        const functionToBeExecuted = () => {
            // something to be done
        }
        return this.base.getExtensionAPI().getEditFlow().securedExecution(functionToBeExecuted)
    }
}

Notes:

Additionally I can recommend using odata2ts to generate the CAP generated metadata file to a d.ts files so that you can use the types out of the box.

ravish-garg commented 1 year ago

@ravish-garg I can give a brief description of the setup I'm quite happy with. Actually it's not that different to a JS based Fiori Elements controller extension.

Assumptions:

* You are familiar with the basic setup of a UI5 TS app (e.g. from the [SAP-samples](https://github.com/SAP-samples)).

* You use [ui5-middleware-fe-mockserver](https://www.npmjs.com/package/@sap-ux/ui5-middleware-fe-mockserver) for local and automated testing.

Additionally:

* Use [ui5-tooling-transpile](https://www.npmjs.com/package/ui5-tooling-transpile) to transpile the ts source to js on-the-fly when requested (e.g. from [ui5-middleware-fe-mockserver](https://www.npmjs.com/package/@sap-ux/ui5-middleware-fe-mockserver)) or when building the archive.

* Use a `.babelrc.json` that looks as follows:
{
  "sourceMaps": true,
  "ignore": [
    "**/*.d.ts"
  ],
  "presets": [
    ["@babel/preset-env", {
      "targets": "defaults"
    }],
    ["transform-ui5", {
      "overridesToOverride": true
    }],
    "@babel/preset-typescript"
  ]
}

Notes:

* The `overridesToOverride` is only needed in case of UI5 versions < 1.112. So the `.babelrc.json` might not be needed on higher UI5 versions (I currently use 1.108.16).

With this setup your Fiori Elements controller extension looks as follows:

import ControllerExtension from "sap/ui/core/mvc/ControllerExtension";
import type ExtensionAPI from "sap/fe/templates/ObjectPage/ExtensionAPI";
import type EditFlow from "sap/fe/core/controllerextensions/EditFlow";

interface FioriElementsControllerExtension extends ControllerExtension {
    base: {
        getExtensionAPI: () => ExtensionAPI
    }
}

/**
 * @namespace the.id.of.my.app.and.the.path.to.the.controller.extension.folder
 */
export default class MySpecificExtension extends ControllerExtension {
    static overrides = {
        onInit: function (this: MySpecificExtension) {
            this.foo();
        }
    };

    /**
     * Get the fiori elements extension API
     * @return {ExtensionAPI} the extension API
     */
    getExtensionAPI () {
        return (this as unknown as FioriElementsControllerExtension).base.getExtensionAPI();
    }    

    /**
     * Some method
     */
    foo () {
        // imagine some meaningful code here and then calling securedExecution of the extensionAPI
        return (this.getExtensionAPI().getEditFlow() as EditFlow).securedExecution( //function to be executed)
    }
}

Notes:

* The types available in UI5 1.108.16 currently don't know the `base` property, hence the workaround with the `FioriElementsControllerExtension` interface

* The return type of `getEditFlow()` is also not `sap/fe/core/controllerextensions/EditFlow`, hence the explicit type cast

Additionally I can recommend using odata2ts to generate the CAP generated metadata file to a d.ts files so that you can use the types out of the box.

Hi @heimwege Thanks for the sample code. Adding a bit more context: UI5 version : 1.71.xx (On prem customer, so no quick option to update to latest versions ) The "overrides" in the .ts controller works for the ControllerExtension hooks.

I wanted to utilize the other hooks as mentioned here How do I invoke these in a .ts controller?

Happy to take this elsewhere, if this is not the right place.

heimwege commented 1 year ago

@ravish-garg this feature seems to be for OData v2 only, so I'm afraid I can't really help here as I'm using v4. Basically I would expect the basic setup to work as described above (as you already confirmed). The only difference is how you need to write the actual controller extension file so that the transpiled result satisfies the description in the documentation. Maybe @akudev can give a hint here.

akudev commented 1 year ago

@ravish-garg Well, I'm not from Fiori Elements and not an expert on extensions, but as I understand https://ui5.sap.com/#/topic/382a6c39fd494c12a4ee23c8659909bd, there are two things happening: 1.) in manifest.json there is STTA_MP.ext.controller.DetailsExtension registered as extension - this is unaffected by TypeScript - and 2.) this extension is implemented.

If the second code block is complete (and the sap.ui.define in the beginning suggests it is), then what is implemented there is not even a real controller, but just an object with one method, but that's probably fine as I guess simply methods with certain names are searched, regardless of the object type holding them. But this means that in TypeScript you just need to define an object holding this method (or anything else with this method, like a class or so) and it should be found. As long as the UI5 loading returns this thing when the module is required.

When for example you write this as file content in TypeScript:

export default {
    onBeforeRebindTableExtension: function(oEvent: any) {
      ...
    }
}

Then it looks like this after transpilation:

"use strict";

sap.ui.define([], function () {
  var __exports = {
    onBeforeRebindTableExtension: function (oEvent) {
      ...
    }
  };
  return __exports;
});

And this is essentially the same thing that can be seen in the documentation, so I would expect it works fine.

ravish-garg commented 1 year ago

@akudev Thank you Andreas for this. This has worked out like a charm and I can commence my TS extension journey 💯

akudev commented 1 year ago

Maybe this also helps, and for future reference, the Controller Extension topic is also covered in this Fiori Elements tutorial, more specifically in its exercise 8.

Furthermore it was shown in this UI5con 2023 session, starting from 1:15:25. The end result of this session is contained in the repository https://github.com/MariusFreitag/ui5con2023.

The topic "Controller Extension in TypeScript" will also be covered in the SDK soon, albeit not in more detail than explained above, at least at first.

dfenerski commented 12 months ago

Ok I am late to the party but for me it worked without using overrides ? I could configure it with the following .babelrc.json:

    "presets": [
        [
            "transform-ui5",
            {
                "onlyMoveClassPropsUsingThis": true
            }
        ],
        ["@babel/preset-typescript", {}]
    ]
akudev commented 11 months ago

@dfenerski Ok, that's a nice finding! Certainly a good workaround and maybe even useful for similar issues in the future.

On the other hand, it is not meant to be the thing to do in the future, but rather a compatibility switch for legacy apps. As such (and because the default had been changed) it's probably not the thing to widely recommend from now. Also it does unfortunately not help with the currently still open controller extension issue in https://github.com/SAP/ui5-typescript/issues/420 because those properties have to use this.

Thanks nevertheless, it surely will be useful for some who read it.

akudev commented 11 months ago

because those properties have to use this

Erm, that's actually wrong: they don't. But I don't think you can write what you need for #420 in a way that makes TS understand it is an instance, but ends up as the needed class in JavaScript.

(Edit: previously "I don't think" was missing, inverting the meaning)