Properties not assignable in classes LightBox, UIArea and Table

[one-github]

Using the @openui5/ts-types-esm@1.93.0 with Typescript version 4.5.0-dev.20210812 there are errors with the type definitions themselves when running tsc:

MARVIN:my_project me$ tsc
node_modules/@openui5/ts-types-esm/types/sap.m.d.ts:34236:5 - error TS2416: Property 'invalidate' in type 'LightBox' is not assignable to the same property in base type 'Control'.
  Type '(oOrigin: object) => this' is not assignable to type '(oOrigin?: ManagedObject | undefined) => void'.
    Types of parameters 'oOrigin' and 'oOrigin' are incompatible.
      Type 'ManagedObject | undefined' is not assignable to type 'object'.
        Type 'undefined' is not assignable to type 'object'.

34236     invalidate(
          ~~~~~~~~~~

node_modules/@openui5/ts-types-esm/types/sap.ui.core.d.ts:33085:5 - error TS2416: Property 'getBindingContext' in type 'UIArea' is not assignable to the same property in base type 'ManagedObject'.
  Type '() => null' is not assignable to type '(sModelName?: string | undefined) => Context'.
    Type 'null' is not assignable to type 'Context'.

33085     getBindingContext(): null;
          ~~~~~~~~~~~~~~~~~

node_modules/@openui5/ts-types-esm/types/sap.ui.core.d.ts:33107:5 - error TS2416: Property 'getId' in type 'UIArea' is not assignable to the same property in base type 'ManagedObject'.
  Type '() => string | null' is not assignable to type '() => string'.
    Type 'string | null' is not assignable to type 'string'.
      Type 'null' is not assignable to type 'string'.

33107     getId(): string | null;
          ~~~~~

node_modules/@openui5/ts-types-esm/types/sap.ui.table.d.ts:5582:5 - error TS2416: Property 'getDragDropConfig' in type 'Table' is not assignable to the same property in base type 'Control'.
  Type '() => undefined' is not assignable to type '() => DragDropBase[]'.
    Type 'undefined' is not assignable to type 'DragDropBase[]'.

5582     getDragDropConfig(): undefined;
         ~~~~~~~~~~~~~~~~~

Found 4 errors.

Installed packages:

@openui5/ts-types-esm@1.93.0
@types/jquery@3.5.6
@types/qunit@2.11.2

tsconfig.json:

{
    "compilerOptions": {
    ...
        "types": ["@openui5/ts-types-esm", "@types/qunit", "jquery"],
    ...
        "lib": ["dom","es7"]
    }
...
}

Typescript version:

MARVIN:my_project me$ tsc --version
Version 4.5.0-dev.20210812

The incompatible declarations can be patched so that they compile correctly:

diff --git a/node_modules/@openui5/ts-types-esm/types/sap.m.d.ts b/node_modules/@openui5/ts-types-esm/types/sap.m.d.ts
index 2d18654..da6cba2 100644
--- a/node_modules/@openui5/ts-types-esm/types/sap.m.d.ts
+++ b/node_modules/@openui5/ts-types-esm/types/sap.m.d.ts
@@ -34066,6 +34066,8 @@ declare module "sap/m/LightBox" {

   import ElementMetadata from "sap/ui/core/ElementMetadata";

+  import ManagedObject from "sap/ui/base/ManagedObject";
+
   /**
    * Represents a popup containing an image and a footer.
    *
@@ -34237,7 +34239,7 @@ declare module "sap/m/LightBox" {
       /**
        * Origin of the invalidation.
        */
-      oOrigin: object
+      oOrigin: ManagedObject
     ): this;
     /**
      * Returns if the LightBox is open.
diff --git a/node_modules/@openui5/ts-types-esm/types/sap.ui.core.d.ts b/node_modules/@openui5/ts-types-esm/types/sap.ui.core.d.ts
index 18d35fa..0e2dc1d 100644
--- a/node_modules/@openui5/ts-types-esm/types/sap.ui.core.d.ts
+++ b/node_modules/@openui5/ts-types-esm/types/sap.ui.core.d.ts
@@ -32980,6 +32980,8 @@ declare module "sap/ui/core/UIArea" {

   import Interface from "sap/ui/base/Interface";

+  import Context from "sap/ui/model/Context";
+
   /**
    * An area in a page that hosts a tree of UI elements.
    *
@@ -33082,7 +33084,7 @@ declare module "sap/ui/core/UIArea" {
     /**
      * Provide getBindingContext, as UIArea can be parent of an element.
      */
-    getBindingContext(): null;
+    getBindingContext(sModelName?: string): Context;
     /**
      * Gets content of aggregation {@link #getContent content}.
      *
@@ -33104,7 +33106,7 @@ declare module "sap/ui/core/UIArea" {
     /**
      * Returns this `UIArea`'s id (as determined from provided RootNode).
      */
-    getId(): string | null;
+    getId(): string;
     /**
      * @deprecated (since 1.1) - use function {@link #getContent} instead
      *
diff --git a/node_modules/@openui5/ts-types-esm/types/sap.ui.table.d.ts b/node_modules/@openui5/ts-types-esm/types/sap.ui.table.d.ts
index 198f825..687e4b3 100644
--- a/node_modules/@openui5/ts-types-esm/types/sap.ui.table.d.ts
+++ b/node_modules/@openui5/ts-types-esm/types/sap.ui.table.d.ts
@@ -3803,6 +3803,8 @@ declare module "sap/ui/table/Table" {

   import TooltipBase from "sap/ui/core/TooltipBase";

+  import DragDropBase from "sap/ui/core/dnd/DragDropBase";
+
   /**
    *  Provides a comprehensive set of features for displaying and dealing with vast amounts of data. The
    * Table control supports desktop PCs and tablet devices. On tablets, special consideration should be given
@@ -5579,7 +5581,7 @@ declare module "sap/ui/table/Table" {
      *      - Group header rows
      *      - Sum rows
      */
-    getDragDropConfig(): undefined;
+    getDragDropConfig(): DragDropBase[];
     /**
      * Gets current value of property {@link #getEditable editable}.
      *

[akudev]

@one-github ok, got it: these four errors occur when in the tsc compiler options "strict" mode is enabled, but "strictPropertyInitialization" is not disabled.

In our sample tsconfig files, this is the case:

        ...
        "strict": true,
        "strictPropertyInitialization": false,
        ...

Let me check whether we can avoid requiring this setting (there are seven more such errors in @sapui5/ts-types-esm). Thanks for your patch proposal! Not all changes in UI5 can be done (because we cannot change incompatibly), but at first glance they look harmless.

[akudev]

Oh, and obviously you can avoid the error message for the time being by adding "strictPropertyInitialization": false, to your tsconfig.

[akudev]

On second glance, the strictPropertyInitialization thing doesn't make any sense. Sorry, it's the "strictNullChecks": false, setting which is needed to avoid the four errors.

[one-github]

Sorry, it's the "strictNullChecks": false, setting which is needed to avoid the four errors.

IMHO having activated this option is important - I want TypeScript to check my code for as many statically detectable sources of error as possible.

[akudev]

That's understandable and should be our goal. Fixes for the issues in Table and LightBox are on the way (as the type definitions are generated, those fixes happen in the OpenUI5 JSDoc), but the two UIArea issues are trickier. The original code shows why: the UIArea can indeed return null here (or does it even always), unlike regular controls.

    /**
     * Provide getBindingContext, as UIArea can be parent of an element.
     *
     * @returns {null} Always returns null.
     *
     * @protected
     */
    UIArea.prototype.getBindingContext = function(){
        return null;
    };

    /**
     * Returns this <code>UIArea</code>'s id (as determined from provided RootNode).
     * @return {string|null} id of this UIArea
     * @public
     */
    UIArea.prototype.getId = function() {
        return this.oRootNode ? this.oRootNode.id : null;
    };

We'll have a deeper look.

[akudev]

Another change fixed one of the UIArea issues: https://github.com/SAP/openui5/commit/ff2b05340eb90375c58bd5875e803e6cb19d8eae

In a further change, the d.ts generation will probably be configured to add a @ts-ignore for UIArea.getId().

Afterwards, the type definitions will be compatible with strictNullChecks:true.

Further changes will do the same for the full SAPUI5 type definitions and will make sure there are no regressions. Thanks for prompting us to improve this!

[akudev]

Well... I just noticed that DefinitelyTyped prevents the use of ts-ignore in d.ts files.

There is a hardcoded check in their dtslint check which our type definitions have to pass: https://github.com/microsoft/dtslint/blob/274f0d0cf64fb03765fb5f70009fe5f00b8ecf9c/src/lint.ts#L130

So I have to revert the ts-ignore for UIArea.getId() (and for the other remaining occurrence in the sap.ui.comp library in SAPUI5). I'm not sure right now how to proceed - we'll discuss it. Until we find a way out, the two issues will remain.

[bd82]

You could consider using // @ts-expect-error instead. But if you mark a code snippet with this comment and no error is detected that in itself would cause a TSC compliation error 😢

https://devblogs.microsoft.com/typescript/announcing-typescript-3-9-beta/#ts-expect-error-comments

[akudev]

As we cannot change the implementation and as we want the JSDoc documentation to be as correctly describing the behavior as possible, we decided to add a generation hint which slightly changes what is generated as type definition: UIArea.getId() is said to return type "string". This will be in effect with the 1.97 release.

[akudev]

There's unfortunately one more issue within OpenUI5 left, which prevents strictNullChecks. It came in before we added a check to our tests. But it was fixed here for 1.98: https://github.com/SAP/openui5/commit/5a4d90ec5b314f210c035e85212b98ceefb05d3a

[akudev]

@one-github With the just published version 1.98.0, full strict mode should be working fine.

See https://github.com/DefinitelyTyped/DefinitelyTyped/pull/58429