cap-js / cds-typer

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

[BUG] Generated index.ts files have syntax errors, e.g. "Cannot redeclare exported variable ..." #278

Closed D027152 closed 1 month ago

D027152 commented 1 month ago

Is there an existing issue for this?

Nature of Your Project

TypeScript

Current Behavior

In our project some generated files by cds typer contan syntax errors.

In detail

The error Property 'Integration' in type '(Anonymous class)' is not assignable to the same property in base type '_cuidAspect<{ new (...args: any[]): _managedAspect<{ new (...args: any[]): _SourceReferenceAspect<TBase>.(Anonymous class); prototype: _SourceReferenceAspect<any>.(Anonymous class); readonly actions: Record<...>; } & TBase>.(Anonymous class); prototype: _managedAspect<...>.(Anonymous class); readonly actions: Record...'. Type 'string' has no properties in common with type 'Integration'.

occurs in the files

Several additional errors, like e.g.

Cannot redeclare exported variable '_PaymentAgreementOutgoingAspect'.
 Cannot redeclare exported variable '_PaymentAgreementIncomingAspect'.
 Class 'PaymentAgreementIncoming' used before its declaration.

occur in the file

Expected Behavior

No syntax errors in generated files

Steps To Reproduce

repo https://github.tools.sap/erp4sme/crypto-for-business branch switch-to-cds-typer-payment-master-data above-mentioned files are located in https://github.tools.sap/erp4sme/crypto-for-business/tree/switch-to-cds-typer-payment-master-data/%40cds-models)

to re-generate the files run npm run cds-typer:dev

Environment

| c4b                    | github.tools.sap/erp4sme/crypto-for-business.git   |
| ---------------------- | -------------------------------- |
| @cap-js/audit-logging  | 0.6.0                            |
| @cap-js/cds-typer      | 0.23.0                           |
| @cap-js/cds-types      | 0.2.0                            |
| @cap-js/change-trackin | 1.0.6                            |
| @cap-js/telemetry      | 0.1.0                            |
| @sap/cds               | 7.9.3                            |
| @sap/cds-common-conten | 1.4.0                            |
| @sap/cds-compiler      | 4.9.6                            |
| @sap/cds-dk            | 7.9.5                            |
| @sap/cds-dk (global)   | 7.7.2                            |
| @sap/cds-fiori         | 1.2.3                            |
| @sap/cds-foss          | 5.0.0                            |
| @sap/cds-mtxs          | 1.18.2                           |
| @sap/eslint-plugin-cds | 3.0.4                            |
| Node.js                | v18.18.0                         |
| home                   | /Users/D027152/Documents/git/dch/crypto-for-business/node_modules/@sap/cds |

cds-typer version 0.23.0

Repository Containing a Minimal Reproducible Example

No response

Anything else?

This is a follow-up of https://github.com/cap-js/cds-typer/issues/270

hakimio commented 1 month ago

How does your entity definition look like. Sth like this maybe:

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

Are you redeclaring the ID?

D027152 commented 1 month ago

The entity that leads to the generated syntax error is DenormalizedPayables: https://github.tools.sap/erp4sme/crypto-for-business/blob/c05a6d6c8c89f2a4918803a4e996b7e5485d2e85/db/payment/payables/Payables.cds#L53

This entity contains the element "Integration", which is defined from Payables.Integration.name: https://github.tools.sap/erp4sme/crypto-for-business/blob/c05a6d6c8c89f2a4918803a4e996b7e5485d2e85/db/payment/payables/Payables.cds#L70 So this "Integration is a string.

The entity , where DenormalizedPayables bases on, is Payables: https://github.tools.sap/erp4sme/crypto-for-business/blob/c05a6d6c8c89f2a4918803a4e996b7e5485d2e85/db/payment/payables/Payables.cds#L20 This contains the aspect SourceReferences, which also contains an element Integration. This "Integration" is an association.

I assume that these two "Integrations" are somehow merged within the generation into the same class leading to the clash. However, I don't understand why. The entity DenormalizedPayables contains as the only element "Integration" the one being a string.

daogrady commented 1 month ago

Hi Stefan,

thanks for the report and the detailed analysis. The problem seems to pop up in scenarios where views/ projections use an alias that collides with the name of a property that is pulled into the view via inheritance. I have therefore completely removed inheritance clauses from views, as they should explicitly list their exposed fields anyway. Would you mind trying out[^1] the related PR to see if it addresses the issues you have found with your model?

Best, Daniel

[^1]: I have since added a second method to the wiki entry for trying out prerelease versions that is more convenient to do.

D027152 commented 1 month ago

Hi Daniel I tried your PR and can confirm that the following files are generated without syntax error, and they look good to me:

I have pushed the generation result in case you also want to have a look at it: https://github.tools.sap/erp4sme/crypto-for-business/tree/switch-to-cds-typer-payment-master-data/%40cds-models

The errors mentioned above in the following file still exist:

I assume that your correction was not meant to solve these as well. Can you please check and let me know if I shall open again a separate bug for this?

Thank you, Stefan

PS: Thank you for the new method to try out prerelease versions! By far more convenient!

daogrady commented 1 month ago

Hi Stefan,

glad we could (at least partially) resolve your issue!

I checked the remaining issue you found and it seems to be something else entirely. So if you wouldn't mind opening a new issue for that, that would be greatly appreciated! I could then just merge the fix for views, which will automatically close this issue here. Feel free to just link to the relevant part of this thread for the details instead of giving a full-blown description.

(On a side note: we consider the types generated by cds-typer to be ephemeral and therefore recommend not checking them into your VCS, but generate them on-site. You may have good reasons to persist the types in your repository, just wanted to mention it.)

Best, Daniel

D027152 commented 1 month ago

Thank you again for the fix!

I have created https://github.com/cap-js/cds-typer/issues/288 as follow-up for the remaining issue.

I am aware that the generated types are ephemeral fro your point of view, and I will check if we finally exclude them from version control.