[BUG] Type references are not assignable with primitive types in typescript

[aschmidt93]

Is there an existing issue for this?

• I have searched the existing issues

Nature of Your Project

TypeScript

Current Behavior

When using a type reference https://cap.cloud.sap/docs/cds/cdl#typereferences to re-use the type of a element of a entity, e.g. String, the generated property in TypeScript can not be assigned the respective primitive, e.g. string.

Expected Behavior

No response

Steps To Reproduce

    entity Foo {
        key name    : String(10);
        key counter : Integer;
    }

    event FooCreated {
        // re-use types
        fooName    : Foo:name;
        fooCounter : Foo:counter;
    }

    const eventParams: BooksServiceTypes.FooCreated = {
      // error
      fooName: "",
      // error
      fooCounter: 1,
    };

Environment

- **cds-typer**: 0.30.0
- **cds**: 8.5.0

Repository Containing a Minimal Reproducible Example

No response

Anything else?

No response

[daogrady]

Hi Andreas,

thank you for your report!

This only happens when the referred property in question is a key. When you remove key from name and regenerate the model, the problem goes away.
Do you actually want to manually assign values to a field that is marked as a key?

Best,
Daniel

[aschmidt93]

Hi Daniel,

Yes, I do want to re-use the type of an key. Consider the following example:

entity Orders : managed {
          @assert.format: '^\d+$'
      key ID              : String(10);
      // ...
}

event OrderCanceled {
    orderID  : Orders:ID;
   // ...
}

// more events

// returns created orderIDs
action createOrder(partialOrder : PartialOrder)                                      returns array of Orders:ID;

Admittedly, I could also define the ID type explicitly, like

type OrderID : String(10);

and re-use it. However, the type reference approach does not require an extra type.

[daogrady]

Hi Andreas,

just to be clear: reusing the type already works as of today, as long as you only read them:

this.on('READ', FooCreated, req => {
  console.log(req.data.fooName)
})

it's the assignment of a new value to this key property that throws the type system off. Manually assigning something to a key, which I generally assume to be a primary key, therefore runs at risk of violating uniqueness. So therefore my question: are you sure you need to have a key field you can write to manually? Do you expect fooName to be a key string, or just a string?

Best,
Daniel

[aschmidt93]

Ah, I see the problem. There are actually two use cases/problems.

are you sure you need to have a key field you can write to manually?

Yes, consider the example above. The ID of Orders is not auto-generated, as we want an incrementing ID and we have to use a string instead of int due to @cap-js/change-tracking only working with strings. Thus, when creating an order, the ID field needs to be assigned manually:

const order : OrdersService.Order = {
  // error
  ID : "0",
  // ...
}
await INSERT.into(Orders).entries(order);

For non-entity types, e.g. events, actions and functions, there are no key fields. In those cases, it's not relevant that the referenced type is a key, is it? Thus, there the primitive type is relevant. Consider the event example:

action cancelOrder(ID : Orders:ID);

event OrderCanceled {
    orderID  : Orders:ID;
   // ...
}

this.on("cancelOrder", req => {
  const order = await SELECT.one.from(Orders).where({ID : req.data.ID});
  // cancel order
  const eventParams : OrdersService.OrderCanceled = {
   // error
    orderID : order.ID
  }
  await this.emit("OrderCanceled", eventParams);
});

[daogrady]

Hi Andreas,

thanks for elaborating! We can probably make this work, but I'd like to check back with the team first:

@David-Kunz could you please let me know whether in the following case OrderCanceled.orderID is supposed to be treated as key & String(10), or just as String(10)? I.e.: is key a strict part of the type and therefore propagated via type references, or is it just a local annotation?

entity Orders : managed {
  key ID: String(10);
}

event OrderCanceled {
  orderID: Orders:ID;
}

Best,
Daniel

[stockbal]

Hi Daniel,

I think we can fix the DeepRequired type so the behavior is as expected.

// service.cds
entity Books : cuid {}

event BookCreated {
  bookId : Books:ID;
}

// _/index.ts
type RemoveKeyType<T> = T extends Key<infer U> ? U : T;

export type DeepRequired<T> = { 
    [K in keyof T]: DeepRequired<RemoveKeyType<T[K]>>
} & Exclude<Required<T>, null>;

// service/index.ts
// event
export declare class BookCreated {
  declare bookId: __.DeepRequired<Book>['ID'] | null;
}

the key part is still included, but it works. I have not tested if this could break something different where DeepRequired is used, but it looks promising.

Regards,
Ludwig

[daogrady]

Hi Ludwig,

thanks for weighing in! That might be a possible approach. At the very least this can be remedied where the type reference is actually used:

type Key<T> = T & { key: true }
type Unkey<T> = T extends Key<infer U> ? U : T

class Foo {
    declare prop: Key<string>
}

class Bar {
    declare prop: Unkey<Foo['prop']>  // DeepRequired be also here
}

new Bar().prop = '42'

which would not interfere with the key type elsewhere.

Bottom line is: we can certainly handle this. Just making sure this is actually supposed to be this way first. 😃

Best,
Daniel

[David-Kunz]

Key properties are not inherited:

​entity Orders : cuid {}
entity Foo { bar: Orders:ID; }

console.log(cds.model.definitions['Orders'].elements.ID.key) // true
console.log(cds.model.definitions['Foo'].elements.bar.key)   // false

[daogrady]

Thank you, David!

@aschmidt93 I have prepared a fix to address this.
Feel free to try it out before I merge and release it.

Best,
Daniel

[stockbal]

Hi Daniel,

wrapping Unkey around DeepRequired works fine for non key fields, but not for key field references

That's why I did the "unkeying" inside the DeepRequired.

Regards,
Ludwig

[daogrady]

Hi Ludwig,

thanks for trying out the fix! The reason I'd prefer to "unkey" the properties there is that DeepRequired is just a verbatim copy of how it is defined in cds-types (as you are likely aware).
At some point(TM) instead of duplicating that type (and others), we will just directly import it from cds-types. I have not investigated what the implications on cds-types would be, when we remove the key inside that deep-requiring mechanism. I'd have to check that out first.

Scratch that, I just noticed that your recent PR in cds-types actually removed any and all use of DeepRequired over there, so it bears no effect on cds-types at all. In that case, we can move on with your suggested solution instead.

Best,
Daniel

[daogrady]

Change included, can be tried out now

[stockbal]

Looks good 👌

[aschmidt93]

Hi Daniel,

the fix seems to do the trick, thank you :)

[daogrady]

Hi Andreas,

thanks for confirming, and also thanks Ludwig, for your suggestion and testing out the fix as well. 👍

Best,
Daniel