search

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

Types within @sapui5/ts-types are invalid #215

Open stnmtz opened 3 years ago

stnmtz commented 3 years ago

Hi,

I wanted to try out TypeScript and UI5 but so far was not able to compile my projects due to 76 type errors of the @sapui5/ts-types module. Thought the problem is on my end, but so far it seems that types inside the @sapui5/ts-types are wrong? I tested with @sapui5/ts-types versions 1.84.4 (76 errors), 1.85.2 (80 errors) and 1.78.17 (2 errors). I didn't test more versions, but it seems the tests are never quite run before publishing the modules?

Even if I run the test script of node_modules/@sapui5/ts-types module, I get the same errors: Can somebody tell me what's up? Do I need more dependencies or are the type definition files really that buggy? Is there a types version which work?

Cheers

> @sapui5/ts-types@1.84.4 test \node_modules\@sapui5\ts-types
> tsc

types/sap.ca.ui.d.ts:5419:23 - error TS2304: Cannot find name 'FileSize'.

5419               oValue: FileSize
                           ~~~~~~~~

types/sap.f.d.ts:11720:7 - error TS2416: Property 'getGridLayoutConfiguration' in type 'GridList' is not assignable to the same property in base type 'IGridConfigurable'.
  Type 'undefined' is not assignable to type '() => GridLayoutBase'.

11720       getGridLayoutConfiguration: undefined;
            ~~~~~~~~~~~~~~~~~~~~~~~~~~

types/sap.gantt.d.ts:4849:21 - error TS2430: Interface '$RelationshipSettings' incorrectly extends interface '$PathSettings'.
  Types of property 'selectedShape' are incompatible.
    Type 'SelectedRelationship | undefined' is not assignable to type 'SelectedShape | undefined'.
      Type 'SelectedRelationship' is missing the following properties from type 'SelectedShape': getHeight, setHeight

4849           interface $RelationshipSettings
                         ~~~~~~~~~~~~~~~~~~~~~

types/sap.gantt.d.ts:17630:34 - error TS2724: 'sap.gantt.simple' has no exported member named '$BaseDiamondSettings'. Did you mean '$AdhocDiamondSettings'?

17630         extends sap.gantt.simple.$BaseDiamondSettings {
                                       ~~~~~~~~~~~~~~~~~~~~

types/sap.m.d.ts:25507:11 - error TS2559: Type 'Button' has no properties in common with type 'IFormContent'.

25507     class Button extends sap.ui.core.Control
                ~~~~~~

types/sap.m.d.ts:26829:11 - error TS2559: Type 'CheckBox' has no properties in common with type 'IFormContent'.

26829     class CheckBox extends sap.ui.core.Control
                ~~~~~~~~

types/sap.m.d.ts:40654:11 - error TS2559: Type 'Image' has no properties in common with type 'IFormContent'.

40654     class Image extends sap.ui.core.Control

........

types/sap.viz.d.ts:36:36 - error TS2694: Namespace 'sap.viz.ui5.api.env' has no exported member 'Format'.

36             ): sap.viz.ui5.api.env.Format;
                                      ~~~~~~

Found 76 errors.

npm ERR! code ELIFECYCLE
npm ERR! errno 2
npm ERR! @sapui5/ts-types@1.84.4 test: `tsc`
npm ERR! Exit status 2
npm ERR!
npm ERR! Failed at the @sapui5/ts-types@1.84.4 test script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm WARN Local package.json exists, but node_modules missing, did you mean to install?
bd82 commented 3 years ago

Hello @stnmtz

The signatures are created from UI5 SDK metadata files (created from UI5 source code) in the UI5 build process.

This metadata is manually created by humans and includes errors, unfortunately, not all of these issues have been fixed in the UI5 build/sources.

The workaround is to skipLibChecks in your tsconfig file.

stnmtz commented 3 years ago

@bd82 Thank you for the reply. I so far wasn't aware of skipLibChecks and will give it a try. In the meanwhile, I have successfully fixed the 1.84.4 typings on my end (also by deleting unused type references).

Doesn't non-compileable types result in subsequent errors whenever one uses UI5 typings? For example, the code completion tools might work for some classes on some versions, but not with another class or another version of the typings. As the type for IFormContent@1.84.4 doesn't compile, I guess all the code completions for controls might be broken, and so on...

