[BUG] [REGRESSION] Extra class properties with incorrect types added

[hakimio]

Is there an existing issue for this?

• [X] I have searched the existing issues

Nature of Your Project

TypeScript

Current Behavior

Let's say we have User entity defined the following way:

export class User extends _._cuidAspect(_._managedAspect(_UserAspect(__.Entity))) {}

The class already inherits cuidAspect and managedAspect. cuidAspect defined as:

export function _cuidAspect<TBase extends new (...args: any[]) => object>(Base: TBase) {
  return class cuid extends Base {
        ID?: string | null;
      static readonly actions: Record<never, never>
  };
}

Now, for whatever reason cds-typer decided to duplicate the inherited fields on the base class with different typing (ID?: string vs ID?: string | null;):

export function _UserAspect<TBase extends new (...args: any[]) => object>(Base: TBase) {
  return class User extends Base {
        ID?: string;
        createdAt?: __.CdsTimestamp | null;
    /**
    * Canonical user ID
    */
        createdBy?: _.User | null;
        modifiedAt?: __.CdsTimestamp | null;
    /**
    * Canonical user ID
    */
        modifiedBy?: _.User | null;
          };
}

The duplicated fields appeared in the service class definitions but not in the db schema classes and because of this inconsistency I know get TS errors:

X [ERROR] NG2: Type 'import("models/gc/crm/index").User' is not assignable to type 'import("models/CrmService/index").User'.
  Types of property 'ID' are incompatible.
    Type 'string | null | undefined' is not assignable to type 'string | undefined'.
      Type 'null' is not assignable to type 'string | undefined'.

Expected Behavior

No TS errors after cds-typer update.

Steps To Reproduce

No response

Environment

- **OS**: Windows 10
- **Node**: 20.11.0
- **npm**:
- **cds-typer**: 0.20.1
- **cds**: 7.8.2

Repository Containing a Minimal Reproducible Example

No response

Anything else?

The last tested cds-typer version without the TS errors: 0.16.0

[daogrady]

Hi Tomas,

thanks for opening this issue, could you please attach the relevant part of your cds model for reproduction? Thanks!

Best, Daniel

[hakimio]

schema.cds

using {
    cuid,
    managed
} from '@sap/cds/common';

namespace gc.crm;

@assert.unique: {email: [email]}
entity Users : cuid, managed {
        name         : String not null  @mandatory;
        username     : String not null  @mandatory;
        isActive     : Boolean default true;
        status       : UserStatus default 'Available';
        phone        : String           @assert.format               : '^\+?[0-9 -]+$';
        email        : String not null  @assert.format               : '^[a-z-\.]+@([a-z-]+\.)+[a-z-]{2,4}$'  @mandatory;
        title        : Association to UserTitles;
}

entity UserTitles : cuid, managed {
    name        : String not null @mandatory;
    description : String(500);
    isActive    : Boolean default true;
    users       : Association to many Users
                      on users.title = $self;
}

crm-service.cds

using {gc.crm as crm} from 'schema';

service CrmService @(path: '/api') {
    entity Users              as projection on crm.Users;
    entity UserTitles         as projection on crm.UserTitles;
}

index.cds

using from 'crm-service';

[daogrady]

Thanks for adding the model! Your Users.status is referring to UserStatus, which is not included. Could you please add that as well?

[hakimio]

Sure, but I don't think it changes anything: types.cds

type UserStatus : String enum {
    Available;
    Busy;
    DoNotDisturb;
    Remote;
}

The problem is mismatching ID types in schema TS definitions and service TS definitions.

[daogrady]

Thanks, I can now reproduce the issue. The minimal repro seems to be:

using { cuid } from '@sap/cds/common';

entity A { x: Association to B; }

entity B : cuid {}

and interestingly, when maintaining an Association to an entity that extends cuid (here: to B), the types for cuid change from

export function _cuidAspect<TBase extends new (...args: any[]) => object>(Base: TBase) {
  return class cuid extends Base {
        ID?: string;
      static readonly actions: Record<never, never>
  };
}

to

export function _cuidAspect<TBase extends new (...args: any[]) => object>(Base: TBase) {
  return class cuid extends Base {
        ID?: string | null;
      static readonly actions: Record<never, never>
  };
}

