SAP / ui5-typescript

Tooling to enable TypeScript support in SAPUI5/OpenUI5 projects
https://sap.github.io/ui5-typescript
Apache License 2.0
202 stars 28 forks source link

Library includes are missing types #386

Closed BenReim closed 1 year ago

BenReim commented 1 year ago

Describe the bug For the given snippet:

sap.ui.define(["sap/ui/core/library" ],
    /**
     * @param {typeof import('sap/ui/core/library')} coreLibrary
     */
    function (coreLibrary) {        
        const CSSSize = coreLibrary.CSSSize;
    });

the following error is raised:

Property 'CSSSize' does not exist on type 'typeof import("sap/ui/core/library")
akudev commented 1 year ago

TL;DR: the type is not missing, but the const counterpart. What do you need it for? I assume to call CSSSize.isValid()?

For the time being, you should be able to work around the problem like this, with ts-ignore:

    /**
     * @type import("sap/ui/core/library").CSSSize
     */
    // @ts-ignore
    var csss = coreLib.CSSSize

Long and evolving version: The core library (and likely the others as well) does not have a default export carrying all the enums etc. as properties, but rather a number of named exports.
So in TS and ES6 JavaScript it works fine when you write:

import { CSSSize } from "sap/ui/core/library";

However, it's noticeable that even in TypeScript CSSSize is not offered as autocomplete term and when the full name is written, the import cannot be created as a "quick fix" in VSCode.

For BarColor on the other hand, autocomplete and an automatic import is available. The difference in the declaration is that the former are types, the latter enums:

export type CSSSize = string;
export enum BarColor {...}

When it comes to usage in JavaScript, the difference is bigger. As CSSSize is a type, you cannot write this anymore to import the named export:

import { CSSSize } from "sap/ui/core/library";

It works for the enums, but for types you get: 'CSSSize' is a type and cannot be imported in JavaScript files. Use 'import("sap/ui/core/library").CSSSize' in a JSDoc type annotation.

Types are simply not a thing in JavaScript code.

Why would you need a type in JS code, then? Because it is not only a type, but also a value, an object which has the isValid() method. This aspect is simply missing in our type definitions because we only export its type nature.

codeworrior commented 1 year ago

We IMO should NOT expose the sap/ui/base/DataType nature of sap.ui.core.CSSSize in TypeScript. This is one of the places where the long existing API design of the UI5 runtime does not translate well into the TypeScript world.

Unfortunately, we exposed two different natures of sap.ui.core.CSSSize under the same name: its type nature and the fact that is an instance of sap/ui/base/DataType. Without first class support for UI5 data types in typescript (which is a bit unlikely, I guess), we cannot represent this duality in a proper manner in TypeScript. We could introduce new names for the 2nd nature, but this wouldn't match the runtime API.

I'm therefore strongly in favour of abandoning the 2nd nature also in the runtime. The DataType nature can already today be retrieved via sap/ui/base/DataType.getType("sap.ui.core.CSSSize"). As the API of all instances of DataType is the same, there's no need to represent the types via different names. It would just be a more convenient way to access them.

BenReim commented 1 year ago

@akudev @codeworrior Understood, thanks for the in-depth explanation!

akudev commented 1 year ago

Alright, then let's close this. Nevertheless thanks for reporting, at least we have the topic now described here for others.