As far as I understood, it would be better to publish a UI5 types version without compiler errors (but maybe with outdated types) than a non-compileable UI5 types version with up-to-date types.

codeworrior commented 3 years ago

We're still in the process of defining the mapping from UI5 APIs to TypeScript. Not all compile errors that you see are "errors" in our metadata. Most IFormContent errors for example are not due to errors in the original sources. They're rather caused by a different understanding of interfaces in UI5 (uses several tag interfaces) and TypeScript (does no longer allow tag interfaces).

Once we're done with the conceptual work for the mapping, we'll adapt the generation and clean-up issues in the sources, or, where this is not possible due to compatibility constraints in UI5, will exclude the corresponding APIs from the ts-types generation. From that point in time on, new errors will break our builds and won't go "unnoticed" (we already check the results at build time, but don't break the build yet).

I know this isn't perfect but a) do we currently mainly target the language server b) do we work on a better solution and c) a possible workaround exists with skipLibChecks. And, last but not least, are these some of the reasons why this project is still in Beta State .

We'll nevertheless discuss whether we'll go the extra mile to exclude all erroneous APIs for now.
@petermuessig , @akudev what do you think?

stnmtz commented 3 years ago

Thank you both for the feedback. I am aware of the Beta State, nevertheless is publishing non-compiling code a big question mark for me - no matter if the code completion plugins ignore those errors and try their best to make senseful annotations :) But maybe my understanding of Beta is just different. I opened the bug as there is a "test" script within the project which seems to be never called. We can close the issue imo.

Nevertheless, I'd like to emphasize a bit on my use case which might help for the discussion: We have a whole lot of TypeScript libraries (based on modules) which we'd like to embed into UI5 applications. Of course, we'd like to use the CodeCompletion and sanity checks of TypeScript also within the UI5 (JavaScript) projects for those libraries. These JS projects are build with the UI5 tooling.

Additionally, we'd like to use TypeScript to develop (Control-)Libraries augmenting UI5. These TypeScript-only libraries can then be compiled to JS and used in any UI5 application. Of course, for these libraries we have to ensure manually that all the required UI5 controls are loaded upfront in the application before they are used/extended in the library (no UI5 tooling here).

We now have a working project with UI5 types and external module-based TypeScript libraries (compiled to JS), but we had to apply changes to the UI5 type definitions, as well as fine tune the tsconfig properties for UI5, which is usually not neccessary for published types. This configuration even works without the use of skipLibCheck. Especially combining the UI5 types with other module-based TS libraries all in an JS project was quite cumbersome. But don't get me wrong here - I do not believe this is because of the UI5 typings, but because of the general behavior of the TS compiler in a JS context (global namespaces typings, local module typings and bringing it all together).

bd82 commented 3 years ago

Hello @stnmtz I agree that producing type signatures that contain compilation errors is a bit strange. But as mentioned earlier the main initial driver for this topic was to support code editing flows (mainly for JavaScript) by providing these Type Signatures to the TS/JS language server.

Also note that this project only contains the signature generator, this generator is later consumed by UI5 build process to create @ui5/ts-types but its still perfectly fine to hold discussions on the general UI5 + TypeScript here. 😄

Regarding your scenario, I cannot really comment as I am not that deeply familiar with UI5 myself. However regarding namespaces vs modules in the generation, that may actually change in the future.

codeworrior commented 3 years ago

Slightly OT:

Triggered by this issue, I have taken a fresh look at the IFormContent topic (e.g. "Type '...' has no properties in common with type sap.ui.core.IFormContent).

It seems that TypeScript no longer runs into such weak type detection issues when the interface inherits from a suitable base class.

This would be an acceptable change on UI5 side as all sap.ui.core.IFormContent implementations are anyhow supposed to be subclasses of sap.ui.core.Control.

But unfortunately, the current version of the dts-generator ignores the extends information for interfaces (as it looks, due to the lack of an example in the APIs so far).

@bd82 I created https://github.com/SAP/ui5-typescript/pull/217 to allow this, can you please take a look?

bd82 commented 3 years ago

@codeworrior I reviewed #217