Closed hakimio closed 2 weeks ago
Hi Tomas,
thanks for opening this issue, could you please attach the relevant part of your cds model for reproduction? Thanks!
Best, Daniel
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';
Thanks for adding the model! Your Users.status
is referring to UserStatus
, which is not included. Could you please add that as well?
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.
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.
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)
No, doesn't seem to be fixed at all for me.
cuidAspect
still defines ID
as nullable, while it's set as non-nullable on the entity itself. Why have you decided to duplicate the properties anyway? And why does it only happen on service entities?
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.
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?
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.
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!
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.
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.
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.
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. 🙂
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
@siarhei-murkou thank you for the fix. Seems to work well.
Is there an existing issue for this?
Nature of Your Project
TypeScript
Current Behavior
Let's say we have
User
entity defined the following way:The class already inherits
cuidAspect
andmanagedAspect
.cuidAspect
defined as:Now, for whatever reason
cds-typer
decided to duplicate the inherited fields on the base class with different typing (ID?: string
vsID?: string | null;
):The duplicated fields appeared in the
service
class definitions but not in thedb schema
classes and because of this inconsistency I know get TS errors:Expected Behavior
No TS errors after
cds-typer
update.Steps To Reproduce
No response
Environment
Repository Containing a Minimal Reproducible Example
No response
Anything else?
The last tested
cds-typer
version without the TS errors:0.16.0