SAP / ui5-typescript

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

Wrong import path in *.gen.d.ts-files #381

Closed dstigl closed 1 year ago

dstigl commented 1 year ago

Hi, there is a problem with the imports of the generated *.gen.d.ts files when using different folder structures for derived classes. For example we have the following folder structure in our custom library: src/customLibrary/base/BaseClass.ts src/customLibrary/DerivedClass.ts src/customLibrary/AnotherDerivedClass.ts

with the following codes: base/BaseClass.ts

import ManagedObject from 'sap/ui/base/ManagedObject';

/**
 * @namespace my.customLibrary.base
 */
export default class BaseClass extends ManagedObject {
  constructor(idOrSettings?: string | $BaseClassSettings);
  constructor(id?: string, settings?: $BaseClassSettings);
  constructor(id?: string, settings?: $BaseClassSettings) {
    super(id, settings);
  }
  static readonly metadata = {
    properties: {
      test: { type: 'string', defaultValue: null },
    },
  };
}

base/BaseClass.gen.d.ts

import { PropertyBindingInfo } from "sap/ui/base/ManagedObject";
import { $ManagedObjectSettings } from "sap/ui/base/ManagedObject";

declare module "./BaseClass" {

    /**
     * Interface defining the settings object used in constructor calls
     */
    interface $BaseClassSettings extends $ManagedObjectSettings {
        test?: string | PropertyBindingInfo;
    }

    export default interface BaseClass {

        // property: test
        getTest(): string;
        setTest(test: string): this;
    }
}

DerivedClass.ts

import BaseClass from './base/BaseClass';
/**
 * @namespace my.customLibrary
 */
export default class DerivedClass extends BaseClass {
  constructor(idOrSettings?: string | $DerivedClass Settings);
  constructor(id?: string, settings?: $DerivedClass Settings);
  constructor(id?: string, settings?: $DerivedClass Settings) {
    super(id, settings);
  }
  static readonly metadata = {
    properties: {
      test: { type: 'string', defaultValue: null },
      testb: { type: 'string', defaultValue: null },
    },
  };
}

DerivedClass.gen.d.ts

import { PropertyBindingInfo } from "sap/ui/base/ManagedObject";
import { $BaseClassSettings } from "./BaseClass";

declare module "./DerivedClass" {

    /**
     * Interface defining the settings object used in constructor calls
     */
    interface $DerivedClassSettings extends $BaseClassSettings {
        test?: string | PropertyBindingInfo;
        testb?: string | PropertyBindingInfo;
    }

    export default interface DerivedClass {

        // property: test
        getTest(): string;
        setTest(test: string): this;

        // property: testb
        getTestb(): string;
        setTestb(testb: string): this;
    }
}

AnotherDerivedClass.ts

import DerivedClass from './DerivedClass';

export default class AnotherDerivedClass extends DerivedClass {
  static readonly metadata = {
    properties: {
      test: { type: 'string', defaultValue: null },
      testb: { type: 'string', defaultValue: null },
      testd: { type: 'string', defaultValue: null },
    },
  };
}

The import path of $BaseClassSettings in DerivedClass.gen.d.ts is wrong, which should be ./base/BaseClass. In this case its not possible to extend the DerivedClass (AnotherDerivedClass), because the generator throws the following error:

