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

Library includes are missing types #386

Closed
BenReim opened this issue Feb 16, 2023 · 4 comments
Closed

Library includes are missing types #386

BenReim opened this issue Feb 16, 2023 · 4 comments
Labels
types ES modules types (recommended) wontfix This will not be worked on

Comments

@BenReim
Copy link

BenReim commented Feb 16, 2023

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
Copy link
Contributor

akudev commented Feb 16, 2023

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.

@akudev akudev added bug Something isn't working types ES modules types (recommended) labels Feb 16, 2023
@codeworrior
Copy link
Member

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.

@codeworrior codeworrior added wontfix This will not be worked on and removed bug Something isn't working labels Feb 16, 2023
@BenReim
Copy link
Author

BenReim commented Feb 17, 2023

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

@akudev
Copy link
Contributor

akudev commented Feb 17, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
types ES modules types (recommended) wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants