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

Wrong source type for generated type ODataModel$DataReceivedEvent #426

Open heimwege opened 7 months ago

heimwege commented 7 months ago

Describe the bug

This is about the dataReceived event of the v4.OdataModel.

Currently it is generated in sap.ui.core.d.ts as follows

export type ODataModel$DataReceivedEvent = Event<
    ODataModel$DataReceivedEventParameters,
    ODataModel
  >;

Having a Fiori Elements list report extension causes the following type error

static overrides = {
        routing: {
            onAfterBinding (this: ListReportExtension) {
        (this.getView().getModel() as ODataModel).attachDataReceived((event: ODataModel$DataReceivedEvent) => {
                    if ((event.getSource() as Binding).getPath?.() !== "/Foo") { // <-- type error TS2352
                        return;
                    }
                    // some important foo
        }
        }
    }
}

type error is TS2352: Conversion of type ODataModel to type Binding may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to unknown first.

The debug console shows that the return type is actually a Binding and not an ODataModel

event.getSource() instanceof sap.ui.model.Binding => true event.getSource() instanceof sap.ui.model.odata.v4.ODataModel => false

Seems like in case of this event (not sure if there are others as well) the getSource() method does not return and instance of the odata model as assumed by the generator but the model binding.

The event documentation looked ok for me that's why I open this issue here because it might be an issue with the generator.

Expected behavior

the following manual adjustment of the generated type fixed the issue

export type ODataModel$DataReceivedEvent = Event<
    ODataModel$DataReceivedEventParameters,
    Binding
  >;
codeworrior commented 7 months ago

The problem rather is that the TypeScript signatures (and the JSDoc comments that they're based on), do not (and IMO cannot) reflect event bubbling. It cannot in general be foreseen what child fires an event that then bubbles up and can be listened to on an ancestor.

The only solution that comes to my mind is to remove the typing of the getSource again. This would be similar to browser events that also can't say of what type the original target of an event could be. It depends on the DOM hierarchy.

Unfortunately, the ability for an event to bubble up is decided on the original event source. Therefore, the ancestor cannot even know (with the information that we currently have) if there's a descendant that fires the same event and allows it to bubble up.

In the case of the ODataModel, the different entities luckily know each other. Maybe we find a less impacting solution for that scenario. But in the general case, I don't see how we could type getSource better than with EventProvider, which would be cumbersome in most cases.