C:\OpenUI5\LibrariesTS\customLibrary\src\libraries\customLibrary\node_modules\@ui5\ts-interface-generator\dist\interfaceGenerationHelper.js:126
                                throw new Error(`${statement.name ? statement.name.text : ""} inherits from ${interestingBaseClass} and has metadata but the parent class ${typeChecker.getFullyQualifiedName(type.getSymbol())} seems to have no settings type. It might have no constructors, this is where the settings type is used.  
                                ^

Error: AnotherDerivedClass inherits from ManagedObject and has metadata but the parent class "C:/OpenUI5/LibrariesTS/customLibrary/src/libraries/customLibrary/DerivedClass".DerivedClass seems to have no settings type. It might have no constructors, this is where the settings type is used.

In case this parent class is also in your project, make sure to add its constructors, then try again. A comment with instructions might be in the console output above.
Otherwise, you can temporarily remove this file (C:/OpenUI5/LibrariesTS/customLibrary/src/libraries/customLibrary/AnotherDerivedClass.ts) from the project and try again to get the console output with the suggested constructors.
In any case, you need to make the parent parent class "C:/OpenUI5/LibrariesTS/customLibrary/src/libraries/customLibrary/DerivedClass".DerivedClass have constructors with typed settings object to overcome this issue.

    at C:\OpenUI5\LibrariesTS\customLibrary\node_modules\@ui5\ts-interface-generator\dist\interfaceGenerationHelper.js:126:39
    at Array.forEach (<anonymous>)
    at C:\OpenUI5\LibrariesTS\customLibrary\node_modules\@ui5\ts-interface-generator\dist\interfaceGenerationHelper.js:68:46
    at Array.forEach (<anonymous>)
    at C:\OpenUI5\LibrariesTS\customLibrary\node_modules\@ui5\ts-interface-generator\dist\interfaceGenerationHelper.js:66:43
    at Array.forEach (<anonymous>)
    at getManagedObjects (C:\OpenUI5\LibrariesTS\customLibrary\node_modules\@ui5\ts-interface-generator\dist\interfaceGenerationHelper.js:62:27)
    at generateInterfaces (C:\OpenUI5\LibrariesTS\customLibrary\node_modules\@ui5\ts-interface-generator\dist\interfaceGenerationHelper.js:38:17)        
    at C:\OpenUI5\LibrariesTS\customLibrary\node_modules\@ui5\ts-interface-generator\dist\generateTSInterfaces.js:133:60
    at Array.forEach (<anonymous>)

It's also not possible to inherit from these classes in our main project where we use "customLibrary"-library. There whe get the following error:

TypeError: Cannot read properties of undefined (reading 'parent')
    at Object.getFullyQualifiedName (C:\OpenUI5\MainProject\node_modules\typescript\lib\typescript.js:51691:27)
    at getInterestingBaseSettingsClass (C:\OpenUI5\MainProject\node_modules\@ui5\ts-interface-generator\dist\interfaceGenerationHelper.js:273:83)
    at getSettingsTypeFromConstructor (C:\OpenUI5\MainProject\node_modules\@ui5\ts-interface-generator\dist\interfaceGenerationHelper.js:238:50)    at getSettingsType (C:\OpenUI5\MainProject\node_modules\@ui5\ts-interface-generator\dist\interfaceGenerationHelper.js:221:38)
    at C:\OpenUI5\MainProject\node_modules\@ui5\ts-interface-generator\dist\interfaceGenerationHelper.js:118:54
    at Array.forEach (<anonymous>)
    at C:\OpenUI5\MainProject\node_modules\@ui5\ts-interface-generator\dist\interfaceGenerationHelper.js:68:46
    at Array.forEach (<anonymous>)
    at C:\OpenUI5\MainProject\node_modules\@ui5\ts-interface-generator\dist\interfaceGenerationHelper.js:66:43
    at Array.forEach (<anonymous>)
petermuessig commented 1 year ago

@akudev, are you aware of this issue here?

akudev commented 1 year ago

@dstigl, thanks for reporting! This was a bit tricky to analyze and solve. The typeChecker.getFullyQualifiedName(symbol); method in the TypeScript compiler API (used here to find how to import the settings type of the base class) does return relative paths for local types ("./xyz"), but not really relative in the sense that they contain instructions to traverse to that path from the current class' path (which is not known at all to this call). For now, I found no other way than to compare the containing file paths of the declarations of both classes and tweak the path accordingly. This fixes the issue. I will do a little more testing, but am fairly confident that this won't break anything.

Another issue you might stumble across when trying the fixed version is that the generator handles the classes in random order (well, probably alphabetical), but does not create a dependency tree of your classes first. This means that during the first run "AnotherDerivedClass" might be handled first and complain that there is no settings type for its superclass "DerivedClass" - because the generator has to create that settings type and will only do so afterwards. The workaround is just to run the generator again, until all such issues have disappeared. This is not an issue when developing from scratch, as the base classes are created first.

@petermuessig Yes, I am.

akudev commented 1 year ago

Version 0.5.3 including the fix has just been released.

akudev commented 1 year ago

By the way, you should not declare the same properties (like "test") also in sub-classes. I guess you did it to get rid of the "Class static side ... incorrectly extends..." error, but there is a better workaround described here.

Also see this issue.

dstigl commented 1 year ago

Thank you