Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
1 task done
D027152 opened this issue Jul 18, 2024 · 6 comments · Fixed by #287
Closed
1 task done
Labels
bug Something isn't working new

Comments

@D027152
Copy link

D027152 commented Jul 18, 2024

Is there an existing issue for this?

  • I have searched the existing issues

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

  • @cds-models/sap/erp4sme/c4b/payment/payables/index.ts
  • @cds-models/sap/erp4sme/c4b/payment/receivables/index.ts
  • @cds-models/sap/erp4sme/c4b/payment/paymentOrders/index.ts
  • @cds-models/sap/erp4sme/c4b/payment/clearings/index.ts
  • @cds-models/sap/erp4sme/c4b/masterData/businessPartners/index.ts

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

  • @cds-models/sap/erp4sme/c4b/masterData/businessPartners/index.ts

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 #270

@hakimio
Copy link

hakimio commented Jul 19, 2024

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

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

Are you redeclaring the ID?

@D027152
Copy link
Author

D027152 commented Jul 19, 2024

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
Copy link
Contributor

daogrady commented Jul 31, 2024

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 out1 the related PR to see if it addresses the issues you have found with your model?

Best,
Daniel

Footnotes

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

@D027152
Copy link
Author

D027152 commented Jul 31, 2024

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

  • @cds-models/sap/erp4sme/c4b/payment/payables/index.ts
  • @cds-models/sap/erp4sme/c4b/payment/receivables/index.ts
  • @cds-models/sap/erp4sme/c4b/payment/paymentOrders/index.ts
  • @cds-models/sap/erp4sme/c4b/payment/clearings/index.ts
  • @cds-models/sap/erp4sme/c4b/masterData/businessPartners/index.ts

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:

  • @cds-models/sap/erp4sme/c4b/masterData/businessPartners/index.ts

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
Copy link
Contributor

daogrady commented Jul 31, 2024

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
Copy link
Author

D027152 commented Jul 31, 2024

Thank you again for the fix!

I have created #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working new
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants