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

Allow generic type definitions & non-default exports #357

Open danielang opened 2 years ago

danielang commented 2 years ago

feat(ts-interface-generator): handeling generic type definitions fix(ts-interface-generator): handling for non-default export ManagedObjects

cla-assistant[bot] commented 2 years ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: danielang
:x: danielang-ortec
You have signed the CLA already but the status is still pending? Let us recheck it.

akudev commented 2 years ago

@danielang Thanks a lot for your contribution! Would you mind also providing a piece of documentation explaining what this allows and what typical use-cases would be? I'm not yet sure about the best place for it, maybe in the Readme here or in the control development or control library sample. Better here, I guess. maybe in a new "Features" section.

danielang commented 2 years ago

Hey @akudev, sure 🙂 I added a section in the readme file of this package. I hope it goes in the right direction. I would be happy to receive some feedback. The changes in this PR allows to use the ts-interface-generator also with named exports. It also allows to make use of TypeScripts generic type parameters, optionally with type constrains.

akudev commented 2 years ago

@danielang While the code is fine and the docu just has minor issues, I think there is a problem in how it works: when the generic class is used in its own API (and probably also when the class is used in APIs of other controls), then the generated methods in the interface do not have the generics part.

Example (shortened):

export default class SampleControl<T extends Toolbar> extends Control {
    static readonly metadata = {
        aggregations: {
            partner: { type: "SampleControl" }
        },
    };
}

causes the generation of

declare module "./SampleControl" {

    interface $SampleControlSettings extends $ControlSettings {
        partner?: SampleControl[] | SampleControl | AggregationBindingInfo | `{${string}}`;
    }

    export default interface SampleControl<T extends Toolbar> {
        // aggregation: partner
        getPartner(): SampleControl[];
        addPartner(partner: SampleControl): this;
        insertPartner(partner: SampleControl, index: number): this;
        removePartner(partner: number | string | SampleControl): this;
        removeAllPartner(): SampleControl[];
        indexOfPartner(partner: SampleControl): number;
        destroyPartner(): this;
    }
}

There's the class name "SampleControl" all over the place - without generics. Which makes TS complain: "Generic type 'SampleControl' requires 1 type argument(s).".

One could argue that the metadata actually it should be:

            partner: { type: "SampleControl<T extends Toolbar>" }

but this doesn't work either because

  1. the interface generator treats this string as a plain class name and tries to import the whole thing (fixable within the generator) and
  2. this string will still be there at runtime - and not be understood by UI5 (only fixable inside the babel transformer plugin, but that's the wrong place, as it shouldn't handle TypeScript input)

Right now I don't know how to deal with this. Of course writing types as strings in the metadata section is legacy not helping here, but it is what we have now and changing it would be a major development effort. We could also simply state that using generic classes in the control API definition is not supported.

What do you think? And you, @codeworrior , @petermuessig ?