which ultimately causes the mismatch between the two declarations of ID. That should not happen. I will investigate and include this into the test suite.

[daogrady]

The root cause here is actually the order in which the elements are handled. It is somehow strange that this occurs only now, but I have prepared a bandaid fix for this which passes the related tests. @hakimio would you mind trying out the related PR on your actual model to confirm it resolves the problem for you? (@siarhei-murkou fyi, since you originally contributed nullability)

[hakimio]

No, doesn't seem to be fixed at all for me.

• Different types are still generated for the service and schema entities even they are defined identically.
  • cuidAspect still defines ID as nullable, while it's set as non-nullable on the entity itself.
  • service entity definitions still have duplicated fields from the inherited aspects while the ones from schema don't have this issue.

Why have you decided to duplicate the properties anyway? And why does it only happen on service entities?

[daogrady]

I will look into this again next week then.

Why have you decided to duplicate the properties anyway?

Since projections can contain as, as well as excluding expressions, it is much more straightforward to explicitly duplicate the entity we are projecting on with the properties that are explicitly given in the projection. As long as they turn out to be of identical type, this does not cause any issue since TS uses duck typing. The problem only arises once the types for some properties diverge, as seems to be the case here.

[daogrady]

Checking in on this again, could you please provide the code where you are using the types? Why are you attempting to assign a model entity to a variable of the type of the projection entity anyway?

[hakimio]

I am only using service entities but those service entities are referring to model entities as their child entities/associations. For example, the User entity defined in the service namespace is referring to UserTitle defined in the model:

title?: __.Association.to<_gc_crm.UserTitle> | null;

Now, if I import UserTitle from the service namespace, I can no longer assign the user.title property on the User entity which is also imported from the same service namespace.

[daogrady]

I am unclear what you mean by "I can no longer assign the user.title property on the User entity"? What can you not assign to it? I have whipped up a small sample of what sounds like what you are trying to do, but that seems to work on my end:

// srv/MyService.ts
import { ApplicationService } from '@sap/cds'

export class MyService extends ApplicationService { async init() { super.init();
    const { User, UserTitle } = await import('#cds-models/CrmService')  // both imported from the service namespace
    const u = new User()
    u.title = new UserTitle()  // no type errors
}}

I have used the CDS snippets you provided above and cds-typer 0.20.2.

I am afraid I am currently doing a lot of reverse engineering/ amending of your model/ setup/ application. Could you possibly attach a minimal, fully self contained example where I can observe the described behaviour, possibly in a repository? That would be highly appreciated!

[hakimio]

Ok, simplified variant of the issue in the following screenshots:

id1: string | undefined

id2: string | undefined | null

not assignable

I have defined the UserTitles entity above. If you don't think that's an issue worth fixing, feel free to close this.

[hakimio]

Hi @daogrady

Here is what could be called "real-world" reproduction of the issue:

// Service imports
import type {Comment, User} from '@my-app/models/MyService';

const comment: Comment = {};
// TS error: types of property ID are incompatible
const user: User = comment.user!;

EDIT: while this might not be a major issue on the backend where the type of user can be automatically inferred from comment.user and User type annotation is not mandatory, it's a much bigger issue if you use the same types in your frontend. For example, I reuse the same auto-generated types on my Angular frontend where Angular inputs have type annotations and I get type error when using Angular strict templates.

[github-actions[bot]]

This issue has not been updated in a while. If it is still relevant, please comment on it to keep it open. The issue will be closed soon if it remains inactive.

[siarhei-murkou]

Hi @daogrady , I've pulled the changes from the PR locally, tried to compile CDS to TS and got the same issue that keys are compiled as nullable fields.

---

Hi @hakimio ,

FYI, I've already prepared some fixes in past, could you please pull this branch locally and check if it fixes your problem?

---

upd: just reopened my PR #258. If the fix solves Tomas' issue, you @daogrady can review it and if it's fine, you can add your e2e tests into it, re-run the pipelines and merge. 🙂

[daogrady]

Hi @siarhei-murkou,

I can confirm that your fix resolves the issue on the minimal model; highly appreciated!

Let's wait a bit for Tomas to check it out on his actual model for added confidence, so I would merge the fix either after we hear back or at the latest at the end of the week.

Best, Daniel

[hakimio]

@siarhei-murkou thank you for the fix. Seems to work well.