cap-js / cds-typer

CDS type generator for JavaScript
Apache License 2.0
25 stars 8 forks source link

[QUESTION] Why is the 'Date' built-in type represented as 'String'? #157

Closed mcastrup closed 4 months ago

mcastrup commented 4 months ago

Question

I noticed that an element of a CDS entity with the type 'Date' has the type 'string' in the generated types. E.g. see the element documentDate:

entity Receivables : cuid, managed, SourceReferences {
    displayId                : String; //readable ID
    Company                  : Association to one Companies; //Payer
    BusinessPartner          : Association to one BusinessPartners; //Payee
    fiatAmount               : Decimal; //Call reuse service to round due to allowed number of digits whereever necessary
    FiatAmountCurrency       : Association to one FiatCurrencies;
    cryptoAmount             : Decimal;
    CryptoAmountCurrency     : Association to one Cryptocurrencies;
    documentDate             : Date;
    dueDate                  : Date;
    LifecycleStatus          : Association to one codelists.ReceivableLifecycleStatusCodes;
    Network                  : Association to one networks.Networks;
    Account                  : Association to one Accounts;
    ExchangeRate             : Association to one ExchangeRates;
    PaymentAgreementIncoming : Association to one PaymentAgreementsIncoming;
}

leads to

export function _ReceivableAspect<TBase extends new (...args: any[]) => object>(Base: TBase) {
  return class Receivable extends Base {
        displayId?: string | null;
        Company?: __.Association.to<_sap_erp4sme_c4b_masterData_companies.Company> | null;
        Company_ID?: string | null;
        BusinessPartner?: __.Association.to<_sap_erp4sme_c4b_masterData_businessPartners.BusinessPartner> | null;
        BusinessPartner_ID?: string | null;
        fiatAmount?: number | null;
        FiatAmountCurrency?: __.Association.to<_sap_erp4sme_c4b_payment_fiatCurrencies.FiatCurrency> | null;
        FiatAmountCurrency_code?: string | null;
        cryptoAmount?: number | null;
        CryptoAmountCurrency?: __.Association.to<_sap_erp4sme_c4b_core_cryptocurrencies.Cryptocurrency> | null;
        CryptoAmountCurrency_ID?: string | null;
        documentDate?: string | null;
        dueDate?: string | null;
        LifecycleStatus?: __.Association.to<_sap_erp4sme_c4b_payment_receivables_codelists.ReceivableLifecycleStatusCode> | null;
        LifecycleStatus_code?: string | null;
        Network?: __.Association.to<_sap_erp4sme_c4b_core_networks.Network> | null;
        Network_ID?: string | null;
        Account?: __.Association.to<_sap_erp4sme_c4b_masterData_accounts.Account> | null;
        Account_ID?: string | null;
        ExchangeRate?: __.Association.to<_sap_erp4sme_c4b_core_cryptocurrencies.ExchangeRate> | null;
        ExchangeRate_ID?: string | null;
        PaymentAgreementIncoming?: __.Association.to<_sap_erp4sme_c4b_masterData_businessPartners.PaymentAgreementIncoming> | null;
        PaymentAgreementIncoming_ID?: string | null;
      static actions: {
    }
  };
}

There is a comment in your Builtins object, in which you already stated that 'Date' may be needed:

const Builtins = {
    UUID: 'string',
    String: 'string',
    Binary: 'string',
    LargeString: 'string',
    LargeBinary: 'Buffer | string | {value: import("stream").Readable, $mediaContentType: string, $mediaContentDispositionFilename?: string, $mediaContentDispositionType?: string}',
    Integer: 'number',
    UInt8: 'number',
    Int16: 'number',
    Int32: 'number',
    Int64: 'number',
    Integer64: 'number',
    Decimal: 'number',
    DecimalFloat: 'number',
    Float: 'number',
    Double: 'number',
    Boolean: 'boolean',
    // note: the date-related types _can_ be Date in some cases, but let's start with string
    Date: 'string',  // yyyy-mm-dd
    DateTime: 'string', // yyyy-mm-dd + time + TZ (precision: seconds
    Time: 'string',
    Timestamp: 'string', // yyy-mm-dd + time + TZ (ms precision)
    //
    Composition: 'Array',
    Association: 'Array'
}

We get now errors in our code like Type 'Date' is not assignable to type 'string', because the generated type of the element is 'string' instead of 'Date'.

Can you change this behavior or can it be configured somehow?

Thanks and best regards, Michael

daogrady commented 4 months ago

Hi Michael,

thanks for digging into to the code to find the culprit. I had confirmed with the runtime team beforehand that in almost all cases, cds Date is in fact supposed to be a string, hence my comment in the code. Could you please show the intended use in your code? Could be that you are just one .toString() away from the solution.

Best, Daniel

daogrady commented 4 months ago

Hi Michael,

I confirmed with the runtime team again that dates in your entities will in fact be strings during runtime. As they are stored as such in the database, converting them to JS Date objects every time would impose a performance hit on the runtime and the most common case for dates is to funnel them from the DB to the frontend anyway. If you need them as dates to do calculations on them, you can convert them to Date objects in your handler code:

this.on('READ', Receivables, ({data}) => {
  const d = new Date(data.documentDate)
  d.setDate(d.getDate() + 7)  // one week later
})

I will remove the misleading comment from the the Builtins.

Best, Daniel