Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ts-interface-generator ] Return type for the method "remove" for aggregation of cardinality 0 .. n does not match runtime behaviour #470

Closed
AnsgarLichter opened this issue Aug 26, 2024 · 3 comments · Fixed by #471
Assignees
Labels
bug Something isn't working ts-interface-generator Related to the ts-interface-generator sub-package

Comments

@AnsgarLichter
Copy link

AnsgarLichter commented Aug 26, 2024

Describe the bug
I was just having a look into OpenUI5 with TypeScript for a private project. I played around with custom controls.
I defined a custom aggreagation with multiple: true:

import Control from "sap/ui/core/Control";
import type { MetadataOptions } from "sap/ui/core/Element";
import RenderManager from "sap/ui/core/RenderManager";

/**
 * @namespace ui5.typescript.helloworld.control
 */
export default class MyControl extends Control {
    // The following three lines were generated and should remain as-is to make TypeScript aware of the constructor signatures
    constructor(idOrSettings?: string | $MyControlSettings);
    constructor(id?: string, settings?: $MyControlSettings);
    constructor(id?: string, settings?: $MyControlSettings) { super(id, settings); }
 
	static readonly metadata: MetadataOptions = {
		properties: {
			"text": "string"
		},
        aggregations: {
            "columns": { type: "sap.ui.core.Control", multiple: true},
        }
	};

	static renderer = {
		apiVersion: 2,
		render: function (rm: RenderManager, control: MyControl): void {
			rm.openStart("div", control);
			rm.openEnd();
			rm.text(control.getText());
			rm.close("div");
		}
	};

	onclick = function() {
		alert("Hello World!");
	}
}

This leads to the following generated interface:

export default interface MyControl {

        // property: text
        getText(): string;
        setText(text: string): this;

        // aggregation: columns
        getColumns(): Control[];
        addColumn(columns: Control): this;
        insertColumn(columns: Control, index: number): this;
        removeColumn(columns: number | string | Control): this;
        removeAllColumns(): Control[];
        indexOfColumn(columns: Control): number;
        destroyColumns(): this;
    }
}

Nevertheless during runtime the method insertColumn and ``removeColumn` return the inserted or removed element:
image

Expected behavior
The interface generator should generate the return type according to runtime behaviour:

 insertColumn(columns: Control, index: number): Control | undefined;
removeColumn(columns: number | string | myColumn): Control | undefined;

I checked with other aggregations with the cardinality 0 .. n. They have all the return type Type of Aggregation |null (see sap.ui.table.Table or sap.m.Page).

Additional context
I used version 0.8.3 of the module @ui5/ts-interface-generator and the following OpenUI5 version:
image

@akudev
Copy link
Contributor

akudev commented Sep 3, 2024

Hi @AnsgarLichter, I think you are right about removeXY - thanks for spotting this!

But for insertXY, "this" is indeed returned, no? You can see this e.g. for the "content" of a Panel: https://jsbin.com/hibahuk/1/edit?html,output
Also in your screenshot, "insertColumn" returns something with ID "_control0", which is the parent control (this), not the inserted one.

@akudev akudev self-assigned this Sep 3, 2024
@akudev akudev added bug Something isn't working ts-interface-generator Related to the ts-interface-generator sub-package labels Sep 3, 2024
@AnsgarLichter
Copy link
Author

AnsgarLichter commented Sep 3, 2024

Hi @akudev,

Yes you are right about insertXY, I oversaw this. Thanks for double-checking.

BR Ansgar

@akudev
Copy link
Contributor

akudev commented Sep 11, 2024

Fixed in 0.8.4 - thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ts-interface-generator Related to the ts-interface-generator sub-package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@akudev @AnsgarLichter and others