[BUG][REGRESSION] TS4114 error introduced in v0.23.0

[hakimio]

Is there an existing issue for this?

• I have searched the existing issues

Nature of Your Project

TypeScript

Current Behavior

Transpiling code produced by v0.23.0 of cds-typer produces error:

X [ERROR] TS4114: This member must have an 'override' modifier because it overrides a member in the base class '_cuidAspect<{ new (...args: any[]): _managedAspect<{ new (...args: any[]): _oldCrmIdAspect<TBase>.(Anonymous class); prototype: _oldCrmIdAspect<any>.(Anonymous class); readonly actions: Record<...>; } & TBase>.(Anonymous class); prototype: _managedAspect<...>.(Anonymous class); readonly actions: Record<...>; } & { ....'. 

    ../models/CrmService/index.ts:9:8:
      9 │         ID?: string;
        ╵         ~~

Expected Behavior

No errors.

Steps To Reproduce

1. Add entity with some aspects:

entity Foo : cuid, managed {}

2. Generate the types
3. Try to transpile the TS code

Environment

- **OS**: Windows 10
- **Node**: 20.13.1
- **npm**: 10.8.1
- **cds-typer**: 0.23.0
- **cds**: 7.9.3

Repository Containing a Minimal Reproducible Example

No response

Anything else?

noImplicitOverride tsconfig config is set to true.

[daogrady]

Hi Tomas,

thanks for reporting this problem! As you have correctly deduced, this problem is directly related to the noImplicitOverride configuration. One layover solution would be to set noImplicitOverride to false.
I fully understand (and support) that you want to make your config as strict as possible.
I'd first have to evaluate whether just slapping override (and declare) on every property has any implications. It is currently a non-trivial issue to determine whether a property already exists in a parent aspect/ entity.

So for now, I recommend removing the noImplicitOverride option is possible. Resolving this issue needs a bit of digging.

Best,
Daniel

[hakimio]

Ok, for now I can disable noImplicitOverride but in the future the issue should definitely be addressed.

Anyway, another reason why I don't like the new v0.23.0 release is that it messes up the type tooltips in the IDE.

Here is how it looked like before in v0.22.0:

And here is how it looks like now in v0.23.0:

Really ugly.

[daogrady]

The latter issue probably stems from me removing the names of the classes generated by the class aspects. Your IDE looks a bit different from mine, but could you please open the generated types and give the class in question a name?
It should look something like this:

export function _UserAspect_<TBase extends new (...args: any[]) => object>(Base: TBase) {
  return class /* add "User" in here */ extends Base { ... };
}
export class User extends _UserAspect(__.Entity) {}

and let me know if adding the name back in prettifies the tooltip.

[hakimio]

Yes, it does fix the IDE tooltip.
I am using JetBrains WebStorm instead of VS Code.

[daogrady]

alright, that is good to know. I wanted to make the generated types a bit leaner and wasn't aware of the effect this would have. I will thus revert that particular change. Thanks for pointing it out.

[dragolea]

hi @daogrady
Will this be fixed in a newer version ?
tsc now throws an error:

error TS2416: Property 'ID' in type '(Anonymous class)' is not assignable to the same property in base type '_managedAspect<{ new (...args: any[]): _cuidAspect<TBase>.(Anonymous class); prototype: _cuidAspect<any>.(Anonymous class); readonly actions: Record<...>; } & TBase>.(Anonymous class) & _cuidAspect<...>.(Anonymous class) & object'.
  Type 'number | undefined' is not assignable to type 'string | undefined'.

226         ID?: number;
            ~~

version 0.22.0 works as expected.

[hakimio]

@dragolea
Might be an issue that your entity looks like the following:

entity Entities: cuid, managed {
    key ID: Integer;
}

cuid aspect sets ID to string and then you redefine it as integer again.

[dragolea]

@hakimio
Thanks for the tip, like always.

[hakimio]

Is it a good tip? 🙂

[dragolea]

yep, fixed the issue

[daogrady]

Hi @hakimio ,

I have prepared a fix that was originally meant to address a related problem. But when toying around with it, it looked like it could also be a fix for your issue. Would you mind trying it out to see if all your cases are addressed or it it needs additional work?

Best,
Daniel

[hakimio]

Hi @daogrady

The issue is almost fixed. Now I'm only getting the errors for actions property:

TS4114: This member must have an 'override' modifier because it overrides a member in the base class '_CodeListAspect<TBase>.CodeList & object'.

    ../models/sap/common/index.ts:71:22:
      71 │       static readonly actions: Record<never, never>
         ╵                       ~~~~~~~

[daogrady]

🤔 that is actually bad news, as we don't want to override in this case, but add to the type. I will have to investigate a bit more on this. Thanks for the feedback!

[daogrady]

Hi @hakimio,

I have extended the approach in the PR to handle the actions property as well. Would you mind checking again?

Best,
Daniel

[hakimio]

Hi @daogrady

While you have resolved the actions combination issue, declare is still required in front of actions like you've added for other properties. Right now I am getting the same error as before.

[daogrady]

Hi Tomas,

thanks for checking! I am a bit unclear on the situation. The current state in the PR I linked should generate the declare modifier for actions. Is that not the case?

Best,
Daniel

[daogrady]

Ah, I see the problem. That's what you get from code duplication. Should be fixed. 🙂

[hakimio]

Seems to be working well now 🙂

[hakimio]

@daogrady the issue has been reintroduced in 0.26.0:

X [ERROR] TS4114: This member must have an 'override' modifier because it overrides a member in the base class '_CodeListAspect<TBase>.CodeList & object'. [plugin angular-compiler]

    ../models/sap/common/index.ts:105:22:
      105 │       static readonly kind: 'entity' | 'type' | 'aspect' = 'entity';

[daogrady]

Hi Tomas,

thanks for pointing this out. This will actually be a bit more complicated to fix as this introduces two diametral requirements. I will look into it, but it could take a while.

Best,
Daniel